hibernate-reactive
hibernate-reactive copied to clipboard
Lock-free BlockingIdentifierGenerator LoHi
This is a version that pay a single allocation cost per each blockSize to save any synchronized to happen.
It can be made gc-free by playing some bit tricks packing both lo/hi into a long (and assuming there are no negative values for both, likely) - or with some more complex coordination mechanism (yet to be designed).
I don't know few things yet before un-drafting:
- If many concurrent
state.next(hi)happen do we need to enforce validation of (hi,lo) based on existing values? new hi > old hi (or diff by blockSize?) && old low >= blockSize. - If many concurrent
state.next(hi)happen do we need to ensure some ordering of updates? eg concurrentnext(10), next(20),next(30)updates in this pr are not yet guaranteed to be set in any specific order and I cannot see any ordering enforced in the original code, but better be safe then sorry with concurrency - blockSize is a constant? I'm assuming it is...
@Sanne @DavideD I'm a bit worried about the original code:
see
final long next = next();
if ( next != -1 ) {
return CompletionStages.completedFuture( next );
}
if ( getBlockSize() <= 1 ) {
return nextHiValue( connectionSupplier )
.thenApply( i -> next( i ) );
}
What happen if 2 concurrent threads both see -1 because they both exhausted the (same) current sequences?
They both compete to query the database to acquire new ones and compete (again) to update hi.
I think this won't end up well unless there's some missing assumption I don't know about the way things are executed here...
And the same can happen with a combiner as well: while within the combiner you can have more then one updates in flights, each one competing to both hit the DB and refresh the new hi.
I'd challenge you in ever noticing the current synchronized on profiling data :)
I'd challenge you in ever noticing the current synchronized on profiling data :)
eheh I know, but it won't cost anything to make it lock free like this I think, I'm more concerned instead by this -> https://github.com/hibernate/hibernate-reactive/pull/1610#issuecomment-1519467511
To quote myself of the future (and @Sanne ) re https://github.com/hibernate/hibernate-reactive/pull/1610#issuecomment-1519467511
this is not a "real" problem leading to break anything, but just an efficiency one; in short, if more then one concurrently try to increase the sequence due to exhaustion and succeed, but update hi/lo in the "wrong" order (ie by creating gaps or "jumping forward" before previously existing ones will be "consumed") it can just cause the consumers to request to the DB more sequences then necessary (again) because in both cases (missing new or "old" created batches) would still cause someone to be left unsatisfied.
The existing tests are not verifying this hence there was no reason to noticed that.
@Sanne undrafting, but feel free to pick what you like for next works on this bud ;)