fix(queues): best effort batching during shutdown #11376
Conversation
|
|
|
|
|
Thank you for this fix - I would qualify it as a bug fix as in the scenarios where it helps, the non-batching behavior would be unexpected. In any case, a CHANGELOG entry should be added. |
JensErat
force-pushed
the
try-to-batch-during-shutdown
branch
from
fc37476
to
4d3e8a3
Compare
|
Rebased to get the "unreleased" section of the changelog and added the changelog entry. Should I squash the commits, or is this done when merging? |
The new batch queue completely disables batch processing during shutdown. When queue entries are sent to a backend service, the accumulated sequential callback execution time will likely exceed the forced shutdown timeout and some data lost.
The new batch queue completely disables batch processing during shutdown. When queue entries are sent to a backend service, the accumulated sequential callback execution time will likely exceed the forced shutdown timeout and some data lost.
JensErat
force-pushed
the
try-to-batch-during-shutdown
branch
from
4d3e8a3
to
adedeb4
Compare
|
@JensErat Sorry, there are new conflicts in your PR that need to be resolved. Once done, I'll squash when I merge the PR. |
Done, luckily only the changelog was affected. |
|
Thanks. We're working on changing how CHANGELOGs work to avoid such trivial conflicts in the future. Will merge after tests have passed. |
The previous fix (#11376) to enable batching during shutdown failed to check the shutdown flag while waiting for more items to batch. If the coalescing delay was large enough, this could cause the last batch to be lost.
The previous fix (#11376) to enable batching during shutdown failed to check the shutdown flag while waiting for more items to batch. If the coalescing delay was large enough, this could cause the last batch to be lost.
…#11607) The previous fix (#11376) to enable batching during shutdown failed to check the shutdown flag while waiting for more items to batch. If the coalescing delay was large enough, this could cause the last batch to be lost. (cherry picked from commit 3bf77d7) Co-authored-by: Hans Hübner <hans.huebner@gmail.com>
…#11608) The previous fix (#11376) to enable batching during shutdown failed to check the shutdown flag while waiting for more items to batch. If the coalescing delay was large enough, this could cause the last batch to be lost. (cherry picked from commit 3bf77d7) Co-authored-by: Hans Hübner <hans.huebner@gmail.com>
Summary
During worker shutdown, batching was stopped entirely. Even if hundreds or thousands of queued items are waiting to be processed, no batches would have been built but hundreds or thousands of individual handler calls executed sequentially (potentially resulting in forced shutdown due to added up latency).
By not stopping batching entirely, but instead falling back to the minimal coalescing time already defined, we still allow best-effort batching, which will optimize shutdown queue flush behavior.
I added two commits: the first added a (failing) test to show batching is broken, the other one fixes batching again.
This is a pretty minimal change and rather optimization, not a bugfix. Do we need a changelog entry/... for this?
Checklist
Jens Erat <jens.erat@mercedes-benz.com>, Mercedes-Benz Tech Innovation GmbH, imprint