Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discuss CHFW Server Configuration #25028

Open
isaacrivriv opened this issue Apr 17, 2023 · 3 comments
Open

Discuss CHFW Server Configuration #25028

isaacrivriv opened this issue Apr 17, 2023 · 3 comments

Comments

@isaacrivriv
Copy link
Member

After going through a review of #21350, we noticed that the Netty bundle uses the CHFW bundle for config properties such as with DefaultChainQuiesceTimeout and some transport properties could be missing from implementation as stated in the CHFW server config documentation

To remove the dependency from the CHFW bundle we should probably change this to get the properties in another way. Discussing options with @mrsaldana we came up with

  1. Having a generic transport config item eventually replacing the chfw config
    • Which will have more priority?
    • Will we still for the time being reuse the properties for the existing config and parse them with Netty for backwards compatibility?
    • How would conflicting configs be determined?
  2. Using the same existing config
    • Misleading since chfw could not be used
    • Would probably need to be renamed eventually?
  3. Controlling it inside the code
    • Making use of defaults and OSGi with code determination to determine what configs to load or not.

In the UFO we have the following points.

Existing channel configurations and behaviors will not change (to the extent possible)

We will strive to preserve existing configuration properties and behaviors with this feature

Any configurations or behaviors that have the potential to change from a user perspective will be socialized

So in those terms I believe probably option 2 would be the one aimed better based on the UFOs perspective.

@hutchig
Copy link
Contributor

hutchig commented Apr 18, 2023

This comment used for things we want to include for configuration 'changes' in the "Service Transfer Education"

@isaacrivriv
Copy link
Member Author

After discussing with the team, we decided to follow option 2 keeping the same config item since the channelfw even if it internally means a different framework to developers, externally it really shouldn't affect/change for users. We will keep this issue open to verify all existing config items as documented in https://openliberty.io/docs/latest/reference/config/channelfw.html are valid for Netty. We should do the same for all other config options like TCPOptions, HTTPOptions, and SSLOptions which looks like #17989 is being used to verify.

For now we will try to see how to parse the channelfw config parsed in Netty if possible without pulling in the channelfw bundle.

@isaacrivriv
Copy link
Member Author

After talking with @joe-chacko, there is a way to have multiple components consuming the same config by using the same configurationPid in the component annotation. Similar to what OutboundSecureFacetImpl and CommsOutboundChain do. I ran a quick test locally by changing the annotation in NettyFrameworkImpl to have the configurationPid point to com.ibm.ws.channelfw and verified the activate method was called with the channelfw config passed so this may be a viable alternative to consume the config without having to pull in the channelfw bundle. Need to look more into this and discuss.

[7/11/23, 13:45:18:332 EDT] 0000002d id=5d7b1cf8 io.openliberty.netty.internal.impl.NettyFrameworkImpl 1 Activate called
org.apache.felix.scr.impl.manager.ComponentContextImpl@a9c3fc9d
{osgi.ds.satisfying.condition.target=(osgi.condition.id=true), component.name=io.openliberty.netty.internal.impl.NettyFrameworkImpl, chainQuiesceTimeout=30000, config.source=file, service.vendor=IBM, warningWaitTime=10000, service.pid=com.ibm.ws.channelfw, chainStartRetryAttempts=60, config.overrides=true, component.id=297, config.displayId=channelfw, chainStartRetryInterval=5000}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Design Issues
Development

No branches or pull requests

3 participants