feat: Ability to reset subscriber upon out of band seek (#662)

Prepares for handling the RESET signal from the server, by:
- Discarding outstanding acks for delivered messages.
- Waiting for the committer to flush pending commits and receive the acknowledgment from the server.
- Then resetting subscriber state, including canceling any in-flight client seeks.

Author: tmdiep

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/cloudpubsub/SubscriberSettings.java

@@ -260,7 +260,11 @@ Subscriber newPartitionSubscriber(Partition partition) throws CheckedApiExceptio
               partition, transformer().orElse(MessageTransforms.toCpsSubscribeTransformer())),
           new AckSetTrackerImpl(wireCommitter),
           nackHandler().orElse(new NackHandler() {}),
-          messageConsumer -> wireSubscriberBuilder.setMessageConsumer(messageConsumer).build(),
+          (messageConsumer, resetHandler) ->
+              wireSubscriberBuilder
+                  .setMessageConsumer(messageConsumer)
+                  .setResetHandler(resetHandler)
+                  .build(),
           perPartitionFlowControlSettings());
     } catch (Throwable t) {
       throw toCanonical(t);

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/cloudpubsub/internal/AckSetTracker.java

@@ -24,4 +24,8 @@ interface AckSetTracker extends ApiService {
   // Track the given message. Returns a Runnable to ack this message if the message is a valid one
   // to add to the ack set. Must be called with strictly increasing offset messages.
   Runnable track(SequencedMessage message) throws CheckedApiException;
+
+  // Discard all outstanding acks and wait for any pending commit offset to be acknowledged by the
+  // server. Throws an exception if the committer shut down due to a permanent error.
+  void waitUntilCommitted() throws CheckedApiException;
 }

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/cloudpubsub/internal/AckSetTrackerImpl.java

@@ -28,21 +28,63 @@
 import com.google.cloud.pubsublite.internal.ExtractStatus;
 import com.google.cloud.pubsublite.internal.TrivialProxyService;
 import com.google.cloud.pubsublite.internal.wire.Committer;
+import com.google.common.collect.ImmutableList;
 import com.google.errorprone.annotations.concurrent.GuardedBy;
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.List;
 import java.util.Optional;
 import java.util.PriorityQueue;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 public class AckSetTrackerImpl extends TrivialProxyService implements AckSetTracker {
+  // Receipt represents an unacked message. It can be cleared, which will cause the ack to be
+  // ignored.
+  private static class Receipt {
+    final Offset offset;
+
+    private final CloseableMonitor m = new CloseableMonitor();
+
+    @GuardedBy("m.monitor")
+    private boolean wasAcked = false;
+
+    @GuardedBy("m.monitor")
+    private Optional<AckSetTrackerImpl> tracker;
+
+    Receipt(Offset offset, AckSetTrackerImpl tracker) {
+      this.offset = offset;
+      this.tracker = Optional.of(tracker);
+    }
+
+    void clear() {
+      try (CloseableMonitor.Hold h = m.enter()) {
+        tracker = Optional.empty();
+      }
+    }
+
+    void onAck() {
+      try (CloseableMonitor.Hold h = m.enter()) {
+        if (!tracker.isPresent()) {
+          return;
+        }
+        if (wasAcked) {
+          CheckedApiException e =
+              new CheckedApiException("Duplicate acks are not allowed.", Code.FAILED_PRECONDITION);
+          tracker.get().onPermanentError(e);
+          throw e.underlying;
+        }
+        wasAcked = true;
+        tracker.get().onAck(offset);
+      }
+    }
+  }
+
   private final CloseableMonitor monitor = new CloseableMonitor();
 
   @GuardedBy("monitor.monitor")
   private final Committer committer;
 
   @GuardedBy("monitor.monitor")
-  private final Deque<Offset> receipts = new ArrayDeque<>();
+  private final Deque<Receipt> receipts = new ArrayDeque<>();
 
   @GuardedBy("monitor.monitor")
   private final PriorityQueue<Offset> acks = new PriorityQueue<>();
@@ -57,23 +99,27 @@ public AckSetTrackerImpl(Committer committer) throws ApiException {
   public Runnable track(SequencedMessage message) throws CheckedApiException {
     final Offset messageOffset = message.offset();
     try (CloseableMonitor.Hold h = monitor.enter()) {
-      checkArgument(receipts.isEmpty() || receipts.peekLast().value() < messageOffset.value());
-      receipts.addLast(messageOffset);
+      checkArgument(
+          receipts.isEmpty() || receipts.peekLast().offset.value() < messageOffset.value());
+      Receipt receipt = new Receipt(messageOffset, this);
+      receipts.addLast(receipt);
+      return receipt::onAck;
     }
-    return new Runnable() {
-      private final AtomicBoolean wasAcked = new AtomicBoolean(false);
+  }
 
-      @Override
-      public void run() {
-        if (wasAcked.getAndSet(true)) {
-          CheckedApiException e =
-              new CheckedApiException("Duplicate acks are not allowed.", Code.FAILED_PRECONDITION);
-          onPermanentError(e);
-          throw e.underlying;
-        }
-        onAck(messageOffset);
-      }
-    };
+  @Override
+  public void waitUntilCommitted() throws CheckedApiException {
+    List<Receipt> receiptsCopy;
+    try (CloseableMonitor.Hold h = monitor.enter()) {
+      receiptsCopy = ImmutableList.copyOf(receipts);
+    }
+    // Clearing receipts here avoids deadlocks due to locks acquired in different order.
+    receiptsCopy.forEach(Receipt::clear);
+    try (CloseableMonitor.Hold h = monitor.enter()) {
+      receipts.clear();
+      acks.clear();
+      committer.waitUntilEmpty();
+    }
   }
 
   private void onAck(Offset offset) {
@@ -82,7 +128,7 @@ private void onAck(Offset offset) {
       Optional<Offset> prefixAckedOffset = Optional.empty();
       while (!receipts.isEmpty()
           && !acks.isEmpty()
-          && receipts.peekFirst().value() == acks.peek().value()) {
+          && receipts.peekFirst().offset.value() == acks.peek().value()) {
         prefixAckedOffset = Optional.of(acks.remove());
         receipts.removeFirst();
       }

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/cloudpubsub/internal/ResettableSubscriberFactory.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2021 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.cloud.pubsublite.cloudpubsub.internal;
+
+import com.google.api.gax.rpc.ApiException;
+import com.google.cloud.pubsublite.SequencedMessage;
+import com.google.cloud.pubsublite.internal.wire.Subscriber;
+import com.google.cloud.pubsublite.internal.wire.SubscriberResetHandler;
+import com.google.common.collect.ImmutableList;
+import java.io.Serializable;
+import java.util.function.Consumer;
+
+public interface ResettableSubscriberFactory extends Serializable {
+  Subscriber newSubscriber(
+      Consumer<ImmutableList<SequencedMessage>> messageConsumer,
+      SubscriberResetHandler resetHandler)
+      throws ApiException;
+}

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/cloudpubsub/internal/SinglePartitionSubscriber.java

@@ -30,7 +30,6 @@
 import com.google.cloud.pubsublite.internal.CheckedApiException;
 import com.google.cloud.pubsublite.internal.ExtractStatus;
 import com.google.cloud.pubsublite.internal.ProxyService;
-import com.google.cloud.pubsublite.internal.wire.SubscriberFactory;
 import com.google.cloud.pubsublite.proto.FlowControlRequest;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
@@ -50,15 +49,16 @@ public SinglePartitionSubscriber(
       MessageTransformer<SequencedMessage, PubsubMessage> transformer,
       AckSetTracker ackSetTracker,
       NackHandler nackHandler,
-      SubscriberFactory wireSubscriberFactory,
+      ResettableSubscriberFactory wireSubscriberFactory,
       FlowControlSettings flowControlSettings)
       throws ApiException {
     this.receiver = receiver;
     this.transformer = transformer;
     this.ackSetTracker = ackSetTracker;
     this.nackHandler = nackHandler;
     this.flowControlSettings = flowControlSettings;
-    this.wireSubscriber = wireSubscriberFactory.newSubscriber(this::onMessages);
+    this.wireSubscriber =
+        wireSubscriberFactory.newSubscriber(this::onMessages, this::onSubscriberReset);
     addServices(ackSetTracker, wireSubscriber);
   }
 
@@ -126,4 +126,10 @@ public void onSuccess(Void result) {
       onPermanentError(ExtractStatus.toCanonical(t));
     }
   }
+
+  @VisibleForTesting
+  boolean onSubscriberReset() throws CheckedApiException {
+    // TODO: handle reset.
+    return false;
+  }
 }

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/wire/ApiExceptionCommitter.java

@@ -21,6 +21,7 @@
 import com.google.api.core.ApiFuture;
 import com.google.api.gax.rpc.ApiException;
 import com.google.cloud.pubsublite.Offset;
+import com.google.cloud.pubsublite.internal.CheckedApiException;
 import com.google.cloud.pubsublite.internal.TrivialProxyService;
 
 class ApiExceptionCommitter extends TrivialProxyService implements Committer {
@@ -35,4 +36,9 @@ class ApiExceptionCommitter extends TrivialProxyService implements Committer {
   public ApiFuture<Void> commitOffset(Offset offset) {
     return toClientFuture(committer.commitOffset(offset));
   }
+
+  @Override
+  public void waitUntilEmpty() throws CheckedApiException {
+    committer.waitUntilEmpty();
+  }
 }

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/wire/CommitState.java

@@ -59,15 +59,13 @@ ApiFuture<Void> addCommit(Offset offset) {
 
   void complete(long numComplete) throws CheckedApiException {
     if (numComplete > currentConnectionFutures.size()) {
-      CheckedApiException error =
-          new CheckedApiException(
-              String.format(
-                  "Received %s completions, which is more than the commits outstanding for this"
-                      + " stream.",
-                  numComplete),
-              Code.FAILED_PRECONDITION);
-      abort(error);
-      throw error;
+      // Note: Throw here to permanently shut down CommitterImpl, which will later call abort().
+      throw new CheckedApiException(
+          String.format(
+              "Received %s completions, which is more than the commits outstanding for this"
+                  + " stream.",
+              numComplete),
+          Code.FAILED_PRECONDITION);
     }
     while (!pastConnectionFutures.isEmpty()) {
       // Past futures refer to commits sent chronologically before the current stream, and thus they

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/wire/Committer.java

@@ -19,8 +19,13 @@
 import com.google.api.core.ApiFuture;
 import com.google.api.core.ApiService;
 import com.google.cloud.pubsublite.Offset;
+import com.google.cloud.pubsublite.internal.CheckedApiException;
 
 public interface Committer extends ApiService {
   // Commit a given offset. Clean shutdown waits for all outstanding commits to complete.
   ApiFuture<Void> commitOffset(Offset offset);
+
+  // Waits until all commits have been sent and acknowledged by the server. Throws an exception if
+  // the committer shut down due to a permanent error.
+  void waitUntilEmpty() throws CheckedApiException;
 }

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/wire/CommitterImpl.java

@@ -46,7 +46,7 @@ public class CommitterImpl extends ProxyService
       new Guard(monitor.monitor) {
         public boolean isSatisfied() {
           // Wait until the state is empty or a permanent error occurred.
-          return state.isEmpty() || hadPermanentError;
+          return state.isEmpty() || permanentError.isPresent();
         }
       };
 
@@ -57,7 +57,7 @@ public boolean isSatisfied() {
   private boolean shutdown = false;
 
   @GuardedBy("monitor.monitor")
-  private boolean hadPermanentError = false;
+  private Optional<CheckedApiException> permanentError = Optional.empty();
 
   @GuardedBy("monitor.monitor")
   private final CommitState state = new CommitState();
@@ -88,7 +88,7 @@ public CommitterImpl(CursorServiceClient client, InitialCommitCursorRequest requ
   @Override
   protected void handlePermanentError(CheckedApiException error) {
     try (CloseableMonitor.Hold h = monitor.enter()) {
-      hadPermanentError = true;
+      permanentError = Optional.of(error);
       shutdown = true;
       state.abort(error);
     }
@@ -144,4 +144,13 @@ public ApiFuture<Void> commitOffset(Offset offset) {
       return ApiFutures.immediateFailedFuture(e);
     }
   }
+
+  @Override
+  public void waitUntilEmpty() throws CheckedApiException {
+    try (CloseableMonitor.Hold h = monitor.enterWhenUninterruptibly(isEmptyOrError)) {
+      if (permanentError.isPresent()) {
+        throw permanentError.get();
+      }
+    }
+  }
 }

google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/wire/NextOffsetTracker.java

@@ -59,4 +59,9 @@ Optional<SeekRequest> requestForRestart() {
                 .setCursor(Cursor.newBuilder().setOffset(offset.value()))
                 .build());
   }
+
+  // Resets the offset tracker to its initial state.
+  void reset() {
+    nextOffset = Optional.empty();
+  }
 }