Fix edge case for active flag for flush on idle #97332
Conversation
Basically the active flag needs to be set to true after the op has been processed by the engine. Else there is an edge case, which may leave unprocessed ops and active false, without a next flush on idle scheduled. Introducing a test for this edge case. Fixes elastic#97154
kingherc
added
>enhancement
:Distributed/Engine
|
Hi @kingherc, I've created a changelog YAML for you. |
|
Pinging @elastic/es-distributed (Team:Distributed) |
| t.start(); | ||
| assertBusy(() -> assertThat(rwl.getQueueLength(), equalTo(1))); | ||
| assertFalse(shard.isActive()); | ||
| } // Allow op to complete |
There was a problem hiding this comment.
Should we also trigger a flush on idle here? If I understand correctly we want to test a race between an ongoing operation and the flush on idle?
There was a problem hiding this comment.
Well, the race was happening if the flush on idle completed before the operation was fully processed. Now that we set the active flag after the operation has been processed, we do not have the race anymore.
Putting a flush on idle here will basically be a no-op because the active flag is already false and it will be skipped (the flush on idle actually flushes if the active flag is true).
There was a problem hiding this comment.
Right, a flush on idle will now be a noop so my suggestion won't make much sense.
Thanks for confirming my understanding
Basically the active flag needs to be set to true after the op has been processed by the engine. Else there is an edge case, which may leave unprocessed ops and active false, without a next flush on idle scheduled. Introducing a test for this edge case.
Fixes #97154