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 592 forks source link

Discuss timeout managment on ChannelPipeline object #20975

Open epj opened 2 years ago

epj commented 2 years ago

In dev/com.ibm.ws.sipcontainer/src/com/ibm/ws/sip/channel/resolver/impl/netty/SipResolverUdpTransport.java there is a TODO comment followed by commented out lines setting timeout values for a ChannelPipeline object. Review comment asks if this TODO needs a revisit. We should discuss if any timeout values should be changed on this object.

mrsaldana commented 2 years ago

There is already some consideration made in io.openliberty.netty.internal.impl . The TCPChannelInitializerImpl gets the tcpOptions defined inactivity timeout to set a new InactivityTimeoutHandler which extends Netty's IdleStateHandler . It is worth mentioning that Netty provides channel handlers for both reading and writing via the constructs ReadTimeoutHandler and WriteTimeoutHandler. These could be programmatically added to the beginning of the pipeline after it has been created.

For instance, the creation of the pipeline could include these handlers by simply doing:

channel.pipeline().addLast("readTimeout", new ReadTimeoutHandler(config.getReadTimeout, TimeUnit.MILLISECONDS)) AND/OR channel.pipeline().addLast("writeTimeout", new WriteTimeoutHandler(config.getWriteTimeout, TimeUnit.MILLISECONDS));

It is also worth noting that a ChannelOption provided by Netty is CONNECT_TIMEOUT_MILLIS. The combination of the read and write timeout with the connect ChannelOption, might make defining an InactivityTimeoutHandler unnecessary. Since it extends the IdleStateHandler, I'd like to investigate further what scenarios trigger the IdleStateHandler that are not already covered by the connect option and the read and write handlers.

The ChanelOption gets added to the bootstrapping, so it could look something similar to:

public static ServerBootstrapExtended createTCPBootstrap(NettyFrameworkImpl framework, Map<String, Object> tcpOptions) throws NettyException {
        BootstrapConfiguration config = new TCPConfigurationImpl(tcpOptions, true);
        ServerBootstrapExtended bs = new ServerBootstrapExtended();
        bs.group(framework.parentGroup, framework.childGroup);
        bs.channel(NioServerSocketChannel.class)
        // ---> Channel option
        .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, config.getConnetTimeoutMillis());

        // apply the existing user config to the Netty TCP channel
        bs.applyConfiguration(config);
        ChannelInitializerWrapper tcpInitializer = new TCPChannelInitializerImpl(config);
        bs.setBaseInitializer(tcpInitializer);
        return bs;
    }

Another popular feature that can have an associated timeout is keep-alive. Netty also has an out-of-the-box handler for this HttpServerKeepAliveHandler . For keep alive requests, we could dynamically alter the existing pipeline to change the read timeout handler to use the configured keep-alive timeout. On successful read, the pipeline can be adjusted again to revert back to the original read timeout. This path is still being investigated. The HTTP channel options we currently provide do allow defining a persistTimeout, so this dynamic update of the read timeout I believe to be worth exploring. Alternatively, the inactivity timeout handler could also be updated in a similar fashion.

epj commented 2 years ago

Existing timeouts defined in server elements:

channelfw
    chainQuiesceTimeout -> io.liberty.netty.internal.impl, com.ibm.ws.sipcontainer

sipEndpoint
    inactivityTimeout -> in io.openliberty.netty.internal.impl
    sessionTimeout -> in com.ibm.ws.sipcontainer
    sslSessionTimeout -> in com.ibm.ws.channel.ssl

sipStack
    timerB
    timerF

Potential additional places to specify timeout values:

SipResolverTcpTransport, SipResolverUdpTransport: pipeline.addLast("writeTimeoutHandler", new WriteTimeoutHandler(WRITE_TIMEOUT, TimeUnit.MILLISECONDS)); pipeline.addLast("readTimeoutHandler", new ReadTimeoutHandler(READ_TIMEOUT, TimeUnit.MILLISECONDS));

Still to investigate:
Is keepAlive used / should be used at transport level? What are defaults for read/wr/connect timeouts for chfw? How to carry them to netty? Use global approach for channel creation, netty framework impl, in SIP? domainResolver addTtl parm - what does it do really?

epj commented 2 years ago

Regarding keepAlive, the SIP stack will respond to a keep alive packet from a client, but otherwise doesn't support keepAlive. Instead there are various timers on the sipStack element that do a finer grained control of the connection. See this link for details: https://www.ibm.com/docs/en/was-liberty/nd?topic=configuration-sipstack

epj commented 2 years ago

Regarding TTL (time to live), SIP is using the term TTL in the code for a few slightly different things, but these are different from the IP standard where packets have a TTL value in them which is decremented as each router forwards it.

In SIP 'ttl' can mean session timeout, like in com/ibm/ws/sip/container/was/SipAppAnnotationsInfo.java. It can also refer to networkaddressCacheTtl, the time to keep an address in cache, as in com/ibm/ws/sip/properties/StackProperties.java. There is provision to pass the value in resolver URIs that can be turned on with \<domainResolver addTtl="true"/>

In any case there is no equivalent function in Netty and this is all implemented at the SIP container level.

epj commented 2 years ago

Currently there are 177 references to timeout (case insensitive) in chfw specific functions whereas there are 112 references in netty specific functions. I'll look more closely at these to see if they can be mined for more opportunities to carry a configurable timeout to the netty side.

epj commented 2 years ago

Looking for the SIP FATs, timerB and timerF are used but the remaining parameters are not used at all in the testing: chainQuiesceTimeout, inactivityTimeout, sessionTimeout, sslSessionTimeout.

epj commented 2 years ago

To clarify an earlier point, currently SipResolverTcpTransport is the only place where WriteTimeoutHandler and ReadTimeoutHandler are currently used. They're commented out in SipResolverUdpTransport.

epj commented 2 years ago

In chfw, SipResolverTcpTransport also uses fixed values for timeouts, so these are not currently configurable by server.xml elements:

private static final int                READ_TIMEOUT =  1500;
private static final int                WRITE_TIMEOUT = 5000;
private static final int                MAX_READ_TIMEOUT_COUNT = 5;
private static final int                MAX_WRITE_QUEUE_SIZE = 5000;

SipResolverUdpTransport for chfw does not specify anything timeout related.

pnicolucci commented 2 years ago

Current status is we need to check the JMS code to see if there are any timeouts we need to address @isaacrivriv