BulkIngester requestsInFlightCount thread-safety

[ginkel]

Java API client version

main

Java version

17

Elasticsearch Version

n/a

Problem description

While going through the new BulkIngester's sources I noticed that the requestsInFlightCount is manipulated by multiple threads, but it is not obvious whether that happens in a thread-safe fashion in all cases (it probably is, just not very obvious).

In addition (this is more like a feature request), I think it would be nice to expose the requestsInFlightCount (just like operationsCount()) for external consumption by metrics as this is probably some very valuable information about the BulkIngester's performance.

[swallez]

Thanks for looking at it @ginkel!

To answer your question, requestsInFlightCount and the other state variables are only modified under the control of an instance of the FnCondition class (as in "functional condition"). The goal of this class is to make it easier to use a Condition and the associated Lock as it's easy to mess things up.

The methods of this helper class make sure the associated lock is held before calling their lambda expression argument, and also make sure the predicate that is associated with the Condition is verified before signally any waiting threads.

And indeed exposing requestsInFlightCount for monitoring purposes is a good idea. Same for currentSize (number of buffered operations what will be part of the next request). I'll prepare a PR for this.

[ginkel]

Thanks for the explanation! We actually found a real race condition in FnCondition during tests (unrelated to requestsInFlightCount), #502 should fix it.

[swallez]

@ginkel PR #513 adds additional properties that expose the BulkIngester's current state.