PR comments

pongad · pongad · commit 0aea0fc5fd02 · 2017-01-22T19:39:36.000+11:00
diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/spi/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/spi/v1/MessageDispatcher.java
@@ -114,7 +114,9 @@ public String toString() {
       for (AckHandler ah : ackHandlers) {
         ackIds.add(ah.ackId);
       }
-      return String.format("ExtensionJob {expiration: %s, nextExtensionSeconds: %d, ackIds: %s}", expiration, nextExtensionSeconds, ackIds);
+      return String.format(
+          "ExtensionJob {expiration: %s, nextExtensionSeconds: %d, ackIds: %s}",
+          expiration, nextExtensionSeconds, ackIds);
     }
   }
 
@@ -138,7 +140,9 @@ public void addAckId(String ackId) {
     }
 
     public String toString() {
-      return String.format("extend %d sec: %s", deadlineExtensionSeconds, ackIds);
+      return String.format(
+          "PendingModifyAckDeadline{extension: %d sec, ackIds: %s}",
+          deadlineExtensionSeconds, ackIds);
     }
   }
 
@@ -284,6 +288,11 @@ public void run() {
           });
     }
 
+    // There is a race condition. setupNextAckDeadlineExtensionAlarm might set
+    // an alarm that fires before this block can run.
+    // The fix is to move setup below this block, but doing so aggravates another
+    // race condition.
+    // TODO(pongad): Fix both races.
     synchronized (outstandingAckHandlers) {
       outstandingAckHandlers.add(
           new ExtensionJob(expiration, INITIAL_ACK_DEADLINE_EXTENSION_SECONDS, ackHandlers));

Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,9 @@ public String toString() {`
`114`	`114`	`for (AckHandler ah : ackHandlers) {`
`115`	`115`	`ackIds.add(ah.ackId);`
`116`	`116`	`}`
`117`		`- return String.format("ExtensionJob {expiration: %s, nextExtensionSeconds: %d, ackIds: %s}", expiration, nextExtensionSeconds, ackIds);`
	`117`	`+ return String.format(`
	`118`	`+ "ExtensionJob {expiration: %s, nextExtensionSeconds: %d, ackIds: %s}",`
	`119`	`+ expiration, nextExtensionSeconds, ackIds);`
`118`	`120`	`}`
`119`	`121`	`}`
`120`	`122`
`@@ -138,7 +140,9 @@ public void addAckId(String ackId) {`
`138`	`140`	`}`
`139`	`141`
`140`	`142`	`public String toString() {`
`141`		`- return String.format("extend %d sec: %s", deadlineExtensionSeconds, ackIds);`
	`143`	`+ return String.format(`
	`144`	`+ "PendingModifyAckDeadline{extension: %d sec, ackIds: %s}",`
	`145`	`+ deadlineExtensionSeconds, ackIds);`
`142`	`146`	`}`
`143`	`147`	`}`
`144`	`148`
`@@ -284,6 +288,11 @@ public void run() {`
`284`	`288`	`});`
`285`	`289`	`}`
`286`	`290`
	`291`	`+ // There is a race condition. setupNextAckDeadlineExtensionAlarm might set`
	`292`	`+ // an alarm that fires before this block can run.`
	`293`	`+ // The fix is to move setup below this block, but doing so aggravates another`
	`294`	`+ // race condition.`
	`295`	`+ // TODO(pongad): Fix both races.`
`287`	`296`	`synchronized (outstandingAckHandlers) {`
`288`	`297`	`outstandingAckHandlers.add(`
`289`	`298`	`new ExtensionJob(expiration, INITIAL_ACK_DEADLINE_EXTENSION_SECONDS, ackHandlers));`