Also manage graceful shutdown of already started async requests #5173
Conversation
Signed-off-by: Thomas Draebing <[email protected]>
| @@ -189,7 +189,7 @@ else if (_wrapWarning.compareAndSet(false, true)) | |||
| _dispatchedStats.decrement(); | |||
| _dispatchedTimeStats.record(dispatched); | |||
|
|
|||
| if (state.isSuspended()) | |||
| if (state.isSuspended() || state.isAsyncStarted()) | |||
There was a problem hiding this comment.
@gregw Do you think we still need to keep the state.isSuspended() check here. The state.isAsyncStarted() doesn't check for the WAITING state.
There was a problem hiding this comment.
@thomasdraebing firstly, yes this is indeed the bug. We spotted it yesterday and were in process of fixing.
The isSuspended() check is not necessary as isAsyncStarted in his context will always be for a request that is being handled, hence all we need is the !blocking check that it will do.
@lachlan-roberts are you working on the unit tests for this plus the other things that we wanted changed? If so, then perhaps pull @thomasdraebing commit into your branch then we can close this one.
There was a problem hiding this comment.
I rebased my changes ontop of this branch and opened PR #5175.
This fixes #5105. The issue was that the async request could already be started, when reaching the finally-block, where the callback handler to facilitate the graceful shutdown is added. In that case, the callback handler is not added, and the stat handler will already count the request as finished.
Signed-off-by: Thomas Draebing [email protected]