Skip to content

Commit 5c24a38

Browse files
committed
fix(batch): improve reconnect and shutdown mechanisms
- change condition for reconnecting -- send will always wait for recv to complete, so checking send.isDone is meaningless. Instead, we rely on the context's state and the data we've observer from the server. - Emit OOM's 'shutdown sequence' via the Recv handle, not via BatchContext::onEvent. This ensures the recv.done is completed correctly on EOF to unblock send's exit. - add a protoc guard to Reconnecting -- an EOF event should not arrive in this state. - fixed BatchContextTest so that stream is not automatically closed when client closes it's half
1 parent a208d7e commit 5c24a38

2 files changed

Lines changed: 101 additions & 73 deletions

File tree

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ private void trySend() {
519519
// processed all previous requests; the WIP buffer is empty in that case.
520520
//
521521
// It is possible that the server will be restarted or the stream will be
522-
// hungup before client receives all Results, in which case we might need
522+
// hung up before client receives all Results, in which case we might need
523523
// to re-submit the items remaining in the WIP buffer.
524524
recv.get();
525525

@@ -610,7 +610,12 @@ private Recv(BatchContext<?> context) {
610610
@Override
611611
public void onNext(Event event) {
612612
try {
613-
context.onEvent(event);
613+
if (event == Event.EOF) {
614+
// Handle synthetic EOF which the Oom state can send to initiate a shutdown.
615+
onCompleted();
616+
} else {
617+
context.onEvent(event);
618+
}
614619
} catch (Exception e) {
615620
context.onEvent(new Event.ClientError(e));
616621
}
@@ -748,11 +753,17 @@ private void onShuttingDown() {
748753

749754
private void onStreamClosed(Event event) {
750755
if (event instanceof Event.StreamHangup hangup) {
756+
// TODO(dyma): impprove logging
751757
hangup.exception().printStackTrace();
752758
}
753-
if (!send.isDone()) {
754-
setState(new Reconnecting(maxReconnectRetries));
759+
760+
// The only time we should not try to reconnect is if the context
761+
// is gracefully shutting down after a call to close() and the server
762+
// has returned Results for all previous batches.
763+
if (closed && wip.isEmpty()) {
764+
return;
755765
}
766+
setState(new Reconnecting(maxReconnectRetries));
756767
}
757768

758769
private void onClientError(Event.ClientError error) {
@@ -790,11 +801,12 @@ private void initiateShutdown() {
790801
// receive an Event.SHUTTING_DOWN, it would cancel this execution of this
791802
// very sequence. Instead, we delegate to our parent BaseState which normally
792803
// handles these events.
804+
final Recv events = (Recv) recv;
793805
if (!Thread.currentThread().isInterrupted()) {
794-
BatchContext.this.onEvent(Event.SHUTTING_DOWN);
806+
events.onNext(Event.SHUTTING_DOWN);
795807
}
796808
if (!Thread.currentThread().isInterrupted()) {
797-
BatchContext.this.onEvent(Event.EOF);
809+
events.onNext(Event.EOF);
798810
}
799811
}
800812

@@ -905,7 +917,10 @@ public void onEvent(Event event) {
905917
} else {
906918
reconnectAfter(2 ^ retries);
907919
}
920+
} else if (event == Event.EOF) {
921+
throw ProtocolViolationException.illegalStateTransition(this, event);
908922
} else {
923+
System.out.println("Event while reconnecting " + event);
909924
super.onEvent(event);
910925
}
911926

0 commit comments

Comments
 (0)