Allow to disable automatically sending PING acks.#9338
Conversation
1bd9bd0 to
8bb9e99
Compare
8bb9e99 to
4807a01
Compare
Motivation: There are situations where the user may want to be more flexible when to send the PING acks for various reasons or be able to attach a listener to the future that is used for the ping ack. To be able to do so we should allow to manage the acks manually. Modifications: - Add constructor to DefaultHttp2ConnectionDecoder that allows to disable the automatically sending of ping acks (default is to send automatically to not break users) - Add methods to AbstractHttp2ConnectionHandlerBuilder (and sub-classes) to either enable ot disable auto acks for pings - Make DefaultHttp2PingFrame constructor public that allows to write acks. - Add unit test Result: More flexible way of handling acks.
4807a01 to
dffd22c
Compare
| } | ||
|
|
||
| /** | ||
| * Determine if PING frame should automatically be acknowledged or not. |
There was a problem hiding this comment.
Was going to suggest to mention here that the default is true but I guess the doc is consistent with autoAckSettingsFrame.
|
This only question about this is if we also need to somehow ensure we ack back in the same order or if its the responsibility to do... I mean I can even not find it really called in the RFC that it needs to be in the same order but I would expect that a lot of implementations may break if it is not the same order.. /cc @ejona86 @Scottmitch @carl-mastrangelo So for example what we could do is have a queue with longs and on each ping ack verify that the payload is the same as for the ping and if not fail the write or tear down the connection with a connection error. |
|
After thinking more about this (and talking with @ejona86) I think it should just be the users responsibility to ensure correct ordering :) |
|
@ejona86 want to take a look as well ? |
|
Yeah, I'll take a look. |
Motivation: There are situations where the user may want to be more flexible when to send the PING acks for various reasons or be able to attach a listener to the future that is used for the ping ack. To be able to do so we should allow to manage the acks manually. Modifications: - Add constructor to DefaultHttp2ConnectionDecoder that allows to disable the automatically sending of ping acks (default is to send automatically to not break users) - Add methods to AbstractHttp2ConnectionHandlerBuilder (and sub-classes) to either enable ot disable auto acks for pings - Make DefaultHttp2PingFrame constructor public that allows to write acks. - Add unit test Result: More flexible way of handling acks.
Motivation:
There are situations where the user may want to be more flexible when to send the PING acks for various reasons or be able to attach a listener to the future that is used for the ping ack. To be able to do so we should allow to manage the acks manually.
Modifications:
Result:
More flexible way of handling acks.