Reinstate testRunnableRunsAtMostOnceAfterCancellation

[DaveCTurner]

This test was failing in #34004 due to a race, and although #34296 made
the failures rarer they did not actually fix the race. Then in #99201 we
fixed the race but the resulting test over-synchronizes and no longer
meaningfully verifies the concurrent behaviour we were originally trying
to check. It also fails for other reasons. This commit reverts back to
the original test showing that we might run the action at most once
after cancellation without any further synchronization, but fixes the
assertion to use the value of the counter observed immediately after the
cancellation since we cannot be sure that no extra iterations execute
before the cancellation completes.

[elasticsearchmachine]

Pinging @elastic/es-core-infra (Team:Core/Infra)

[thecoop]

@DaveCTurner I'm not sure what this change is doing, nor what the issues are with the current code. Could you run through the problems this fixes please?

[DaveCTurner]

The thing that brought it to my attention was that this test is still failing, see https://gradle-enterprise.elastic.co/s/oxapeenawfjii.

But the (original) point of this test is that a cancellation on any old random thread permits at most one more execution before the cancellation kicks in. With the changes in #99201 we're asserting something very different about how this component behaves when running in lockstep with the cancelling thread, which is not the situation that really matters in practice.

[thecoop]

Ah, I hadn't spotted the barrier breaks after an await timeout.

The previous version of the test wasn't asserting a range of runs, it was asserting a very specific number of runs - so it was testing that calling cancel actually does stop the schedule in a timely manner, which the test is still doing.

[DaveCTurner]

so it was testing that calling cancel actually does stop the schedule in a timely manner, which the test is still doing.

Yes, but this test has so much synchronization that it's going to pass even if, say, you remove the volatile modifier on org.elasticsearch.threadpool.Scheduler.ReschedulingRunnable#run

[thecoop]

OK. And this covers the case it was previously failing with - that there could be one additional run after the cancel.

[elasticsearchmachine]

💔 Backport failed

Status	Branch	Result
❌	8.10	Commit could not be cherrypicked due to conflicts
❌	7.17	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 99525