Skip to content

Commit 802d1fc

Browse files
committed
fix(batch): send batch and transition to IN_FLIGHT atomically
1 parent 1e9fea4 commit 802d1fc

1 file changed

Lines changed: 29 additions & 5 deletions

File tree

src/main/java/io/weaviate/client6/v1/api/collections/batch/BatchContext.java

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,15 @@ void setState(State nextState) {
416416
}
417417
}
418418

419-
/** Blocks until a change in {@link #state} causes the predicate to be true. */
419+
/**
420+
* Blocks until a change in {@link #state} causes the predicate to be true.
421+
*
422+
* <p>
423+
* It is safe to acquire {@link #lock} before calling awaitState.
424+
* If a state that satisfies the predicate need to be awaited,
425+
* the {@link #stateChanged} will release the lock to let another
426+
* thread update the state.
427+
*/
420428
void awaitState(Predicate<State> predicate) throws InterruptedException {
421429
requireNonNull(predicate, "predicate is null");
422430

@@ -450,7 +458,9 @@ void closeStream() {
450458
* <p>
451459
* Be mindful that most of the time this callback will run in a hot path
452460
* on a gRPC thread. {@link State} implementations SHOULD offload any
453-
* blocking operations to one of the provided executors.
461+
* blocking operations to one of the provided executors. Because onEvent
462+
* will hold the {@link #lock}, no state changes are guaranteed to happen
463+
* until {@link State#onEvent} returns.
454464
*
455465
* @see #scheduledService
456466
*/
@@ -594,9 +604,23 @@ private void drain() throws InterruptedException {
594604
* @see #IN_FLIGHT
595605
*/
596606
private void flush() throws InterruptedException {
597-
awaitState(State::canSend);
598-
messages.onNext(batch.prepare());
599-
setState(IN_FLIGHT);
607+
lock.lock();
608+
try {
609+
awaitState(State::canSend);
610+
611+
// Send and transition to IN_FLIGHT MUST be done atomically.
612+
//
613+
// Without synchronization, there's a potential race
614+
// where the server Acks the next batch _before_ IN_FLIGHT state
615+
// is set, so when finally set it may block forever. This will most
616+
// likely only manifest in tests, where batches are acked instantly,
617+
// but it's good to have the extra safety layer.
618+
messages.onNext(batch.prepare());
619+
setState(IN_FLIGHT);
620+
} finally {
621+
lock.unlock();
622+
}
623+
600624
awaitState(State::canPrepareNext);
601625
}
602626

0 commit comments

Comments
 (0)