Adding comments

sduskis · sduskis · commit 5a81613ddec3 · 2019-05-03T09:34:45.000-04:00
diff --git a/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/Publisher.java b/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/Publisher.java
@@ -142,6 +142,7 @@ private Publisher(Builder builder) throws IOException {
       retrySettingsBuilder.setMaxAttempts(Integer.MAX_VALUE);
     }
     if (enableMessageOrdering) {
+      // TODO: is there a way to have the default retry settings for requests without an ordering key?
       retrySettingsBuilder
           .setMaxAttempts(Integer.MAX_VALUE)
           .setTotalTimeout(Duration.ofNanos(Long.MAX_VALUE));
diff --git a/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/SequentialExecutorService.java b/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/SequentialExecutorService.java
@@ -223,7 +223,6 @@ public void onSuccess(T msg) {
                       // Step 5.2: on failure
                       @Override
                       public void onFailure(Throwable e) {
-                        keysWithErrors.add(key);
                         future.setException(e);
                         cancelQueuedTasks(key, CANCELLATION_EXCEPTION);
                       }
@@ -250,8 +249,7 @@ void resumePublish(String key) {
 
     /** Cancels every task in the queue associated with {@code key}. */
     private void cancelQueuedTasks(final String key, Throwable e) {
-      // TODO(kimkyung-goog): Ensure execute() fails once cancelQueueTasks() has been ever invoked,
-      // so that no more tasks are scheduled.
+      keysWithErrors.add(key);
       synchronized (tasksByKey) {
         final Queue<CancellableRunnable> tasks = tasksByKey.get(key);
         if (tasks != null) {

Original file line number	Diff line number	Diff line change
`@@ -142,6 +142,7 @@ private Publisher(Builder builder) throws IOException {`
`142`	`142`	`retrySettingsBuilder.setMaxAttempts(Integer.MAX_VALUE);`
`143`	`143`	`}`
`144`	`144`	`if (enableMessageOrdering) {`
	`145`	`+ // TODO: is there a way to have the default retry settings for requests without an ordering key?`
`145`	`146`	`retrySettingsBuilder`
`146`	`147`	`.setMaxAttempts(Integer.MAX_VALUE)`
`147`	`148`	`.setTotalTimeout(Duration.ofNanos(Long.MAX_VALUE));`