---

sduskis · sduskis · commit ff619f733834 · 2019-05-31T07:44:30.000-04:00
yaml --- r: 35375 b: refs/heads/pubsub-ordering-keys c: da6dd3c h: refs/heads/master i: 35373: cb6b5ac 35371: 3d4cec9 35367: 139bd26 35359: 425cbd7
diff --git a/[refs] b/[refs]
@@ -155,7 +155,7 @@ refs/tags/v0.72.0: a7703f2593ba312c0b2dde6fdfd4f5c764bb55ac
 refs/tags/v0.73.0: 21241ea8be9439cc5764c4944cdce21d34ce4f9e
 refs/tags/v0.74.0: 9d1f733dbbf790de7b494418523b69c4a9a57638
 refs/heads/ignoretest: 23c412ae07af3d0ab1caa2d44d5bc5c0ccb8b31d
-refs/heads/pubsub-ordering-keys: 30879687cce9936a6f7ee43af9110af2e2884625
+refs/heads/pubsub-ordering-keys: da6dd3ca83cab33f15f8a122c774487b325cdef7
 refs/tags/v0.75.0: c3673089ae09a897c1b4cf7dfe167fe4f8ab32fb
 refs/tags/v0.76.0: 395b016826d3ddf9cb8b34919636df15a4dbd032
 refs/tags/v0.77.0: 28a85a77883ccf5d48f297fd0ef3b3dca6ce01f0
diff --git a/branches/pubsub-ordering-keys/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/Publisher.java b/branches/pubsub-ordering-keys/google-cloud-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/Publisher.java
@@ -220,7 +220,6 @@ public ApiFuture<String> publish(PubsubMessage message) {
     List<OutstandingBatch> batchesToSend;
     messagesBatchLock.lock();
     try {
-      // Check if the next message makes the current batch exceed the max batch byte size.
       MessagesBatch messagesBatch = messagesBatches.get(orderingKey);
       if (messagesBatch == null) {
         messagesBatch = new MessagesBatch(batchingSettings, orderingKey);
@@ -232,23 +231,21 @@ public ApiFuture<String> publish(PubsubMessage message) {
         messagesBatches.remove(orderingKey);
       }
       setupAlarm();
+      if (!batchesToSend.isEmpty()) {
+        // TODO: if this is not an ordering keys scenario, will this do anything?
+        publishAllWithoutInflight();
+
+        // TODO: if this is an ordering keys scenario, is this safe without messagesBatchLock?
+        for (final OutstandingBatch batch : batchesToSend) {
+          logger.log(Level.FINER, "Scheduling a batch for immediate sending.");
+          publishOutstandingBatch(batch);
+        }
+      }
     } finally {
       messagesBatchLock.unlock();
     }
 
     messagesWaiter.incrementPendingMessages(1);
-
-    if (!batchesToSend.isEmpty()) {
-      // TODO: if this is not an ordering keys scenario, will this do anything?
-      publishAllWithoutInflight();
-
-      // TODO: if this is an ordering keys scenario, is this safe without messagesBatchLock?
-      for (final OutstandingBatch batch : batchesToSend) {
-        logger.log(Level.FINER, "Scheduling a batch for immediate sending.");
-        publishOutstandingBatch(batch);
-      }
-    }
-
     return outstandingPublish.publishResult;
   }
 
@@ -331,11 +328,6 @@ private void publishAllWithoutInflight() {
         if (batch.isEmpty()) {
           it.remove();
         } else if (key.isEmpty() || !sequentialExecutor.hasTasksInflight(key)) {
-          // TODO(kimkyung-goog): Do not release `messagesBatchLock` when publishing a batch. If
-          // it's released, the order of publishing cannot be guaranteed if `publish()` is called
-          // while this function is running. This locking mechanism needs to be improved if it
-          // causes any performance degradation.
-
           // TODO: Will this cause a performance problem for non-ordering keys scenarios?
           publishOutstandingBatch(batch.popOutstandingBatch());
           it.remove();
