OpenLiberty / open-liberty

Open Liberty is a highly composable, fast to start, dynamic application server runtime environment
https://openliberty.io
Eclipse Public License 2.0
1.15k stars 587 forks source link

Discuss CHFW Server Configuration #25028

Open isaacrivriv opened 1 year ago

isaacrivriv commented 1 year ago

After going through a review of https://github.com/OpenLiberty/open-liberty/pull/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 commented 1 year ago

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

isaacrivriv commented 1 year ago

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 https://github.com/OpenLiberty/open-liberty/issues/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 commented 1 year ago

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}