vy commented on PR #2767: URL: https://github.com/apache/logging-log4j2/pull/2767#issuecomment-2254161981
For one, I don't think it is correct to keep the state (i.e., `SSLContext`) in configuration DTOs. I think the state should have rather been implemented as a part of `SslSocketManager`. But I guess it is too late for that given `SslConfiguration#getSslContext()` et al. As a result, we need to manage the state in several places: `SslConfiguration`, `SslSocketManager`, `SocketAppender`, etc. @ppkarwasz, in the light of your feedback, I revised my proposal as follows: 1. Introduce `getId()` to `SslConfiguration` and use it in the `name` provided to `getManager()` in `SslSocketManager`. This will ensure `SslSocketManager` will evict old connections upon a reconfiguration attempt with a different `SslConfiguration#getId()` value. 2. Introduce `reloadInterval` to `SslConfiguration` and use it with `Configuration#getScheduler().schedule()` in `SocketAppender` to periodically call `Configuration#getLoggerContext().reconfigure()` – iff, `reloadInterval` is provided and `protocol` is SSL. > Reload the TLS socket. The easiest way to do it is to trigger a Log4j Core reconfiguration. @ppkarwasz, is it so? Can't we instead create a new `DelegatingOutputStreamManager` extending from `SslSocketManager` that allows gracefully swapping (i.e., start the new, set the new, stop the old) the underlying `SslSocketManager` with another one? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
