From 76539e898f5c4e9462d0b1501c7c98c0cc628214 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 27 Mar 2023 16:01:29 +0000 Subject: [PATCH 01/67] receipt-modack for exactly once --- .../cloud/pubsub/v1/MessageDispatcher.java | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 3c772819a..e05d9f44f 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -372,13 +372,35 @@ void processReceivedMessages(List messages) { // totally expire so that pubsub service sends us the message again. continue; } - outstandingBatch.add(new OutstandingMessage(message, ackHandler)); - pendingReceipts.add(ackRequestData); + if (this.exactlyOnceDeliveryEnabled.get()) { + pendingReceipts.add(ackRequestData); + List modackRequestData = new ArrayList(); + List ackRequestDataReceipts = new ArrayList(); + pendingReceipts.drainTo(ackRequestDataReceipts); + if (!ackRequestDataReceipts.isEmpty()) { + modackRequestData.add( + new ModackRequestData(this.getMessageDeadlineSeconds(), ackRequestDataReceipts)); + } + ackProcessor.sendModackOperations(modackRequestData); + ApiFuture.addCallback(ackReqData.messageFuture, new ApiFutureCallback() { + @Override + public void onFailure(Throwable throwable) { + System.out.println("Error with receipt modack"); + } + @Override + public void onSuccess() { + outstandingBatch.add(new OutstandingMessage(message, ackHandler)); + } + }, MoreExecutors.directExecutor()); + } else { + outstandingBatch.add(new OutstandingMessage(message, ackHandler)); + pendingReceipts.add(ackRequestData); + } } - processBatch(outstandingBatch); } + private void processBatch(List batch) { messagesWaiter.incrementPendingCount(batch.size()); for (OutstandingMessage message : batch) { From a06a4b7b8f3b099cf7fb76e3a6eb16fd424bca61 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 27 Mar 2023 16:55:42 +0000 Subject: [PATCH 02/67] changing setup --- .../cloud/pubsub/v1/MessageDispatcher.java | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index e05d9f44f..7a67320ee 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -374,29 +374,31 @@ void processReceivedMessages(List messages) { } if (this.exactlyOnceDeliveryEnabled.get()) { pendingReceipts.add(ackRequestData); - List modackRequestData = new ArrayList(); - List ackRequestDataReceipts = new ArrayList(); - pendingReceipts.drainTo(ackRequestDataReceipts); - if (!ackRequestDataReceipts.isEmpty()) { - modackRequestData.add( - new ModackRequestData(this.getMessageDeadlineSeconds(), ackRequestDataReceipts)); - } - ackProcessor.sendModackOperations(modackRequestData); - ApiFuture.addCallback(ackReqData.messageFuture, new ApiFutureCallback() { - @Override - public void onFailure(Throwable throwable) { - System.out.println("Error with receipt modack"); - } - @Override - public void onSuccess() { - outstandingBatch.add(new OutstandingMessage(message, ackHandler)); - } - }, MoreExecutors.directExecutor()); } else { outstandingBatch.add(new OutstandingMessage(message, ackHandler)); pendingReceipts.add(ackRequestData); } } + if (this.exactlyOnceDeliveryEnabled.get()) { + List modackRequestData = new ArrayList(); + List ackRequestDataReceipts = new ArrayList(); + pendingReceipts.drainTo(ackRequestDataReceipts); + if (!ackRequestDataReceipts.isEmpty()) { + modackRequestData.add( + new ModackRequestData(this.getMessageDeadlineSeconds(), ackRequestDataReceipts)); + } + ackProcessor.sendModackOperations(modackRequestData); + ApiFuture.addCallback(ackReqData.messageFuture, new ApiFutureCallback() { + @Override + public void onFailure(Throwable throwable) { + System.out.println("Error with receipt modack"); + } + @Override + public void onSuccess() { + outstandingBatch.add(new OutstandingMessage(message, ackHandler)); + } + }, MoreExecutors.directExecutor()); + } processBatch(outstandingBatch); } From d8a17797b940010428c352f51ae427eb1ff30f74 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 3 Apr 2023 13:30:01 +0000 Subject: [PATCH 03/67] changing the pendingReceipt List --- .../cloud/pubsub/v1/MessageDispatcher.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 7a67320ee..b9b40c92a 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -89,6 +89,7 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingAcks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); + private final LinkedBlockingQueue exactlyOncePendingReceipts = new LinkedBlockingQueue<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); @@ -373,7 +374,7 @@ void processReceivedMessages(List messages) { continue; } if (this.exactlyOnceDeliveryEnabled.get()) { - pendingReceipts.add(ackRequestData); + exactlyOncePendingReceipts.add(ackRequestData); } else { outstandingBatch.add(new OutstandingMessage(message, ackHandler)); pendingReceipts.add(ackRequestData); @@ -382,7 +383,7 @@ void processReceivedMessages(List messages) { if (this.exactlyOnceDeliveryEnabled.get()) { List modackRequestData = new ArrayList(); List ackRequestDataReceipts = new ArrayList(); - pendingReceipts.drainTo(ackRequestDataReceipts); + exactlyOncePendingReceipts.drainTo(ackRequestDataReceipts); if (!ackRequestDataReceipts.isEmpty()) { modackRequestData.add( new ModackRequestData(this.getMessageDeadlineSeconds(), ackRequestDataReceipts)); @@ -395,11 +396,20 @@ public void onFailure(Throwable throwable) { } @Override public void onSuccess() { - outstandingBatch.add(new OutstandingMessage(message, ackHandler)); + // outstandingBatch.add(new OutstandingMessage(message, ackHandler)); + // processBatch(outstandingBatch); + try { + flowController.reserve(1, message.receivedMessage.getMessage().getSerializedSize()); + } catch (FlowControlException unexpectedException) { + // This should be a blocking flow controller and never throw an exception. + throw new IllegalStateException("Flow control unexpected exception", unexpectedException); + } + processOutstandingMessage(addDeliveryInfoCount(message.receivedMessage), message.ackHandler); } }, MoreExecutors.directExecutor()); + } else { + processBatch(outstandingBatch); } - processBatch(outstandingBatch); } From 680ac2d89507a2b9f33530c8181d1a5bdfaea065 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 16:06:32 +0000 Subject: [PATCH 04/67] using scheduled fixed rate --- .../cloud/pubsub/v1/MessageDispatcher.java | 33 ++----------------- 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index b9b40c92a..2dd8a9e44 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -380,36 +380,7 @@ void processReceivedMessages(List messages) { pendingReceipts.add(ackRequestData); } } - if (this.exactlyOnceDeliveryEnabled.get()) { - List modackRequestData = new ArrayList(); - List ackRequestDataReceipts = new ArrayList(); - exactlyOncePendingReceipts.drainTo(ackRequestDataReceipts); - if (!ackRequestDataReceipts.isEmpty()) { - modackRequestData.add( - new ModackRequestData(this.getMessageDeadlineSeconds(), ackRequestDataReceipts)); - } - ackProcessor.sendModackOperations(modackRequestData); - ApiFuture.addCallback(ackReqData.messageFuture, new ApiFutureCallback() { - @Override - public void onFailure(Throwable throwable) { - System.out.println("Error with receipt modack"); - } - @Override - public void onSuccess() { - // outstandingBatch.add(new OutstandingMessage(message, ackHandler)); - // processBatch(outstandingBatch); - try { - flowController.reserve(1, message.receivedMessage.getMessage().getSerializedSize()); - } catch (FlowControlException unexpectedException) { - // This should be a blocking flow controller and never throw an exception. - throw new IllegalStateException("Flow control unexpected exception", unexpectedException); - } - processOutstandingMessage(addDeliveryInfoCount(message.receivedMessage), message.ackHandler); - } - }, MoreExecutors.directExecutor()); - } else { - processBatch(outstandingBatch); - } + processBatch(outstandingBatch); } @@ -566,6 +537,8 @@ void processOutstandingOperations() { List ackRequestDataReceipts = new ArrayList(); pendingReceipts.drainTo(ackRequestDataReceipts); + exactlyOncePendingReceipts.drainTo(ackRequestDataReceipts); + if (!ackRequestDataReceipts.isEmpty()) { modackRequestData.add( new ModackRequestData(this.getMessageDeadlineSeconds(), ackRequestDataReceipts)); From ec652a67fdf0fc4fea334e28808670d21f0a1ff6 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 20:21:01 +0000 Subject: [PATCH 05/67] using blocked queues --- .../cloud/pubsub/v1/MessageDispatcher.java | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 2dd8a9e44..351836ea0 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -91,6 +91,8 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue exactlyOncePendingReceipts = new LinkedBlockingQueue<>(); + private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); + private final LinkedBlockingQueue exactlyOnceOutstandingBatch = new LinkedBlockingQueue<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -106,6 +108,10 @@ public enum AckReply { NACK } + private class OutstandingExactlyOnceBatch { + + } + /** Handles callbacks for acking/nacking messages from the {@link MessageReceiver}. */ private class AckHandler implements ApiFutureCallback { private final AckRequestData ackRequestData; @@ -262,6 +268,15 @@ public void run() { TimeUnit.SECONDS); } processOutstandingOperations(); + List outstandingBatch = new ArrayList<>(); + if(exactlyOnceOutstandingBatch.peek().isMessageReadyForDelivery && + (exactlyOnceOutstandingBatch.peek().receivedMessage.getAckId() == + exactlyOncePendingBatch.peek().receivedMessage.getAckId())){ + outstandingBatch.add(exactlyOnceOutstandingBatch.peek()); + exactlyOncePendingBatch.poll(); + exactlyOnceOutstandingBatch.poll(); + } + processBatch(outstandingBatch); } catch (Throwable t) { // Catch everything so that one run failing doesn't prevent subsequent runs. logger.log(Level.WARNING, "failed to run periodic job", t); @@ -345,6 +360,8 @@ private static class OutstandingMessage { private final ReceivedMessage receivedMessage; private final AckHandler ackHandler; + private boolean isMessageReadyForDelivery; + private OutstandingMessage(ReceivedMessage receivedMessage, AckHandler ackHandler) { this.receivedMessage = receivedMessage; this.ackHandler = ackHandler; @@ -373,10 +390,27 @@ void processReceivedMessages(List messages) { // totally expire so that pubsub service sends us the message again. continue; } + OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { - exactlyOncePendingReceipts.add(ackRequestData); + ApiFutureCallback callback = new ApiFutureCallback() { + @Override + public void onFailure(Throwable throwable) { + System.out.println("Error with receipt modack"); + } + + @Override + public void onSuccess(AckResponse ackResponse){ + if(ackResponse == AckResponse.SUCCESSFUL){ + outstandingMessage.isMessageReadyForDelivery = true; + exactlyOnceOutstandingBatch.add(outstandingMessage); + } + } + }; + exactlyOncePendingBatch.add(outstandingMessage); + ApiFutures.addCallback(ackRequestData.getMessageFutureIfExists(), callback, MoreExecutors.directExecutor()); + pendingReceipts.add(ackRequestData); } else { - outstandingBatch.add(new OutstandingMessage(message, ackHandler)); + outstandingBatch.add(outstandingMessage); pendingReceipts.add(ackRequestData); } } @@ -524,7 +558,9 @@ void extendDeadlines() { @InternalApi void processOutstandingOperations() { + List modackRequestData = new ArrayList(); + List exactlyOnceModackRequestData = new ArrayList(); // Nacks are modacks with an expiration of 0 List nackRequestDataList = new ArrayList(); @@ -537,8 +573,6 @@ void processOutstandingOperations() { List ackRequestDataReceipts = new ArrayList(); pendingReceipts.drainTo(ackRequestDataReceipts); - exactlyOncePendingReceipts.drainTo(ackRequestDataReceipts); - if (!ackRequestDataReceipts.isEmpty()) { modackRequestData.add( new ModackRequestData(this.getMessageDeadlineSeconds(), ackRequestDataReceipts)); From 4236363fe95263785d738a6fd583a16dae287f42 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 20:27:33 +0000 Subject: [PATCH 06/67] using blocked queues --- .../cloud/pubsub/v1/MessageDispatcher.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 351836ea0..1b4590e25 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -89,8 +89,6 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingAcks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); - private final LinkedBlockingQueue exactlyOncePendingReceipts = new LinkedBlockingQueue<>(); - private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue exactlyOnceOutstandingBatch = new LinkedBlockingQueue<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); @@ -108,10 +106,6 @@ public enum AckReply { NACK } - private class OutstandingExactlyOnceBatch { - - } - /** Handles callbacks for acking/nacking messages from the {@link MessageReceiver}. */ private class AckHandler implements ApiFutureCallback { private final AckRequestData ackRequestData; @@ -269,7 +263,7 @@ public void run() { } processOutstandingOperations(); List outstandingBatch = new ArrayList<>(); - if(exactlyOnceOutstandingBatch.peek().isMessageReadyForDelivery && + if(exactlyOnceOutstandingBatch.peek().getIsMessageReadyForDelivery && (exactlyOnceOutstandingBatch.peek().receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId())){ outstandingBatch.add(exactlyOnceOutstandingBatch.peek()); @@ -366,6 +360,14 @@ private OutstandingMessage(ReceivedMessage receivedMessage, AckHandler ackHandle this.receivedMessage = receivedMessage; this.ackHandler = ackHandler; } + + private boolean getIsMessageReadyForDelivery(){ + return this.isMessageReadyForDelivery; + } + + private void setIsMessageReadyForDelivery(boolean isReady){ + this.isMessageReadyForDelivery = isReady; + } } void processReceivedMessages(List messages) { @@ -401,7 +403,7 @@ public void onFailure(Throwable throwable) { @Override public void onSuccess(AckResponse ackResponse){ if(ackResponse == AckResponse.SUCCESSFUL){ - outstandingMessage.isMessageReadyForDelivery = true; + outstandingMessage.setIsMessageReadyForDelivery(true); exactlyOnceOutstandingBatch.add(outstandingMessage); } } From 88dfc84daaad8428d4ecb678a92d90e531e89add Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 20:33:18 +0000 Subject: [PATCH 07/67] using blocked queues --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 1b4590e25..c353342e0 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -263,7 +263,7 @@ public void run() { } processOutstandingOperations(); List outstandingBatch = new ArrayList<>(); - if(exactlyOnceOutstandingBatch.peek().getIsMessageReadyForDelivery && + if(exactlyOnceOutstandingBatch.peek().getIsMessageReadyForDelivery() && (exactlyOnceOutstandingBatch.peek().receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId())){ outstandingBatch.add(exactlyOnceOutstandingBatch.peek()); @@ -397,7 +397,9 @@ void processReceivedMessages(List messages) { ApiFutureCallback callback = new ApiFutureCallback() { @Override public void onFailure(Throwable throwable) { - System.out.println("Error with receipt modack"); + logger.log( + Level.WARNING, + "Pending Receipt Processing Failed"); } @Override From 1741342e6008fa520d38c842e74c774b6152ab6c Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 20:39:12 +0000 Subject: [PATCH 08/67] adding null safety --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index c353342e0..235b14fc2 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -263,7 +263,8 @@ public void run() { } processOutstandingOperations(); List outstandingBatch = new ArrayList<>(); - if(exactlyOnceOutstandingBatch.peek().getIsMessageReadyForDelivery() && + if(!exactlyOnceOutstandingBatch.isEmpty() && !exactlyOncePendingBatch.isEmpty() && + exactlyOnceOutstandingBatch.peek().getIsMessageReadyForDelivery() && (exactlyOnceOutstandingBatch.peek().receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId())){ outstandingBatch.add(exactlyOnceOutstandingBatch.peek()); @@ -564,7 +565,6 @@ void extendDeadlines() { void processOutstandingOperations() { List modackRequestData = new ArrayList(); - List exactlyOnceModackRequestData = new ArrayList(); // Nacks are modacks with an expiration of 0 List nackRequestDataList = new ArrayList(); From b5d9706b47fdb4ba2d828883ab7c999cfaeba22a Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 20:51:24 +0000 Subject: [PATCH 09/67] adding null safety --- .../cloud/pubsub/v1/MessageDispatcher.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 235b14fc2..daa610b02 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -90,7 +90,7 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); - private final LinkedBlockingQueue exactlyOnceOutstandingBatch = new LinkedBlockingQueue<>(); + private final List exactlyOnceOutstandingBatch = new ArrayList<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -263,13 +263,14 @@ public void run() { } processOutstandingOperations(); List outstandingBatch = new ArrayList<>(); - if(!exactlyOnceOutstandingBatch.isEmpty() && !exactlyOncePendingBatch.isEmpty() && - exactlyOnceOutstandingBatch.peek().getIsMessageReadyForDelivery() && - (exactlyOnceOutstandingBatch.peek().receivedMessage.getAckId() == - exactlyOncePendingBatch.peek().receivedMessage.getAckId())){ - outstandingBatch.add(exactlyOnceOutstandingBatch.peek()); - exactlyOncePendingBatch.poll(); - exactlyOnceOutstandingBatch.poll(); + for(OutstandingMessage message: exactlyOnceOutstandingBatch){ + if(!exactlyOncePendingBatch.isEmpty() && + message.getIsMessageReadyForDelivery() && + message.receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId()){ + outstandingBatch.add(message); + exactlyOncePendingBatch.poll(); + exactlyOnceOutstandingBatch.remove(message); + } } processBatch(outstandingBatch); } catch (Throwable t) { From baa470bcb5f7fe25fb9a34fcc5a402ba803a7e69 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 21:31:22 +0000 Subject: [PATCH 10/67] removing list --- .../google/cloud/pubsub/v1/MessageDispatcher.java | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index daa610b02..398a3674e 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -90,7 +90,6 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); - private final List exactlyOnceOutstandingBatch = new ArrayList<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -263,14 +262,9 @@ public void run() { } processOutstandingOperations(); List outstandingBatch = new ArrayList<>(); - for(OutstandingMessage message: exactlyOnceOutstandingBatch){ - if(!exactlyOncePendingBatch.isEmpty() && - message.getIsMessageReadyForDelivery() && - message.receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId()){ - outstandingBatch.add(message); - exactlyOncePendingBatch.poll(); - exactlyOnceOutstandingBatch.remove(message); - } + while(!exactlyOncePendingBatch.isEmpty() + && exactlyOncePendingBatch.peek().getIsMessageReadyForDelivery()){ + outstandingBatch.add(exactlyOncePendingBatch.poll()); } processBatch(outstandingBatch); } catch (Throwable t) { @@ -408,7 +402,6 @@ public void onFailure(Throwable throwable) { public void onSuccess(AckResponse ackResponse){ if(ackResponse == AckResponse.SUCCESSFUL){ outstandingMessage.setIsMessageReadyForDelivery(true); - exactlyOnceOutstandingBatch.add(outstandingMessage); } } }; From d7adf962f94c0b570a26a77c6ad49d5dc85ec423 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 21:38:11 +0000 Subject: [PATCH 11/67] adding list back --- .../cloud/pubsub/v1/MessageDispatcher.java | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 398a3674e..d35e6be30 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -90,6 +90,7 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); + private final List exactlyOnceOutstandingBatch = new ArrayList<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -262,9 +263,13 @@ public void run() { } processOutstandingOperations(); List outstandingBatch = new ArrayList<>(); - while(!exactlyOncePendingBatch.isEmpty() - && exactlyOncePendingBatch.peek().getIsMessageReadyForDelivery()){ - outstandingBatch.add(exactlyOncePendingBatch.poll()); + for(OutstandingMessage message: exactlyOnceOutstandingBatch){ + if(!exactlyOncePendingBatch.isEmpty() && + message.receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId()){ + outstandingBatch.add(message); + exactlyOncePendingBatch.poll(); + exactlyOnceOutstandingBatch.remove(message); + } } processBatch(outstandingBatch); } catch (Throwable t) { @@ -350,20 +355,11 @@ private static class OutstandingMessage { private final ReceivedMessage receivedMessage; private final AckHandler ackHandler; - private boolean isMessageReadyForDelivery; - private OutstandingMessage(ReceivedMessage receivedMessage, AckHandler ackHandler) { this.receivedMessage = receivedMessage; this.ackHandler = ackHandler; } - private boolean getIsMessageReadyForDelivery(){ - return this.isMessageReadyForDelivery; - } - - private void setIsMessageReadyForDelivery(boolean isReady){ - this.isMessageReadyForDelivery = isReady; - } } void processReceivedMessages(List messages) { @@ -401,7 +397,7 @@ public void onFailure(Throwable throwable) { @Override public void onSuccess(AckResponse ackResponse){ if(ackResponse == AckResponse.SUCCESSFUL){ - outstandingMessage.setIsMessageReadyForDelivery(true); + exactlyOnceOutstandingBatch.add(outstandingMessage); } } }; From 349d0c44141c7166c384e5af35ceeec00a40dd85 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 18 Apr 2023 21:47:54 +0000 Subject: [PATCH 12/67] if permanent failure, remove outstandingmsg from queue --- .../main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 1 + 1 file changed, 1 insertion(+) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index d35e6be30..444c79ec5 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -392,6 +392,7 @@ public void onFailure(Throwable throwable) { logger.log( Level.WARNING, "Pending Receipt Processing Failed"); + exactlyOncePendingBatch.remove(outstandingMessage); } @Override From 3f877d5d0995d04c955d4e7d349d13f2e6e3008c Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 19 May 2023 16:10:56 +0000 Subject: [PATCH 13/67] adding snippet of test --- .../com/google/cloud/pubsub/v1/MessageDispatcher.java | 7 ++++--- .../google/cloud/pubsub/v1/MessageDispatcherTest.java | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 444c79ec5..b4318a4f4 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -399,16 +399,17 @@ public void onFailure(Throwable throwable) { public void onSuccess(AckResponse ackResponse){ if(ackResponse == AckResponse.SUCCESSFUL){ exactlyOnceOutstandingBatch.add(outstandingMessage); + } else { + exactlyOncePendingBatch.remove(outstandingMessage); } } }; exactlyOncePendingBatch.add(outstandingMessage); - ApiFutures.addCallback(ackRequestData.getMessageFutureIfExists(), callback, MoreExecutors.directExecutor()); - pendingReceipts.add(ackRequestData); + ApiFutures.addCallback(ackRequestData.getMessageFutureIfExists(), callback, MoreExecutors.directExecutor(); } else { outstandingBatch.add(outstandingMessage); - pendingReceipts.add(ackRequestData); } + pendingReceipts.add(ackRequestData); } processBatch(outstandingBatch); } diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 0b48e0991..63edee329 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -139,6 +139,16 @@ public void testReceiptMessageReceiver() { .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumer.class)); } + @Test + public void testBatchSizeofExactlyOnceDelivered() { + MessageReceiver mockMessageReceiver = mock(MessageReceiver.class); + MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiver); + List receivedMessageList = new ArrayList(); + Collections.addAll(receivedMessageList, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE); + messageDispatcher.processReceivedMessages(receivedMessageList); + + } + @Test public void testReceiptMessageReceiverWithAckResponse() { MessageReceiverWithAckResponse mockMessageReceiverWithAckResponse = From 1423c01f31ca5370664e89646a104f45c9202c45 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 19 May 2023 21:32:22 +0000 Subject: [PATCH 14/67] adding method to streaming subscriber --- .../cloud/pubsub/v1/MessageDispatcher.java | 52 ++++++++++++------- .../v1/StreamingSubscriberConnection.java | 1 + .../pubsub/v1/MessageDispatcherTest.java | 18 +++---- 3 files changed, 43 insertions(+), 28 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index b4318a4f4..24dfbc235 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -386,26 +386,26 @@ void processReceivedMessages(List messages) { } OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { - ApiFutureCallback callback = new ApiFutureCallback() { - @Override - public void onFailure(Throwable throwable) { - logger.log( - Level.WARNING, - "Pending Receipt Processing Failed"); - exactlyOncePendingBatch.remove(outstandingMessage); - } - - @Override - public void onSuccess(AckResponse ackResponse){ - if(ackResponse == AckResponse.SUCCESSFUL){ - exactlyOnceOutstandingBatch.add(outstandingMessage); - } else { - exactlyOncePendingBatch.remove(outstandingMessage); - } - } - }; + // ApiFutureCallback callback = new ApiFutureCallback() { + // @Override + // public void onFailure(Throwable throwable) { + // logger.log( + // Level.WARNING, + // "Pending Receipt Processing Failed"); + // exactlyOncePendingBatch.remove(outstandingMessage); + // } + // + // @Override + // public void onSuccess(AckResponse ackResponse){ + // if(ackResponse == AckResponse.SUCCESSFUL){ + // exactlyOnceOutstandingBatch.add(outstandingMessage); + // } else { + // exactlyOncePendingBatch.remove(outstandingMessage); + // } + // } + // }; exactlyOncePendingBatch.add(outstandingMessage); - ApiFutures.addCallback(ackRequestData.getMessageFutureIfExists(), callback, MoreExecutors.directExecutor(); + // ApiFutures.addCallback(checkIfModackCompleted(), callback, MoreExecutors.directExecutor()); } else { outstandingBatch.add(outstandingMessage); } @@ -414,6 +414,20 @@ public void onSuccess(AckResponse ackResponse){ processBatch(outstandingBatch); } + void modackCompleted() { + OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); + exactlyOnceOutstandingBatch.add(outstandingMessage); + List outstandingBatch = new ArrayList<>(); + for(OutstandingMessage message: exactlyOnceOutstandingBatch){ + if(!exactlyOncePendingBatch.isEmpty() && + message.receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId()){ + outstandingBatch.add(message); + exactlyOncePendingBatch.poll(); + exactlyOnceOutstandingBatch.remove(message); + } + } + processBatch(outstandingBatch); + } private void processBatch(List batch) { messagesWaiter.incrementPendingCount(batch.size()); diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java index e7046c1be..54f0484b6 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java @@ -519,6 +519,7 @@ public void onSuccess(Empty empty) { for (AckRequestData ackRequestData : ackRequestDataList) { // This will check if a response is needed, and if it has already been set ackRequestData.setResponse(AckResponse.SUCCESSFUL, setResponseOnSuccess); + messageDispatcher.modackCompleted(); // Remove from our pending operations pendingRequests.remove(ackRequestData); } diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 63edee329..8c4334a58 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -139,15 +139,15 @@ public void testReceiptMessageReceiver() { .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumer.class)); } - @Test - public void testBatchSizeofExactlyOnceDelivered() { - MessageReceiver mockMessageReceiver = mock(MessageReceiver.class); - MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiver); - List receivedMessageList = new ArrayList(); - Collections.addAll(receivedMessageList, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE); - messageDispatcher.processReceivedMessages(receivedMessageList); - - } + // @Test + // public void testBatchSizeofExactlyOnceDelivered() { + // MessageReceiver mockMessageReceiver = mock(MessageReceiver.class); + // MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiver); + // List receivedMessageList = new ArrayList(); + // Collections.addAll(receivedMessageList, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE); + // messageDispatcher.processReceivedMessages(receivedMessageList); + // + // } @Test public void testReceiptMessageReceiverWithAckResponse() { From 2f8a40853b3c97b1febd96df0d70d96852059c00 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 19 May 2023 21:33:22 +0000 Subject: [PATCH 15/67] adding method to streaming subscriber --- .../cloud/pubsub/v1/MessageDispatcher.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 24dfbc235..0b588d48b 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -262,16 +262,16 @@ public void run() { TimeUnit.SECONDS); } processOutstandingOperations(); - List outstandingBatch = new ArrayList<>(); - for(OutstandingMessage message: exactlyOnceOutstandingBatch){ - if(!exactlyOncePendingBatch.isEmpty() && - message.receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId()){ - outstandingBatch.add(message); - exactlyOncePendingBatch.poll(); - exactlyOnceOutstandingBatch.remove(message); - } - } - processBatch(outstandingBatch); + // List outstandingBatch = new ArrayList<>(); + // for(OutstandingMessage message: exactlyOnceOutstandingBatch){ + // if(!exactlyOncePendingBatch.isEmpty() && + // message.receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId()){ + // outstandingBatch.add(message); + // exactlyOncePendingBatch.poll(); + // exactlyOnceOutstandingBatch.remove(message); + // } + // } + // processBatch(outstandingBatch); } catch (Throwable t) { // Catch everything so that one run failing doesn't prevent subsequent runs. logger.log(Level.WARNING, "failed to run periodic job", t); From aa35b977e055de0bb965b0476e875236f6ccf160 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 19 May 2023 22:27:52 +0000 Subject: [PATCH 16/67] adding notifyAcks --- .../cloud/pubsub/v1/MessageDispatcher.java | 63 +++++++++---------- .../v1/StreamingSubscriberConnection.java | 3 +- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 0b588d48b..23bd16754 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -28,6 +28,7 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.pubsub.v1.PubsubMessage; import com.google.pubsub.v1.ReceivedMessage; +import io.opencensus.trace.Link; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -89,8 +90,9 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingAcks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); - private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); + private final ConcurrentMap outstandingReceipts = new ConcurrentHashMap<>(); private final List exactlyOnceOutstandingBatch = new ArrayList<>(); + private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -373,7 +375,7 @@ void processReceivedMessages(List messages) { AckRequestData ackRequestData = builder.build(); AckHandler ackHandler = new AckHandler(ackRequestData, message.getMessage().getSerializedSize(), totalExpiration); - if (pendingMessages.putIfAbsent(message.getAckId(), ackHandler) != null) { + if (!this.exactlyOnceDeliveryEnabled.get() && pendingMessages.putIfAbsent(message.getAckId(), ackHandler) != null) { // putIfAbsent puts ackHandler if ackID isn't previously mapped, then return the // previously-mapped element. // If the previous element is not null, we already have the message and the new one is @@ -386,26 +388,8 @@ void processReceivedMessages(List messages) { } OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { - // ApiFutureCallback callback = new ApiFutureCallback() { - // @Override - // public void onFailure(Throwable throwable) { - // logger.log( - // Level.WARNING, - // "Pending Receipt Processing Failed"); - // exactlyOncePendingBatch.remove(outstandingMessage); - // } - // - // @Override - // public void onSuccess(AckResponse ackResponse){ - // if(ackResponse == AckResponse.SUCCESSFUL){ - // exactlyOnceOutstandingBatch.add(outstandingMessage); - // } else { - // exactlyOncePendingBatch.remove(outstandingMessage); - // } - // } - // }; exactlyOncePendingBatch.add(outstandingMessage); - // ApiFutures.addCallback(checkIfModackCompleted(), callback, MoreExecutors.directExecutor()); + outstandingReceipts.put(message.getAckId(), outstandingMessage); } else { outstandingBatch.add(outstandingMessage); } @@ -414,19 +398,34 @@ void processReceivedMessages(List messages) { processBatch(outstandingBatch); } - void modackCompleted() { - OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); - exactlyOnceOutstandingBatch.add(outstandingMessage); - List outstandingBatch = new ArrayList<>(); - for(OutstandingMessage message: exactlyOnceOutstandingBatch){ - if(!exactlyOncePendingBatch.isEmpty() && - message.receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId()){ - outstandingBatch.add(message); - exactlyOncePendingBatch.poll(); - exactlyOnceOutstandingBatch.remove(message); + void notifyAckSuccess(AckRequestData ackRequestData) { + + if(outstandingReceipts.containsKey(ackRequestData.getAckId())) { + OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()); + pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), + outstandingMessage.ackHandler); + + exactlyOnceOutstandingBatch.add(outstandingMessage); + List outstandingBatch = new ArrayList<>(); + for (OutstandingMessage message : exactlyOnceOutstandingBatch) { + if (!exactlyOncePendingBatch.isEmpty() && + message.receivedMessage.getAckId() + == exactlyOncePendingBatch.peek().receivedMessage.getAckId()) { + outstandingBatch.add(message); + exactlyOncePendingBatch.poll(); + exactlyOnceOutstandingBatch.remove(message); + } } + processBatch(outstandingBatch); + } + } + + void notifyAckFailed(AckRequestData ackRequestData) { + if(outstandingReceipts.containsKey(ackRequestData.getAckId())){ + outstandingReceipts.remove(ackRequestData); + OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()); + exactlyOncePendingBatch.remove(outstandingMessage); } - processBatch(outstandingBatch); } private void processBatch(List batch) { diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java index 54f0484b6..980d413f4 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java @@ -519,7 +519,7 @@ public void onSuccess(Empty empty) { for (AckRequestData ackRequestData : ackRequestDataList) { // This will check if a response is needed, and if it has already been set ackRequestData.setResponse(AckResponse.SUCCESSFUL, setResponseOnSuccess); - messageDispatcher.modackCompleted(); + messageDispatcher.notifyAckSuccess(ackRequestData); // Remove from our pending operations pendingRequests.remove(ackRequestData); } @@ -542,6 +542,7 @@ public void onFailure(Throwable t) { Map metadataMap = getMetadataMapFromThrowable(t); ackRequestDataList.forEach( ackRequestData -> { + messageDispatcher.notifyAckFailed(ackRequestData); String ackId = ackRequestData.getAckId(); if (metadataMap.containsKey(ackId)) { // An error occured From 71a90df1d393284bb6f05039dfb04706340b97b8 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Wed, 24 May 2023 22:09:45 +0000 Subject: [PATCH 17/67] changing notifyAckFailed calls --- .../google/cloud/pubsub/v1/StreamingSubscriberConnection.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java index 980d413f4..0470b8a8d 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/StreamingSubscriberConnection.java @@ -542,7 +542,6 @@ public void onFailure(Throwable t) { Map metadataMap = getMetadataMapFromThrowable(t); ackRequestDataList.forEach( ackRequestData -> { - messageDispatcher.notifyAckFailed(ackRequestData); String ackId = ackRequestData.getAckId(); if (metadataMap.containsKey(ackId)) { // An error occured @@ -558,12 +557,15 @@ public void onFailure(Throwable t) { "Permanent error invalid ack id message, will not resend", errorMessage); ackRequestData.setResponse(AckResponse.INVALID, setResponseOnSuccess); + messageDispatcher.notifyAckFailed(ackRequestData); } else { logger.log(Level.INFO, "Unknown error message, will not resend", errorMessage); ackRequestData.setResponse(AckResponse.OTHER, setResponseOnSuccess); + messageDispatcher.notifyAckFailed(ackRequestData); } } else { ackRequestData.setResponse(AckResponse.SUCCESSFUL, setResponseOnSuccess); + messageDispatcher.notifyAckSuccess(ackRequestData); } // Remove from our pending pendingRequests.remove(ackRequestData); From 4dfa1fddb38f1353f4edbbc7a9ec9018382064d2 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 30 May 2023 05:12:20 +0000 Subject: [PATCH 18/67] addressing some comments --- .../cloud/pubsub/v1/MessageDispatcher.java | 36 +++++++------------ 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 23bd16754..b50412734 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -264,16 +264,6 @@ public void run() { TimeUnit.SECONDS); } processOutstandingOperations(); - // List outstandingBatch = new ArrayList<>(); - // for(OutstandingMessage message: exactlyOnceOutstandingBatch){ - // if(!exactlyOncePendingBatch.isEmpty() && - // message.receivedMessage.getAckId() == exactlyOncePendingBatch.peek().receivedMessage.getAckId()){ - // outstandingBatch.add(message); - // exactlyOncePendingBatch.poll(); - // exactlyOnceOutstandingBatch.remove(message); - // } - // } - // processBatch(outstandingBatch); } catch (Throwable t) { // Catch everything so that one run failing doesn't prevent subsequent runs. logger.log(Level.WARNING, "failed to run periodic job", t); @@ -402,21 +392,21 @@ void notifyAckSuccess(AckRequestData ackRequestData) { if(outstandingReceipts.containsKey(ackRequestData.getAckId())) { OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()); - pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), - outstandingMessage.ackHandler); - - exactlyOnceOutstandingBatch.add(outstandingMessage); - List outstandingBatch = new ArrayList<>(); - for (OutstandingMessage message : exactlyOnceOutstandingBatch) { - if (!exactlyOncePendingBatch.isEmpty() && - message.receivedMessage.getAckId() - == exactlyOncePendingBatch.peek().receivedMessage.getAckId()) { - outstandingBatch.add(message); - exactlyOncePendingBatch.poll(); - exactlyOnceOutstandingBatch.remove(message); + if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), + outstandingMessage.ackHandler) == null) { + exactlyOnceOutstandingBatch.add(outstandingMessage); + List outstandingBatch = new ArrayList<>(); + for (OutstandingMessage message : exactlyOnceOutstandingBatch) { + if (!exactlyOncePendingBatch.isEmpty() && + message.receivedMessage.getAckId() + == exactlyOncePendingBatch.peek().receivedMessage.getAckId()) { + outstandingBatch.add(message); + exactlyOncePendingBatch.poll(); + exactlyOnceOutstandingBatch.remove(message); + } } + processBatch(outstandingBatch); } - processBatch(outstandingBatch); } } From a893dd6464be876350339671d78a93d33b4c73a5 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 30 May 2023 06:08:59 +0000 Subject: [PATCH 19/67] changed logic to use one datastructure --- .../cloud/pubsub/v1/MessageDispatcher.java | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index b50412734..4db495f6f 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -31,6 +31,7 @@ import io.opencensus.trace.Link; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -90,7 +91,7 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingAcks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); - private final ConcurrentMap outstandingReceipts = new ConcurrentHashMap<>(); + private final LinkedHashMap> outstandingReceipts = new LinkedHashMap>(); private final List exactlyOnceOutstandingBatch = new ArrayList<>(); private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); @@ -379,7 +380,7 @@ void processReceivedMessages(List messages) { OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { exactlyOncePendingBatch.add(outstandingMessage); - outstandingReceipts.put(message.getAckId(), outstandingMessage); + outstandingReceipts.put(message.getAckId(), new Pair<>(outstandingMessage, false)); } else { outstandingBatch.add(outstandingMessage); } @@ -391,22 +392,34 @@ void processReceivedMessages(List messages) { void notifyAckSuccess(AckRequestData ackRequestData) { if(outstandingReceipts.containsKey(ackRequestData.getAckId())) { - OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()); - if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), - outstandingMessage.ackHandler) == null) { - exactlyOnceOutstandingBatch.add(outstandingMessage); - List outstandingBatch = new ArrayList<>(); - for (OutstandingMessage message : exactlyOnceOutstandingBatch) { - if (!exactlyOncePendingBatch.isEmpty() && - message.receivedMessage.getAckId() - == exactlyOncePendingBatch.peek().receivedMessage.getAckId()) { - outstandingBatch.add(message); - exactlyOncePendingBatch.poll(); - exactlyOnceOutstandingBatch.remove(message); + OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()).get(0); + // if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { + // exactlyOnceOutstandingBatch.add(outstandingMessage); + // List outstandingBatch = new ArrayList<>(); + // for (OutstandingMessage message : exactlyOnceOutstandingBatch) { + // if (!exactlyOncePendingBatch.isEmpty() && + // message.receivedMessage.getAckId() + // == exactlyOncePendingBatch.peek().receivedMessage.getAckId()) { + // outstandingBatch.add(message); + // exactlyOncePendingBatch.poll(); + // exactlyOnceOutstandingBatch.remove(message); + // } + // } + // Setting to true means that the receipt is complete + outstandingReceipts.get(ackRequestData.getAckId()).set(1, true); + List completedReceipts = new ArrayList<>(); + if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { + for(Map.Entry> pr: outstandingReceipts.entrySet()){ + String ackId = pr.getKey(); + if(pr.getValue().get(1)){ + outstandingReceipts.remove(ackId); + completedReceipts.add(pr.getValue().get(0)); + } else { + break; } } - processBatch(outstandingBatch); } + processBatch(completedReceipts); } } From 3193b81d62c79a68dedae2dd06ec04259d8748dc Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 5 Jun 2023 01:57:34 +0000 Subject: [PATCH 20/67] fixing notifyFailed --- .../cloud/pubsub/v1/MessageDispatcher.java | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 4db495f6f..0b06b7843 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -93,7 +93,6 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); private final LinkedHashMap> outstandingReceipts = new LinkedHashMap>(); private final List exactlyOnceOutstandingBatch = new ArrayList<>(); - private final LinkedBlockingQueue exactlyOncePendingBatch = new LinkedBlockingQueue<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -379,7 +378,6 @@ void processReceivedMessages(List messages) { } OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { - exactlyOncePendingBatch.add(outstandingMessage); outstandingReceipts.put(message.getAckId(), new Pair<>(outstandingMessage, false)); } else { outstandingBatch.add(outstandingMessage); @@ -393,24 +391,13 @@ void notifyAckSuccess(AckRequestData ackRequestData) { if(outstandingReceipts.containsKey(ackRequestData.getAckId())) { OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()).get(0); - // if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { - // exactlyOnceOutstandingBatch.add(outstandingMessage); - // List outstandingBatch = new ArrayList<>(); - // for (OutstandingMessage message : exactlyOnceOutstandingBatch) { - // if (!exactlyOncePendingBatch.isEmpty() && - // message.receivedMessage.getAckId() - // == exactlyOncePendingBatch.peek().receivedMessage.getAckId()) { - // outstandingBatch.add(message); - // exactlyOncePendingBatch.poll(); - // exactlyOnceOutstandingBatch.remove(message); - // } - // } // Setting to true means that the receipt is complete outstandingReceipts.get(ackRequestData.getAckId()).set(1, true); List completedReceipts = new ArrayList<>(); if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { for(Map.Entry> pr: outstandingReceipts.entrySet()){ String ackId = pr.getKey(); + // If receipt is complete then add to completedReceipts to process the batch if(pr.getValue().get(1)){ outstandingReceipts.remove(ackId); completedReceipts.add(pr.getValue().get(0)); @@ -425,9 +412,7 @@ void notifyAckSuccess(AckRequestData ackRequestData) { void notifyAckFailed(AckRequestData ackRequestData) { if(outstandingReceipts.containsKey(ackRequestData.getAckId())){ - outstandingReceipts.remove(ackRequestData); - OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()); - exactlyOncePendingBatch.remove(outstandingMessage); + outstandingReceipts.remove(ackRequestData.getAckId()); } } From bb0c2a9b486b2cc762c6b29306d85371bb9e9ad1 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 5 Jun 2023 17:56:37 +0000 Subject: [PATCH 21/67] fixing notifyFailed --- .../com/google/cloud/pubsub/v1/MessageDispatcher.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 0b06b7843..6eb081443 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -50,6 +50,7 @@ import org.threeten.bp.Duration; import org.threeten.bp.Instant; import org.threeten.bp.temporal.ChronoUnit; +import com.google.common.base.Pair; /** * Dispatches messages to a message receiver while handling the messages acking and lease @@ -390,17 +391,18 @@ void processReceivedMessages(List messages) { void notifyAckSuccess(AckRequestData ackRequestData) { if(outstandingReceipts.containsKey(ackRequestData.getAckId())) { - OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()).get(0); + OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()).getFirst(); // Setting to true means that the receipt is complete - outstandingReceipts.get(ackRequestData.getAckId()).set(1, true); + Pair receiptInfo = Pair.of(1,true); + outstandingReceipts.get(ackRequestData.getAckId()).set(receiptInfo); List completedReceipts = new ArrayList<>(); if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { for(Map.Entry> pr: outstandingReceipts.entrySet()){ String ackId = pr.getKey(); // If receipt is complete then add to completedReceipts to process the batch - if(pr.getValue().get(1)){ + if(pr.getValue().getSecond()){ outstandingReceipts.remove(ackId); - completedReceipts.add(pr.getValue().get(0)); + completedReceipts.add(pr.getValue().getFirst()); } else { break; } From 60de4e1dafe273efa80b050c2f9ee6d17a868434 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 15 Jun 2023 18:49:42 +0000 Subject: [PATCH 22/67] changing Pair to custom class --- .../cloud/pubsub/v1/MessageDispatcher.java | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 6eb081443..bee298a2e 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -50,7 +50,6 @@ import org.threeten.bp.Duration; import org.threeten.bp.Instant; import org.threeten.bp.temporal.ChronoUnit; -import com.google.common.base.Pair; /** * Dispatches messages to a message receiver while handling the messages acking and lease @@ -92,7 +91,7 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingAcks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); - private final LinkedHashMap> outstandingReceipts = new LinkedHashMap>(); + private final LinkedHashMap outstandingReceipts = new LinkedHashMap(); private final List exactlyOnceOutstandingBatch = new ArrayList<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); @@ -355,6 +354,31 @@ private OutstandingMessage(ReceivedMessage receivedMessage, AckHandler ackHandle } + private static class ReceiptCompleteData{ + private OutstandingMessage outstandingMessage; + private Boolean receiptComplete; + + private ReceiptCompleteData(OutstandingMessage outstandingMessage, Boolean receiptComplete){ + this.outstandingMessage = outstandingMessage; + this.receiptComplete = receiptComplete; + } + + private OutstandingMessage getOutstandingMessage(){ + return this.outstandingMessage; + } + private Boolean getReceiptComplete(){ + return this.receiptComplete; + } + + private void setOutstandingMessage(OutstandingMessage outstandingMessage){ + this.outstandingMessage = outstandingMessage; + } + private void setReceiptComplete(Boolean receiptComplete){ + this.receiptComplete = receiptComplete; + } + + } + void processReceivedMessages(List messages) { Instant totalExpiration = now().plus(maxAckExtensionPeriod); List outstandingBatch = new ArrayList<>(messages.size()); @@ -379,7 +403,7 @@ void processReceivedMessages(List messages) { } OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { - outstandingReceipts.put(message.getAckId(), new Pair<>(outstandingMessage, false)); + outstandingReceipts.put(message.getAckId(), new ReceiptCompleteData(outstandingMessage, false)); } else { outstandingBatch.add(outstandingMessage); } @@ -391,18 +415,19 @@ void processReceivedMessages(List messages) { void notifyAckSuccess(AckRequestData ackRequestData) { if(outstandingReceipts.containsKey(ackRequestData.getAckId())) { - OutstandingMessage outstandingMessage = outstandingReceipts.get(ackRequestData.getAckId()).getFirst(); + ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); + OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); // Setting to true means that the receipt is complete - Pair receiptInfo = Pair.of(1,true); - outstandingReceipts.get(ackRequestData.getAckId()).set(receiptInfo); + receiptCompleteData.setReceiptComplete(true); + outstandingReceipts.put(ackRequestData.getAckId(), receiptCompleteData); List completedReceipts = new ArrayList<>(); if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { - for(Map.Entry> pr: outstandingReceipts.entrySet()){ - String ackId = pr.getKey(); + for(Map.Entry receipts: outstandingReceipts.entrySet()){ + String ackId = receipts.getKey(); // If receipt is complete then add to completedReceipts to process the batch - if(pr.getValue().getSecond()){ + if(receipts.getValue().getReceiptComplete()){ outstandingReceipts.remove(ackId); - completedReceipts.add(pr.getValue().getFirst()); + completedReceipts.add(receipts.getValue().getOutstandingMessage()); } else { break; } From 0d9d41b4381cfcc7cbb23df92c77809709c53432 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 15:11:42 +0000 Subject: [PATCH 23/67] removing the not needed data structure --- .../cloud/pubsub/v1/MessageDispatcher.java | 1 - .../pubsub/v1/MessageDispatcherTest.java | 29 +++++++++++++------ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index bee298a2e..fed165629 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -92,7 +92,6 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); private final LinkedHashMap outstandingReceipts = new LinkedHashMap(); - private final List exactlyOnceOutstandingBatch = new ArrayList<>(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 8c4334a58..4ab4eca2e 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -139,15 +139,26 @@ public void testReceiptMessageReceiver() { .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumer.class)); } - // @Test - // public void testBatchSizeofExactlyOnceDelivered() { - // MessageReceiver mockMessageReceiver = mock(MessageReceiver.class); - // MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiver); - // List receivedMessageList = new ArrayList(); - // Collections.addAll(receivedMessageList, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE); - // messageDispatcher.processReceivedMessages(receivedMessageList); - // - // } + @Test + public void testReceiptModackForExactlyOnceDelivered() { + MessageReceiverWithAckResponse mockMessageReceiverWithAckResponse = + mock(MessageReceiverWithAckResponse.class); + MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiverWithAckResponse); + messageDispatcher.setExactlyOnceDeliveryEnabled(true); + + messageDispatcher.processReceivedMessages(Collections.singletonList(TEST_MESSAGE)); + + messageDispatcher.processOutstandingOperations(); + verify(mockMessageReceiverWithAckResponse, never()) + .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); + + AckRequestData ackRequestData = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); + messageDispatcher.notifyAckSuccess(ackRequestData); + messageDispatcher.processOutstandingOperations(); + verify(mockMessageReceiverWithAckResponse, times(1)) + .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); + + } @Test public void testReceiptMessageReceiverWithAckResponse() { From 19c51db9d7c3a749f0a255d9873b6352525627aa Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 16:12:38 +0000 Subject: [PATCH 24/67] Fixing test --- .../pubsub/v1/MessageDispatcherTest.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 4ab4eca2e..8bb6309cc 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -155,8 +155,23 @@ public void testReceiptModackForExactlyOnceDelivered() { AckRequestData ackRequestData = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); messageDispatcher.notifyAckSuccess(ackRequestData); messageDispatcher.processOutstandingOperations(); - verify(mockMessageReceiverWithAckResponse, times(1)) - .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); + + List ackRequestDataList = new ArrayList(); + ackRequestDataList.add(ackRequestData); + List modackRequestDataList = new ArrayList(); + modackRequestDataList.add(new ModackRequestData(MIN_ACK_DEADLINE_SECONDS, ackRequestData)); + + verify(mockAckProcessor, times(1)) + .sendModackOperations( + argThat( + new CustomArgumentMatchers.ModackRequestDataListMatcher(modackRequestDataList))); + verify(mockAckProcessor, times(1)) + .sendAckOperations( + argThat(new CustomArgumentMatchers.AckRequestDataListMatcher(ackRequestDataList))); + + // + // verify(mockMessageReceiverWithAckResponse, times(1)) + // .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); } From ba5f30b55a534220fd13afe1b6d371bc509f5885 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 16:35:22 +0000 Subject: [PATCH 25/67] Fixing test --- .../pubsub/v1/MessageDispatcherTest.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 8bb6309cc..c707ad80e 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -141,15 +141,15 @@ public void testReceiptMessageReceiver() { @Test public void testReceiptModackForExactlyOnceDelivered() { - MessageReceiverWithAckResponse mockMessageReceiverWithAckResponse = - mock(MessageReceiverWithAckResponse.class); - MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiverWithAckResponse); + // MessageReceiverWithAckResponse mockMessageReceiverWithAckResponse = + // mock(MessageReceiverWithAckResponse.class); + MessageDispatcher messageDispatcher = getMessageDispatcher(messageReceiverWithAckResponse); messageDispatcher.setExactlyOnceDeliveryEnabled(true); messageDispatcher.processReceivedMessages(Collections.singletonList(TEST_MESSAGE)); messageDispatcher.processOutstandingOperations(); - verify(mockMessageReceiverWithAckResponse, never()) + verify(messageReceiverWithAckResponse, never()) .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); AckRequestData ackRequestData = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); @@ -161,17 +161,17 @@ public void testReceiptModackForExactlyOnceDelivered() { List modackRequestDataList = new ArrayList(); modackRequestDataList.add(new ModackRequestData(MIN_ACK_DEADLINE_SECONDS, ackRequestData)); - verify(mockAckProcessor, times(1)) - .sendModackOperations( - argThat( - new CustomArgumentMatchers.ModackRequestDataListMatcher(modackRequestDataList))); - verify(mockAckProcessor, times(1)) - .sendAckOperations( - argThat(new CustomArgumentMatchers.AckRequestDataListMatcher(ackRequestDataList))); + // verify(mockAckProcessor, times(1)) + // .sendModackOperations( + // argThat( + // new CustomArgumentMatchers.ModackRequestDataListMatcher(modackRequestDataList))); + // verify(mockAckProcessor, times(1)) + // .sendAckOperations( + // argThat(new CustomArgumentMatchers.AckRequestDataListMatcher(ackRequestDataList))); // - // verify(mockMessageReceiverWithAckResponse, times(1)) - // .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); + verify(messageReceiverWithAckResponse, times(1)) + .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); } From 9d808eba7600128042afa043732dc44e4d7aeb7c Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 16:43:20 +0000 Subject: [PATCH 26/67] Fixing test --- .../pubsub/v1/MessageDispatcherTest.java | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index c707ad80e..4129aaf8d 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -141,15 +141,16 @@ public void testReceiptMessageReceiver() { @Test public void testReceiptModackForExactlyOnceDelivered() { - // MessageReceiverWithAckResponse mockMessageReceiverWithAckResponse = - // mock(MessageReceiverWithAckResponse.class); - MessageDispatcher messageDispatcher = getMessageDispatcher(messageReceiverWithAckResponse); + + MessageReceiverWithAckResponse mockMessageReceiverWithAckResponse = + mock(MessageReceiverWithAckResponse.class); + MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiverWithAckResponse); messageDispatcher.setExactlyOnceDeliveryEnabled(true); messageDispatcher.processReceivedMessages(Collections.singletonList(TEST_MESSAGE)); messageDispatcher.processOutstandingOperations(); - verify(messageReceiverWithAckResponse, never()) + verify(mockMessageReceiverWithAckResponse, never()) .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); AckRequestData ackRequestData = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); @@ -161,17 +162,10 @@ public void testReceiptModackForExactlyOnceDelivered() { List modackRequestDataList = new ArrayList(); modackRequestDataList.add(new ModackRequestData(MIN_ACK_DEADLINE_SECONDS, ackRequestData)); - // verify(mockAckProcessor, times(1)) - // .sendModackOperations( - // argThat( - // new CustomArgumentMatchers.ModackRequestDataListMatcher(modackRequestDataList))); - // verify(mockAckProcessor, times(1)) - // .sendAckOperations( - // argThat(new CustomArgumentMatchers.AckRequestDataListMatcher(ackRequestDataList))); - - // - verify(messageReceiverWithAckResponse, times(1)) - .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); + verify(mockAckProcessor, times(1)) + .sendModackOperations( + argThat( + new CustomArgumentMatchers.ModackRequestDataListMatcher(modackRequestDataList))); } From 1a16d9aab237a5d7b6803cab6c5bf4a223591627 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 16:45:09 +0000 Subject: [PATCH 27/67] Fixing test --- .../java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 4129aaf8d..2b5d1c059 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -157,8 +157,6 @@ public void testReceiptModackForExactlyOnceDelivered() { messageDispatcher.notifyAckSuccess(ackRequestData); messageDispatcher.processOutstandingOperations(); - List ackRequestDataList = new ArrayList(); - ackRequestDataList.add(ackRequestData); List modackRequestDataList = new ArrayList(); modackRequestDataList.add(new ModackRequestData(MIN_ACK_DEADLINE_SECONDS, ackRequestData)); From e5718ab9d22c2de7cebacb9e30cf1913ea9a0b8e Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 16:51:59 +0000 Subject: [PATCH 28/67] fixing format --- .../cloud/pubsub/v1/MessageDispatcher.java | 38 ++++++++++--------- .../pubsub/v1/MessageDispatcherTest.java | 1 - 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index fed165629..7bf84f7ac 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -28,7 +28,6 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.pubsub.v1.PubsubMessage; import com.google.pubsub.v1.ReceivedMessage; -import io.opencensus.trace.Link; import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; @@ -91,7 +90,8 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingAcks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); - private final LinkedHashMap outstandingReceipts = new LinkedHashMap(); + private final LinkedHashMap outstandingReceipts = + new LinkedHashMap(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -350,32 +350,32 @@ private OutstandingMessage(ReceivedMessage receivedMessage, AckHandler ackHandle this.receivedMessage = receivedMessage; this.ackHandler = ackHandler; } - } - private static class ReceiptCompleteData{ + private static class ReceiptCompleteData { private OutstandingMessage outstandingMessage; private Boolean receiptComplete; - private ReceiptCompleteData(OutstandingMessage outstandingMessage, Boolean receiptComplete){ + private ReceiptCompleteData(OutstandingMessage outstandingMessage, Boolean receiptComplete) { this.outstandingMessage = outstandingMessage; this.receiptComplete = receiptComplete; } - private OutstandingMessage getOutstandingMessage(){ + private OutstandingMessage getOutstandingMessage() { return this.outstandingMessage; } - private Boolean getReceiptComplete(){ + + private Boolean getReceiptComplete() { return this.receiptComplete; } - private void setOutstandingMessage(OutstandingMessage outstandingMessage){ + private void setOutstandingMessage(OutstandingMessage outstandingMessage) { this.outstandingMessage = outstandingMessage; } - private void setReceiptComplete(Boolean receiptComplete){ + + private void setReceiptComplete(Boolean receiptComplete) { this.receiptComplete = receiptComplete; } - } void processReceivedMessages(List messages) { @@ -389,7 +389,8 @@ void processReceivedMessages(List messages) { AckRequestData ackRequestData = builder.build(); AckHandler ackHandler = new AckHandler(ackRequestData, message.getMessage().getSerializedSize(), totalExpiration); - if (!this.exactlyOnceDeliveryEnabled.get() && pendingMessages.putIfAbsent(message.getAckId(), ackHandler) != null) { + if (!this.exactlyOnceDeliveryEnabled.get() + && pendingMessages.putIfAbsent(message.getAckId(), ackHandler) != null) { // putIfAbsent puts ackHandler if ackID isn't previously mapped, then return the // previously-mapped element. // If the previous element is not null, we already have the message and the new one is @@ -402,7 +403,8 @@ void processReceivedMessages(List messages) { } OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { - outstandingReceipts.put(message.getAckId(), new ReceiptCompleteData(outstandingMessage, false)); + outstandingReceipts.put( + message.getAckId(), new ReceiptCompleteData(outstandingMessage, false)); } else { outstandingBatch.add(outstandingMessage); } @@ -413,18 +415,20 @@ void processReceivedMessages(List messages) { void notifyAckSuccess(AckRequestData ackRequestData) { - if(outstandingReceipts.containsKey(ackRequestData.getAckId())) { + if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); // Setting to true means that the receipt is complete receiptCompleteData.setReceiptComplete(true); outstandingReceipts.put(ackRequestData.getAckId(), receiptCompleteData); List completedReceipts = new ArrayList<>(); - if (pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { - for(Map.Entry receipts: outstandingReceipts.entrySet()){ + if (pendingMessages.putIfAbsent( + outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) + == null) { + for (Map.Entry receipts : outstandingReceipts.entrySet()) { String ackId = receipts.getKey(); // If receipt is complete then add to completedReceipts to process the batch - if(receipts.getValue().getReceiptComplete()){ + if (receipts.getValue().getReceiptComplete()) { outstandingReceipts.remove(ackId); completedReceipts.add(receipts.getValue().getOutstandingMessage()); } else { @@ -437,7 +441,7 @@ void notifyAckSuccess(AckRequestData ackRequestData) { } void notifyAckFailed(AckRequestData ackRequestData) { - if(outstandingReceipts.containsKey(ackRequestData.getAckId())){ + if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { outstandingReceipts.remove(ackRequestData.getAckId()); } } diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 2b5d1c059..44e469f32 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -164,7 +164,6 @@ public void testReceiptModackForExactlyOnceDelivered() { .sendModackOperations( argThat( new CustomArgumentMatchers.ModackRequestDataListMatcher(modackRequestDataList))); - } @Test From 3a520edaf6d137f198593b1398fbc73129824bf1 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 18:42:32 +0000 Subject: [PATCH 29/67] fixing test to call receiveMessage --- .../com/google/cloud/pubsub/v1/MessageDispatcherTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 44e469f32..2a8f9a8c0 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -160,10 +160,9 @@ public void testReceiptModackForExactlyOnceDelivered() { List modackRequestDataList = new ArrayList(); modackRequestDataList.add(new ModackRequestData(MIN_ACK_DEADLINE_SECONDS, ackRequestData)); - verify(mockAckProcessor, times(1)) - .sendModackOperations( - argThat( - new CustomArgumentMatchers.ModackRequestDataListMatcher(modackRequestDataList))); + // Need to change to test correct contents of the message - will need custom matcher + verify(mockMessageReceiverWithAckResponse, times(1)) + .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); } @Test From 6ca0337c8394e35cc8b08ff05678f57d4e50f3c4 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 19:12:00 +0000 Subject: [PATCH 30/67] testing test failure --- samples/snippets/src/test/java/pubsub/SubscriberIT.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/samples/snippets/src/test/java/pubsub/SubscriberIT.java b/samples/snippets/src/test/java/pubsub/SubscriberIT.java index ee4e03068..ca26ff00c 100644 --- a/samples/snippets/src/test/java/pubsub/SubscriberIT.java +++ b/samples/snippets/src/test/java/pubsub/SubscriberIT.java @@ -236,8 +236,10 @@ public void testSubscriberExactlyOnceDelivery() throws Exception { bout.reset(); SubscribeWithExactlyOnceConsumerWithResponseExample .subscribeWithExactlyOnceConsumerWithResponseExample(projectId, subscriptionEodId); + int i = 0; for (String messageId : messageIds) { - assertThat(bout.toString()).contains("Message successfully acked: " + messageId); + assertThat(bout.toString()).contains("Message received: " + i + "/n" + "Message successfully acked: " + messageId); + i++; } } } From 5553dedcb905784a66adad85f9cb1b229ba5540c Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 19:24:43 +0000 Subject: [PATCH 31/67] testing test failure --- samples/snippets/src/test/java/pubsub/SubscriberIT.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/samples/snippets/src/test/java/pubsub/SubscriberIT.java b/samples/snippets/src/test/java/pubsub/SubscriberIT.java index ca26ff00c..4a17892da 100644 --- a/samples/snippets/src/test/java/pubsub/SubscriberIT.java +++ b/samples/snippets/src/test/java/pubsub/SubscriberIT.java @@ -238,7 +238,8 @@ public void testSubscriberExactlyOnceDelivery() throws Exception { .subscribeWithExactlyOnceConsumerWithResponseExample(projectId, subscriptionEodId); int i = 0; for (String messageId : messageIds) { - assertThat(bout.toString()).contains("Message received: " + i + "/n" + "Message successfully acked: " + messageId); + assertThat(bout.toString()).contains("Message received: " + i + "/n" + + "Message successfully acked: " + messageId); i++; } } From 85bb43d1c784068d4b22b799e0fed88f181b779e Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 19:28:43 +0000 Subject: [PATCH 32/67] testing test failure --- samples/snippets/src/test/java/pubsub/SubscriberIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/snippets/src/test/java/pubsub/SubscriberIT.java b/samples/snippets/src/test/java/pubsub/SubscriberIT.java index 4a17892da..88be443bf 100644 --- a/samples/snippets/src/test/java/pubsub/SubscriberIT.java +++ b/samples/snippets/src/test/java/pubsub/SubscriberIT.java @@ -238,8 +238,8 @@ public void testSubscriberExactlyOnceDelivery() throws Exception { .subscribeWithExactlyOnceConsumerWithResponseExample(projectId, subscriptionEodId); int i = 0; for (String messageId : messageIds) { - assertThat(bout.toString()).contains("Message received: " + i + "/n" + - "Message successfully acked: " + messageId); + assertThat(bout.toString()).contains("Message received: " + i + "/n" + + "Message successfully acked: " + messageId); i++; } } From c8c4b637696f8fe3e11cb37b58bc744466a5af34 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 19:43:26 +0000 Subject: [PATCH 33/67] increasing timestamp to test --- .../test/java/com/google/cloud/pubsub/it/ITPubSubTest.java | 2 +- samples/snippets/src/test/java/pubsub/SubscriberIT.java | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java index 290b9927d..258e90473 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java @@ -55,7 +55,7 @@ public class ITPubSubTest { private static final int MAX_INBOUND_MESSAGE_SIZE = 20 * 1024 * 1024; - @Rule public Timeout globalTimeout = Timeout.seconds(300); + @Rule public Timeout globalTimeout = Timeout.seconds(1000); @AutoValue abstract static class MessageAndConsumer { diff --git a/samples/snippets/src/test/java/pubsub/SubscriberIT.java b/samples/snippets/src/test/java/pubsub/SubscriberIT.java index 88be443bf..0f99b788e 100644 --- a/samples/snippets/src/test/java/pubsub/SubscriberIT.java +++ b/samples/snippets/src/test/java/pubsub/SubscriberIT.java @@ -118,7 +118,7 @@ public interface CheckedRunnable { void run() throws Exception; } - @Rule public Timeout globalTimeout = Timeout.seconds(600); // 10 minute timeout + @Rule public Timeout globalTimeout = Timeout.seconds(1000); // 10 minute timeout @BeforeClass public static void checkRequirements() { @@ -236,11 +236,8 @@ public void testSubscriberExactlyOnceDelivery() throws Exception { bout.reset(); SubscribeWithExactlyOnceConsumerWithResponseExample .subscribeWithExactlyOnceConsumerWithResponseExample(projectId, subscriptionEodId); - int i = 0; for (String messageId : messageIds) { - assertThat(bout.toString()).contains("Message received: " + i + "/n" - + "Message successfully acked: " + messageId); - i++; + assertThat(bout.toString()).contains("Message successfully acked: " + messageId); } } } From aed52a16635d87fd22ff72eae9a70ae8654f11bf Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 26 Jun 2023 20:08:04 +0000 Subject: [PATCH 34/67] increasing timestamp to test --- samples/snippets/src/test/java/pubsub/SubscriberIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/snippets/src/test/java/pubsub/SubscriberIT.java b/samples/snippets/src/test/java/pubsub/SubscriberIT.java index 0f99b788e..8e670b4d0 100644 --- a/samples/snippets/src/test/java/pubsub/SubscriberIT.java +++ b/samples/snippets/src/test/java/pubsub/SubscriberIT.java @@ -118,7 +118,7 @@ public interface CheckedRunnable { void run() throws Exception; } - @Rule public Timeout globalTimeout = Timeout.seconds(1000); // 10 minute timeout + @Rule public Timeout globalTimeout = Timeout.seconds(10000); // 10 minute timeout @BeforeClass public static void checkRequirements() { From d8fb0fae688723345b334e478f89d230984dcc3f Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 27 Jun 2023 20:06:07 +0000 Subject: [PATCH 35/67] adding log statement for testing --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 4 ++++ samples/snippets/src/test/java/pubsub/SubscriberIT.java | 1 + 2 files changed, 5 insertions(+) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 7bf84f7ac..70e099c9e 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -403,6 +403,7 @@ void processReceivedMessages(List messages) { } OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { + System.out.println("Process Received Message: " + message); outstandingReceipts.put( message.getAckId(), new ReceiptCompleteData(outstandingMessage, false)); } else { @@ -418,6 +419,7 @@ void notifyAckSuccess(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); + System.out.println("Message Dispatche, ack successful: " + outstandingMessage.receivedMessage); // Setting to true means that the receipt is complete receiptCompleteData.setReceiptComplete(true); outstandingReceipts.put(ackRequestData.getAckId(), receiptCompleteData); @@ -431,6 +433,7 @@ void notifyAckSuccess(AckRequestData ackRequestData) { if (receipts.getValue().getReceiptComplete()) { outstandingReceipts.remove(ackId); completedReceipts.add(receipts.getValue().getOutstandingMessage()); + System.out.println("Message Dispatche, adding to completed receipts: " + receipts.getValue().getOutstandingMessage().receivedMessage); } else { break; } @@ -442,6 +445,7 @@ void notifyAckSuccess(AckRequestData ackRequestData) { void notifyAckFailed(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { + System.out.println("Message Dispatcher, failed: " + outstandingReceipts.get(ackRequestData.getAckId()).getOutstandingMessage().receivedMessage); outstandingReceipts.remove(ackRequestData.getAckId()); } } diff --git a/samples/snippets/src/test/java/pubsub/SubscriberIT.java b/samples/snippets/src/test/java/pubsub/SubscriberIT.java index 8e670b4d0..03d6a6d66 100644 --- a/samples/snippets/src/test/java/pubsub/SubscriberIT.java +++ b/samples/snippets/src/test/java/pubsub/SubscriberIT.java @@ -237,6 +237,7 @@ public void testSubscriberExactlyOnceDelivery() throws Exception { SubscribeWithExactlyOnceConsumerWithResponseExample .subscribeWithExactlyOnceConsumerWithResponseExample(projectId, subscriptionEodId); for (String messageId : messageIds) { + System.out.println("SubscriberIT: " + messageId); assertThat(bout.toString()).contains("Message successfully acked: " + messageId); } } From b90a6b6e2486a57485b43f411c91957d3279aba1 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 27 Jun 2023 20:10:32 +0000 Subject: [PATCH 36/67] Fixing lint --- .../google/cloud/pubsub/v1/MessageDispatcher.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 70e099c9e..a68c99102 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -419,7 +419,8 @@ void notifyAckSuccess(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); - System.out.println("Message Dispatche, ack successful: " + outstandingMessage.receivedMessage); + System.out.println( + "Message Dispatche, ack successful: " + outstandingMessage.receivedMessage); // Setting to true means that the receipt is complete receiptCompleteData.setReceiptComplete(true); outstandingReceipts.put(ackRequestData.getAckId(), receiptCompleteData); @@ -433,7 +434,9 @@ void notifyAckSuccess(AckRequestData ackRequestData) { if (receipts.getValue().getReceiptComplete()) { outstandingReceipts.remove(ackId); completedReceipts.add(receipts.getValue().getOutstandingMessage()); - System.out.println("Message Dispatche, adding to completed receipts: " + receipts.getValue().getOutstandingMessage().receivedMessage); + System.out.println( + "Message Dispatche, adding to completed receipts: " + + receipts.getValue().getOutstandingMessage().receivedMessage); } else { break; } @@ -445,7 +448,12 @@ void notifyAckSuccess(AckRequestData ackRequestData) { void notifyAckFailed(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { - System.out.println("Message Dispatcher, failed: " + outstandingReceipts.get(ackRequestData.getAckId()).getOutstandingMessage().receivedMessage); + System.out.println( + "Message Dispatcher, failed: " + + outstandingReceipts + .get(ackRequestData.getAckId()) + .getOutstandingMessage() + .receivedMessage); outstandingReceipts.remove(ackRequestData.getAckId()); } } From 0681eca95f7f004e57796cb49ad763dddcfe174f Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 29 Jun 2023 16:09:02 +0000 Subject: [PATCH 37/67] Adding more logs --- .../com/google/cloud/pubsub/v1/MessageDispatcher.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index a68c99102..84c0aa94a 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -420,7 +420,7 @@ void notifyAckSuccess(AckRequestData ackRequestData) { ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); System.out.println( - "Message Dispatche, ack successful: " + outstandingMessage.receivedMessage); + "Message Dispatcher, ack successful: " + outstandingMessage.receivedMessage); // Setting to true means that the receipt is complete receiptCompleteData.setReceiptComplete(true); outstandingReceipts.put(ackRequestData.getAckId(), receiptCompleteData); @@ -435,7 +435,7 @@ void notifyAckSuccess(AckRequestData ackRequestData) { outstandingReceipts.remove(ackId); completedReceipts.add(receipts.getValue().getOutstandingMessage()); System.out.println( - "Message Dispatche, adding to completed receipts: " + "Message Dispatcher, adding to completed receipts: " + receipts.getValue().getOutstandingMessage().receivedMessage); } else { break; @@ -463,6 +463,7 @@ private void processBatch(List batch) { for (OutstandingMessage message : batch) { // This is a blocking flow controller. We have already incremented messagesWaiter, so // shutdown will block on processing of all these messages anyway. + System.out.println("Processing batch message: " + message.receivedMessage); try { flowController.reserve(1, message.receivedMessage.getMessage().getSerializedSize()); } catch (FlowControlException unexpectedException) { @@ -504,10 +505,12 @@ public void run() { // Message expired while waiting. We don't extend these messages anymore, // so it was probably sent to someone else. Don't work on it. // Don't nack it either, because we'd be nacking someone else's message. + System.out.println("Message exprited: " + message); ackHandler.forget(); return; } if (shouldSetMessageFuture()) { + System.out.println("Message setting future on: " + message); // This is the message future that is propagated to the user SettableApiFuture messageFuture = ackHandler.getMessageFutureIfExists(); @@ -515,6 +518,7 @@ public void run() { new AckReplyConsumerWithResponseImpl(ackReplySettableApiFuture, messageFuture); receiverWithAckResponse.receiveMessage(message, ackReplyConsumerWithResponse); } else { + System.out.println("Message NOT setting future on: " + message); final AckReplyConsumer ackReplyConsumer = new AckReplyConsumerImpl(ackReplySettableApiFuture); receiver.receiveMessage(message, ackReplyConsumer); @@ -525,8 +529,10 @@ public void run() { } }; if (message.getOrderingKey().isEmpty()) { + System.out.println("Delivering Message"); executor.execute(deliverMessageTask); } else { + System.out.println("Delivering Ordered Message"); sequentialExecutor.submit(message.getOrderingKey(), deliverMessageTask); } } From 6ebd401c62e5f7b140ca546dce665d02356eb841 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 17 Jul 2023 18:34:12 +0000 Subject: [PATCH 38/67] batch size log --- .../main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 1 + 1 file changed, 1 insertion(+) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 84c0aa94a..3ad0cfe2e 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -459,6 +459,7 @@ void notifyAckFailed(AckRequestData ackRequestData) { } private void processBatch(List batch) { + System.out.println("Processing batch size: " + batch.size()); messagesWaiter.incrementPendingCount(batch.size()); for (OutstandingMessage message : batch) { // This is a blocking flow controller. We have already incremented messagesWaiter, so From 10a43f33100ec8bd3d6b3216e5ca020d2995efd7 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 17 Jul 2023 18:47:05 +0000 Subject: [PATCH 39/67] changing method to syncronized --- .../main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 3ad0cfe2e..b06161ebe 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -414,7 +414,7 @@ void processReceivedMessages(List messages) { processBatch(outstandingBatch); } - void notifyAckSuccess(AckRequestData ackRequestData) { + synchronized void notifyAckSuccess(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); From ead3c51756b3efd60538dfbd7c46ce9a7cb02b65 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 20 Jul 2023 15:33:33 +0000 Subject: [PATCH 40/67] fixing for loop to not remove as we are iterating --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index b06161ebe..e2179346f 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -428,7 +428,9 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { if (pendingMessages.putIfAbsent( outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { - for (Map.Entry receipts : outstandingReceipts.entrySet()) { + LinkedHashMap outstandingReceiptsCopy = outstandingReceipts; + + for (Map.Entry receipts : outstandingReceiptsCopy.entrySet()) { String ackId = receipts.getKey(); // If receipt is complete then add to completedReceipts to process the batch if (receipts.getValue().getReceiptComplete()) { From bb357262fe49235a9e3fb8273a25af80b1ddb83f Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 20 Jul 2023 20:40:22 +0000 Subject: [PATCH 41/67] trying a concurrent map --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index e2179346f..51a74ce88 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -90,8 +90,8 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingAcks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); - private final LinkedHashMap outstandingReceipts = - new LinkedHashMap(); + private final ConcurrentHashMap outstandingReceipts = + new ConcurrentHashMap(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -428,7 +428,7 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { if (pendingMessages.putIfAbsent( outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { - LinkedHashMap outstandingReceiptsCopy = outstandingReceipts; + ConcurrentHashMap outstandingReceiptsCopy = outstandingReceipts; for (Map.Entry receipts : outstandingReceiptsCopy.entrySet()) { String ackId = receipts.getKey(); From f9abf69894b9268d8bf40325dadc27fba3289dc2 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 17 Aug 2023 20:13:10 +0000 Subject: [PATCH 42/67] fix: syncronizing notifyFailed --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 2 +- .../src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java | 2 +- samples/snippets/src/test/java/pubsub/SubscriberIT.java | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 51a74ce88..8cf4e2045 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -448,7 +448,7 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { } } - void notifyAckFailed(AckRequestData ackRequestData) { + synchronized void notifyAckFailed(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { System.out.println( "Message Dispatcher, failed: " diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java index 258e90473..290b9927d 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/it/ITPubSubTest.java @@ -55,7 +55,7 @@ public class ITPubSubTest { private static final int MAX_INBOUND_MESSAGE_SIZE = 20 * 1024 * 1024; - @Rule public Timeout globalTimeout = Timeout.seconds(1000); + @Rule public Timeout globalTimeout = Timeout.seconds(300); @AutoValue abstract static class MessageAndConsumer { diff --git a/samples/snippets/src/test/java/pubsub/SubscriberIT.java b/samples/snippets/src/test/java/pubsub/SubscriberIT.java index 03d6a6d66..ee4e03068 100644 --- a/samples/snippets/src/test/java/pubsub/SubscriberIT.java +++ b/samples/snippets/src/test/java/pubsub/SubscriberIT.java @@ -118,7 +118,7 @@ public interface CheckedRunnable { void run() throws Exception; } - @Rule public Timeout globalTimeout = Timeout.seconds(10000); // 10 minute timeout + @Rule public Timeout globalTimeout = Timeout.seconds(600); // 10 minute timeout @BeforeClass public static void checkRequirements() { @@ -237,7 +237,6 @@ public void testSubscriberExactlyOnceDelivery() throws Exception { SubscribeWithExactlyOnceConsumerWithResponseExample .subscribeWithExactlyOnceConsumerWithResponseExample(projectId, subscriptionEodId); for (String messageId : messageIds) { - System.out.println("SubscriberIT: " + messageId); assertThat(bout.toString()).contains("Message successfully acked: " + messageId); } } From 551528bb72eb5c33b043c0b0b6a4ecbff56725a7 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 17 Aug 2023 20:16:47 +0000 Subject: [PATCH 43/67] fix: removing unused import --- .../main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 8cf4e2045..d3edb31dd 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -30,7 +30,6 @@ import com.google.pubsub.v1.ReceivedMessage; import java.util.ArrayList; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; From 04124fe2a83e2ce20264c77b24dd285d0c4f2208 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 17 Aug 2023 20:27:00 +0000 Subject: [PATCH 44/67] fix: reformat --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index d3edb31dd..2bbd1a4c2 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -427,7 +427,8 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { if (pendingMessages.putIfAbsent( outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) == null) { - ConcurrentHashMap outstandingReceiptsCopy = outstandingReceipts; + ConcurrentHashMap outstandingReceiptsCopy = + outstandingReceipts; for (Map.Entry receipts : outstandingReceiptsCopy.entrySet()) { String ackId = receipts.getKey(); From 7dcc8bcd42d82de04767348c7e96af6273b721a1 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 18 Aug 2023 16:02:57 +0000 Subject: [PATCH 45/67] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index ff71d5c59..5e309d777 100644 --- a/README.md +++ b/README.md @@ -51,20 +51,20 @@ If you are using Maven without BOM, add this to your dependencies: If you are using Gradle 5.x or later, add this to your dependencies: ```Groovy -implementation platform('com.google.cloud:libraries-bom:26.1.5') +implementation platform('com.google.cloud:libraries-bom:26.22.0') implementation 'com.google.cloud:google-cloud-pubsub' ``` If you are using Gradle without BOM, add this to your dependencies: ```Groovy -implementation 'com.google.cloud:google-cloud-pubsub:1.122.1' +implementation 'com.google.cloud:google-cloud-pubsub:1.124.1' ``` If you are using SBT, add this to your dependencies: ```Scala -libraryDependencies += "com.google.cloud" % "google-cloud-pubsub" % "1.122.1" +libraryDependencies += "com.google.cloud" % "google-cloud-pubsub" % "1.124.1" ``` ## Authentication From f4ca8a5d3ec0300c297ef59143bc7d3a78359ce3 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 18 Aug 2023 17:12:07 +0000 Subject: [PATCH 46/67] fix: removing System.out.println statements --- .../cloud/pubsub/v1/MessageDispatcher.java | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 2bbd1a4c2..cd7387126 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -402,7 +402,6 @@ void processReceivedMessages(List messages) { } OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { - System.out.println("Process Received Message: " + message); outstandingReceipts.put( message.getAckId(), new ReceiptCompleteData(outstandingMessage, false)); } else { @@ -418,8 +417,6 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); - System.out.println( - "Message Dispatcher, ack successful: " + outstandingMessage.receivedMessage); // Setting to true means that the receipt is complete receiptCompleteData.setReceiptComplete(true); outstandingReceipts.put(ackRequestData.getAckId(), receiptCompleteData); @@ -436,9 +433,6 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { if (receipts.getValue().getReceiptComplete()) { outstandingReceipts.remove(ackId); completedReceipts.add(receipts.getValue().getOutstandingMessage()); - System.out.println( - "Message Dispatcher, adding to completed receipts: " - + receipts.getValue().getOutstandingMessage().receivedMessage); } else { break; } @@ -450,23 +444,15 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { synchronized void notifyAckFailed(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { - System.out.println( - "Message Dispatcher, failed: " - + outstandingReceipts - .get(ackRequestData.getAckId()) - .getOutstandingMessage() - .receivedMessage); outstandingReceipts.remove(ackRequestData.getAckId()); } } private void processBatch(List batch) { - System.out.println("Processing batch size: " + batch.size()); messagesWaiter.incrementPendingCount(batch.size()); for (OutstandingMessage message : batch) { // This is a blocking flow controller. We have already incremented messagesWaiter, so // shutdown will block on processing of all these messages anyway. - System.out.println("Processing batch message: " + message.receivedMessage); try { flowController.reserve(1, message.receivedMessage.getMessage().getSerializedSize()); } catch (FlowControlException unexpectedException) { @@ -508,12 +494,10 @@ public void run() { // Message expired while waiting. We don't extend these messages anymore, // so it was probably sent to someone else. Don't work on it. // Don't nack it either, because we'd be nacking someone else's message. - System.out.println("Message exprited: " + message); ackHandler.forget(); return; } if (shouldSetMessageFuture()) { - System.out.println("Message setting future on: " + message); // This is the message future that is propagated to the user SettableApiFuture messageFuture = ackHandler.getMessageFutureIfExists(); @@ -521,7 +505,6 @@ public void run() { new AckReplyConsumerWithResponseImpl(ackReplySettableApiFuture, messageFuture); receiverWithAckResponse.receiveMessage(message, ackReplyConsumerWithResponse); } else { - System.out.println("Message NOT setting future on: " + message); final AckReplyConsumer ackReplyConsumer = new AckReplyConsumerImpl(ackReplySettableApiFuture); receiver.receiveMessage(message, ackReplyConsumer); @@ -532,10 +515,8 @@ public void run() { } }; if (message.getOrderingKey().isEmpty()) { - System.out.println("Delivering Message"); executor.execute(deliverMessageTask); } else { - System.out.println("Delivering Ordered Message"); sequentialExecutor.submit(message.getOrderingKey(), deliverMessageTask); } } From fc9b8a88f7e781fb4e3fe9ef1b9d29ecd4da6575 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 18 Aug 2023 22:49:18 +0000 Subject: [PATCH 47/67] fix: reviewign comments --- .../cloud/pubsub/v1/MessageDispatcher.java | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index cd7387126..e0e6e2926 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -30,8 +30,11 @@ import com.google.pubsub.v1.ReceivedMessage; import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executor; @@ -89,8 +92,8 @@ class MessageDispatcher { private final LinkedBlockingQueue pendingAcks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingNacks = new LinkedBlockingQueue<>(); private final LinkedBlockingQueue pendingReceipts = new LinkedBlockingQueue<>(); - private final ConcurrentHashMap outstandingReceipts = - new ConcurrentHashMap(); + private final LinkedHashMap outstandingReceipts = + new LinkedHashMap(); private final AtomicInteger messageDeadlineSeconds = new AtomicInteger(); private final AtomicBoolean extendDeadline = new AtomicBoolean(true); private final Lock jobLock; @@ -419,24 +422,24 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); // Setting to true means that the receipt is complete receiptCompleteData.setReceiptComplete(true); - outstandingReceipts.put(ackRequestData.getAckId(), receiptCompleteData); List completedReceipts = new ArrayList<>(); - if (pendingMessages.putIfAbsent( - outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) - == null) { - ConcurrentHashMap outstandingReceiptsCopy = - outstandingReceipts; - - for (Map.Entry receipts : outstandingReceiptsCopy.entrySet()) { - String ackId = receipts.getKey(); - // If receipt is complete then add to completedReceipts to process the batch - if (receipts.getValue().getReceiptComplete()) { - outstandingReceipts.remove(ackId); - completedReceipts.add(receipts.getValue().getOutstandingMessage()); - } else { - break; - } + // if (pendingMessages.putIfAbsent( + // outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) + // == null) { + + for (Iterator> it = outstandingReceipts.entrySet().iterator(); it.hasNext();) { + Map.Entry receipt = it.next(); + String ackId = receipt.getKey(); + // If receipt is complete then add to completedReceipts to process the batch + if (receipt.getValue().getReceiptComplete()) { + it.remove(); + pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler); + completedReceipts.add(receipt.getValue().getOutstandingMessage()); + } else { + break; } + // } + } processBatch(completedReceipts); } From 3091823b587dc8bfbb5d2ec14bf702cd1f0f56e1 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 18 Aug 2023 22:54:00 +0000 Subject: [PATCH 48/67] fix: lint --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index e0e6e2926..0ad80e13c 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -427,13 +427,16 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { // outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) // == null) { - for (Iterator> it = outstandingReceipts.entrySet().iterator(); it.hasNext();) { + for (Iterator> it = + outstandingReceipts.entrySet().iterator(); + it.hasNext(); ) { Map.Entry receipt = it.next(); String ackId = receipt.getKey(); // If receipt is complete then add to completedReceipts to process the batch if (receipt.getValue().getReceiptComplete()) { it.remove(); - pendingMessages.putIfAbsent(outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler); + pendingMessages.putIfAbsent( + outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler); completedReceipts.add(receipt.getValue().getOutstandingMessage()); } else { break; From 1cc036541b35529263e66fd1a18cf0187408903f Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 18 Aug 2023 23:14:11 +0000 Subject: [PATCH 49/67] adding another ordering key test example --- .../pubsub/v1/MessageDispatcherTest.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 2a8f9a8c0..28114b93a 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -138,7 +138,39 @@ public void testReceiptMessageReceiver() { verify(mockMessageReceiver, never()) .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumer.class)); } + @Test + public void testReceiptModackWithOrderingForExactlyOnceDelivered() { + + MessageReceiverWithAckResponse mockMessageReceiverWithAckResponse = + mock(MessageReceiverWithAckResponse.class); + MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiverWithAckResponse); + messageDispatcher.setExactlyOnceDeliveryEnabled(true); + + messageDispatcher.processReceivedMessages(Arrays.asList(TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE)); + messageDispatcher.processOutstandingOperations(); + verify(mockMessageReceiverWithAckResponse, never()) + .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); + + AckRequestData ackRequestData = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); + AckRequestData ackRequestData1 = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); + AckRequestData ackRequestData2 = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); + messageDispatcher.notifyAckSuccess(ackRequestData2); + messageDispatcher.processOutstandingOperations(); + + // Need to change to test correct contents of the message - will need custom matcher + verify(mockMessageReceiverWithAckResponse, times(1)) + .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); + + messageDispatcher.notifyAckSuccess(ackRequestData1); + messageDispatcher.notifyAckSuccess(ackRequestData); + messageDispatcher.processOutstandingOperations(); + + // Need to change to test correct contents of the message - will need custom matcher + verify(mockMessageReceiverWithAckResponse, times(2)) + .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); + + } @Test public void testReceiptModackForExactlyOnceDelivered() { From d906e118c88b121cc9e08648888d927a869271d2 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Fri, 18 Aug 2023 23:37:20 +0000 Subject: [PATCH 50/67] fix: trying to run this test again --- .../com/google/cloud/pubsub/v1/MessageDispatcherTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 28114b93a..aafd6d0d0 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -138,6 +138,7 @@ public void testReceiptMessageReceiver() { verify(mockMessageReceiver, never()) .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumer.class)); } + @Test public void testReceiptModackWithOrderingForExactlyOnceDelivered() { @@ -146,7 +147,8 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiverWithAckResponse); messageDispatcher.setExactlyOnceDeliveryEnabled(true); - messageDispatcher.processReceivedMessages(Arrays.asList(TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE)); + messageDispatcher.processReceivedMessages( + Arrays.asList(TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE)); messageDispatcher.processOutstandingOperations(); verify(mockMessageReceiverWithAckResponse, never()) @@ -162,15 +164,15 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); - messageDispatcher.notifyAckSuccess(ackRequestData1); messageDispatcher.notifyAckSuccess(ackRequestData); + messageDispatcher.notifyAckSuccess(ackRequestData1); messageDispatcher.processOutstandingOperations(); // Need to change to test correct contents of the message - will need custom matcher verify(mockMessageReceiverWithAckResponse, times(2)) .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); - } + @Test public void testReceiptModackForExactlyOnceDelivered() { From 46c25117249869761acab33998eb781c5b9a2399 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Sat, 19 Aug 2023 00:04:12 +0000 Subject: [PATCH 51/67] fix: trying to run this test again --- .../java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index aafd6d0d0..8ccb72508 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -169,7 +169,7 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { messageDispatcher.processOutstandingOperations(); // Need to change to test correct contents of the message - will need custom matcher - verify(mockMessageReceiverWithAckResponse, times(2)) + verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); } From b8edb54b9b65a3096b99463579f693e58f23cd6f Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 21 Aug 2023 14:26:10 +0000 Subject: [PATCH 52/67] fix: removing commented code --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 0ad80e13c..afb7c7b65 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -423,9 +423,6 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { // Setting to true means that the receipt is complete receiptCompleteData.setReceiptComplete(true); List completedReceipts = new ArrayList<>(); - // if (pendingMessages.putIfAbsent( - // outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler) - // == null) { for (Iterator> it = outstandingReceipts.entrySet().iterator(); @@ -441,7 +438,6 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { } else { break; } - // } } processBatch(completedReceipts); From 14e7292b617bb064457a98d4117145063234ff89 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Mon, 21 Aug 2023 14:31:32 +0000 Subject: [PATCH 53/67] fix: removing commented code --- .../main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index afb7c7b65..26421f0bc 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -438,7 +438,6 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { } else { break; } - } processBatch(completedReceipts); } From eb780ca3a29ab63f7a3ba6a7f2bfbf59c3183399 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 15:58:28 +0000 Subject: [PATCH 54/67] resolving the comments from review --- .../google/cloud/pubsub/v1/MessageDispatcher.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 26421f0bc..1d9b575a6 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -422,31 +422,28 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); // Setting to true means that the receipt is complete receiptCompleteData.setReceiptComplete(true); - List completedReceipts = new ArrayList<>(); + List outstandingBatch = new ArrayList<>(); for (Iterator> it = outstandingReceipts.entrySet().iterator(); it.hasNext(); ) { Map.Entry receipt = it.next(); - String ackId = receipt.getKey(); // If receipt is complete then add to completedReceipts to process the batch if (receipt.getValue().getReceiptComplete()) { it.remove(); pendingMessages.putIfAbsent( - outstandingMessage.receivedMessage.getAckId(), outstandingMessage.ackHandler); - completedReceipts.add(receipt.getValue().getOutstandingMessage()); + receipt.getKey(), receipt.getValue().getOutstandingMessage().ackHandler); + outstandingBatch.add(receipt.getValue().getOutstandingMessage()); } else { break; } } - processBatch(completedReceipts); + processBatch(outstandingBatch); } } synchronized void notifyAckFailed(AckRequestData ackRequestData) { - if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { - outstandingReceipts.remove(ackRequestData.getAckId()); - } + outstandingReceipts.remove(ackRequestData.getAckId()); } private void processBatch(List batch) { From aa5848b782442e7ea6ebaca8770be284cd5362bc Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 17:58:24 +0000 Subject: [PATCH 55/67] adding custom matcher --- .../pubsub/v1/MessageDispatcherTest.java | 43 +++++++++++++++---- .../cloud/pubsub/v1/MessageMatcher.java | 17 ++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 8ccb72508..3e2066fce 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -30,7 +30,9 @@ import java.util.concurrent.*; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentMatcher; import org.threeten.bp.Duration; +import com.google.cloud.pubsub.v1.MessageMatcher; public class MessageDispatcherTest { private static final ByteString MESSAGE_DATA = ByteString.copyFromUtf8("message-data"); @@ -110,6 +112,10 @@ public void receiveMessage( }; } + public boolean customMessageMatcher(ReceivedMessage receivedMessage, PubsubMessage message2) { + return (receivedMessage.getMessage() == message2); + } + @Test public void testSetupAndTeardown() { MessageDispatcher messageDispatcher = getMessageDispatcher(); @@ -147,30 +153,51 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { MessageDispatcher messageDispatcher = getMessageDispatcher(mockMessageReceiverWithAckResponse); messageDispatcher.setExactlyOnceDeliveryEnabled(true); + ReceivedMessage TEST_MESSAGE1 = + ReceivedMessage.newBuilder() + .setAckId("ACK_ID1") + .setMessage(PubsubMessage.newBuilder().setData(ByteString.copyFromUtf8("message-data1")).build()) + .setDeliveryAttempt(DELIVERY_INFO_COUNT) + .build(); + ReceivedMessage TEST_MESSAGE2 = + ReceivedMessage.newBuilder() + .setAckId("ACK_ID2") + .setMessage(PubsubMessage.newBuilder().setData(ByteString.copyFromUtf8("message-data2")).build()) + .setDeliveryAttempt(DELIVERY_INFO_COUNT) + .build(); + ReceivedMessage TEST_MESSAGE3 = + ReceivedMessage.newBuilder() + .setAckId("ACK_ID3") + .setMessage(PubsubMessage.newBuilder().setData(ByteString.copyFromUtf8("message-data3")).build()) + .setDeliveryAttempt(DELIVERY_INFO_COUNT) + .build(); + messageDispatcher.processReceivedMessages( - Arrays.asList(TEST_MESSAGE, TEST_MESSAGE, TEST_MESSAGE)); + Arrays.asList(TEST_MESSAGE2, TEST_MESSAGE1, TEST_MESSAGE3)); messageDispatcher.processOutstandingOperations(); verify(mockMessageReceiverWithAckResponse, never()) .receiveMessage(eq(TEST_MESSAGE.getMessage()), any(AckReplyConsumerWithResponse.class)); - AckRequestData ackRequestData = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); - AckRequestData ackRequestData1 = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); - AckRequestData ackRequestData2 = AckRequestData.newBuilder(TEST_MESSAGE.getAckId()).build(); + AckRequestData ackRequestData1 = AckRequestData.newBuilder(TEST_MESSAGE1.getAckId()).build(); + AckRequestData ackRequestData2 = AckRequestData.newBuilder(TEST_MESSAGE2.getAckId()).build(); + AckRequestData ackRequestData3 = AckRequestData.newBuilder(TEST_MESSAGE3.getAckId()).build(); messageDispatcher.notifyAckSuccess(ackRequestData2); messageDispatcher.processOutstandingOperations(); // Need to change to test correct contents of the message - will need custom matcher - verify(mockMessageReceiverWithAckResponse, times(1)) - .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); - messageDispatcher.notifyAckSuccess(ackRequestData); messageDispatcher.notifyAckSuccess(ackRequestData1); + messageDispatcher.notifyAckSuccess(ackRequestData3); messageDispatcher.processOutstandingOperations(); // Need to change to test correct contents of the message - will need custom matcher verify(mockMessageReceiverWithAckResponse, times(1)) - .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); + .receiveMessage(argThat(new MessageMatcher(TEST_MESSAGE3.getMessage())), any(AckReplyConsumerWithResponse.class)); + verify(mockMessageReceiverWithAckResponse, times(1)) + .receiveMessage(argThat(new MessageMatcher(TEST_MESSAGE2.getMessage())), any(AckReplyConsumerWithResponse.class)); + verify(mockMessageReceiverWithAckResponse, times(1)) + .receiveMessage(argThat(new MessageMatcher(TEST_MESSAGE1.getMessage())), any(AckReplyConsumerWithResponse.class)); } @Test diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java new file mode 100644 index 000000000..7261f0d75 --- /dev/null +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java @@ -0,0 +1,17 @@ +package com.google.cloud.pubsub.v1; + +import com.google.pubsub.v1.PubsubMessage; +import org.mockito.ArgumentMatcher; + +public class MessageMatcher implements ArgumentMatcher { + + private PubsubMessage message1; + + public MessageMatcher(PubsubMessage message) { + message1 = message; + } + @Override + public boolean matches(PubsubMessage message2) { + return (message1 == message2); + } +} From ac3840e95b02d5988c20c41f83865225440ed368 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 17:58:42 +0000 Subject: [PATCH 56/67] adding custom matcher --- .../com/google/cloud/pubsub/v1/MessageDispatcherTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 3e2066fce..40c1951d4 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -112,10 +112,6 @@ public void receiveMessage( }; } - public boolean customMessageMatcher(ReceivedMessage receivedMessage, PubsubMessage message2) { - return (receivedMessage.getMessage() == message2); - } - @Test public void testSetupAndTeardown() { MessageDispatcher messageDispatcher = getMessageDispatcher(); From e201ae40035f518e9e9afc8c15f8e16504965fd0 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 18:00:04 +0000 Subject: [PATCH 57/67] adding custom matcher --- .../java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 40c1951d4..dd8a19f95 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -169,7 +169,7 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { .build(); messageDispatcher.processReceivedMessages( - Arrays.asList(TEST_MESSAGE2, TEST_MESSAGE1, TEST_MESSAGE3)); + Arrays.asList(TEST_MESSAGE3, TEST_MESSAGE2, TEST_MESSAGE1)); messageDispatcher.processOutstandingOperations(); verify(mockMessageReceiverWithAckResponse, never()) From 1c94e5786b09d47b98075a3de099fdc74b53000a Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 18:12:50 +0000 Subject: [PATCH 58/67] adding custom matcher --- .../google/cloud/pubsub/v1/MessageMatcher.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java index 7261f0d75..acf384d3d 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java @@ -1,3 +1,19 @@ +/* + * Copyright 2017 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.pubsub.v1; import com.google.pubsub.v1.PubsubMessage; From 3e9c83f2b90418c82a8e6e1a458c74deff79c8ce Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 19:01:16 +0000 Subject: [PATCH 59/67] adding custom matcher correcting the matching statement --- .../test/java/com/google/cloud/pubsub/v1/MessageMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java index acf384d3d..f533ea278 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java @@ -28,6 +28,6 @@ public MessageMatcher(PubsubMessage message) { } @Override public boolean matches(PubsubMessage message2) { - return (message1 == message2); + return (message1.getData() == message2.getData()); } } From 3aef16b55e60eaf5039d4860bb8a140f16e6e0c2 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 20:21:34 +0000 Subject: [PATCH 60/67] lint --- .../pubsub/v1/MessageDispatcherTest.java | 29 ++++++++++++++----- .../cloud/pubsub/v1/MessageMatcher.java | 1 + 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index dd8a19f95..4542c71de 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -30,9 +30,7 @@ import java.util.concurrent.*; import org.junit.Before; import org.junit.Test; -import org.mockito.ArgumentMatcher; import org.threeten.bp.Duration; -import com.google.cloud.pubsub.v1.MessageMatcher; public class MessageDispatcherTest { private static final ByteString MESSAGE_DATA = ByteString.copyFromUtf8("message-data"); @@ -152,19 +150,28 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { ReceivedMessage TEST_MESSAGE1 = ReceivedMessage.newBuilder() .setAckId("ACK_ID1") - .setMessage(PubsubMessage.newBuilder().setData(ByteString.copyFromUtf8("message-data1")).build()) + .setMessage( + PubsubMessage.newBuilder() + .setData(ByteString.copyFromUtf8("message-data1")) + .build()) .setDeliveryAttempt(DELIVERY_INFO_COUNT) .build(); ReceivedMessage TEST_MESSAGE2 = ReceivedMessage.newBuilder() .setAckId("ACK_ID2") - .setMessage(PubsubMessage.newBuilder().setData(ByteString.copyFromUtf8("message-data2")).build()) + .setMessage( + PubsubMessage.newBuilder() + .setData(ByteString.copyFromUtf8("message-data2")) + .build()) .setDeliveryAttempt(DELIVERY_INFO_COUNT) .build(); ReceivedMessage TEST_MESSAGE3 = ReceivedMessage.newBuilder() .setAckId("ACK_ID3") - .setMessage(PubsubMessage.newBuilder().setData(ByteString.copyFromUtf8("message-data3")).build()) + .setMessage( + PubsubMessage.newBuilder() + .setData(ByteString.copyFromUtf8("message-data3")) + .build()) .setDeliveryAttempt(DELIVERY_INFO_COUNT) .build(); @@ -189,11 +196,17 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { // Need to change to test correct contents of the message - will need custom matcher verify(mockMessageReceiverWithAckResponse, times(1)) - .receiveMessage(argThat(new MessageMatcher(TEST_MESSAGE3.getMessage())), any(AckReplyConsumerWithResponse.class)); + .receiveMessage( + argThat(new MessageMatcher(TEST_MESSAGE3.getMessage())), + any(AckReplyConsumerWithResponse.class)); verify(mockMessageReceiverWithAckResponse, times(1)) - .receiveMessage(argThat(new MessageMatcher(TEST_MESSAGE2.getMessage())), any(AckReplyConsumerWithResponse.class)); + .receiveMessage( + argThat(new MessageMatcher(TEST_MESSAGE2.getMessage())), + any(AckReplyConsumerWithResponse.class)); verify(mockMessageReceiverWithAckResponse, times(1)) - .receiveMessage(argThat(new MessageMatcher(TEST_MESSAGE1.getMessage())), any(AckReplyConsumerWithResponse.class)); + .receiveMessage( + argThat(new MessageMatcher(TEST_MESSAGE1.getMessage())), + any(AckReplyConsumerWithResponse.class)); } @Test diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java index f533ea278..8c9641201 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java @@ -26,6 +26,7 @@ public class MessageMatcher implements ArgumentMatcher { public MessageMatcher(PubsubMessage message) { message1 = message; } + @Override public boolean matches(PubsubMessage message2) { return (message1.getData() == message2.getData()); From f8db4092acc39740e835690fd48a4ceb8d3017d6 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 21:21:41 +0000 Subject: [PATCH 61/67] removing comments --- .../com/google/cloud/pubsub/v1/MessageDispatcherTest.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 4542c71de..b2e8c4971 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -188,13 +188,10 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { messageDispatcher.notifyAckSuccess(ackRequestData2); messageDispatcher.processOutstandingOperations(); - // Need to change to test correct contents of the message - will need custom matcher - messageDispatcher.notifyAckSuccess(ackRequestData1); messageDispatcher.notifyAckSuccess(ackRequestData3); messageDispatcher.processOutstandingOperations(); - // Need to change to test correct contents of the message - will need custom matcher verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( argThat(new MessageMatcher(TEST_MESSAGE3.getMessage())), @@ -230,9 +227,8 @@ public void testReceiptModackForExactlyOnceDelivered() { List modackRequestDataList = new ArrayList(); modackRequestDataList.add(new ModackRequestData(MIN_ACK_DEADLINE_SECONDS, ackRequestData)); - // Need to change to test correct contents of the message - will need custom matcher verify(mockMessageReceiverWithAckResponse, times(1)) - .receiveMessage(any(PubsubMessage.class), any(AckReplyConsumerWithResponse.class)); + .receiveMessage(argThat(new MessageMatcher(TEST_MESSAGE.getMessage())), any(AckReplyConsumerWithResponse.class)); } @Test From 11af2a0533e60caadfb122f3ef35eab8ab1ab69e Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 21:22:59 +0000 Subject: [PATCH 62/67] removing comments --- .../main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 1d9b575a6..3c99569bb 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -428,7 +428,7 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { outstandingReceipts.entrySet().iterator(); it.hasNext(); ) { Map.Entry receipt = it.next(); - // If receipt is complete then add to completedReceipts to process the batch + // If receipt is complete then add to outstandingBatch to process the batch if (receipt.getValue().getReceiptComplete()) { it.remove(); pendingMessages.putIfAbsent( From 3200173f30af70a01fa7be390ef6307cccd128a0 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Tue, 22 Aug 2023 21:34:37 +0000 Subject: [PATCH 63/67] removing comments --- .../com/google/cloud/pubsub/v1/MessageDispatcherTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index b2e8c4971..f1068a746 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -228,7 +228,9 @@ public void testReceiptModackForExactlyOnceDelivered() { modackRequestDataList.add(new ModackRequestData(MIN_ACK_DEADLINE_SECONDS, ackRequestData)); verify(mockMessageReceiverWithAckResponse, times(1)) - .receiveMessage(argThat(new MessageMatcher(TEST_MESSAGE.getMessage())), any(AckReplyConsumerWithResponse.class)); + .receiveMessage( + argThat(new MessageMatcher(TEST_MESSAGE.getMessage())), + any(AckReplyConsumerWithResponse.class)); } @Test From e06b9689aa7f92a4b5202a5d1fc076561d7d4a84 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Wed, 23 Aug 2023 20:49:10 +0000 Subject: [PATCH 64/67] changing messageMatcher to messageDataMatcher, and fixing other nit things --- .../cloud/pubsub/v1/MessageDispatcher.java | 38 +++++++++---------- ...geMatcher.java => MessageDataMatcher.java} | 4 +- .../pubsub/v1/MessageDispatcherTest.java | 8 ++-- 3 files changed, 23 insertions(+), 27 deletions(-) rename google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/{MessageMatcher.java => MessageDataMatcher.java} (87%) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 3c99569bb..3edaa6075 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -358,25 +358,21 @@ private static class ReceiptCompleteData { private OutstandingMessage outstandingMessage; private Boolean receiptComplete; - private ReceiptCompleteData(OutstandingMessage outstandingMessage, Boolean receiptComplete) { + private ReceiptCompleteData(OutstandingMessage outstandingMessage) { this.outstandingMessage = outstandingMessage; - this.receiptComplete = receiptComplete; + this.receiptComplete = false; } private OutstandingMessage getOutstandingMessage() { return this.outstandingMessage; } - private Boolean getReceiptComplete() { + private Boolean isReceiptComplete() { return this.receiptComplete; } - private void setOutstandingMessage(OutstandingMessage outstandingMessage) { - this.outstandingMessage = outstandingMessage; - } - - private void setReceiptComplete(Boolean receiptComplete) { - this.receiptComplete = receiptComplete; + private void notifyReceiptComplete() { + this.receiptComplete = true; } } @@ -391,8 +387,12 @@ void processReceivedMessages(List messages) { AckRequestData ackRequestData = builder.build(); AckHandler ackHandler = new AckHandler(ackRequestData, message.getMessage().getSerializedSize(), totalExpiration); - if (!this.exactlyOnceDeliveryEnabled.get() - && pendingMessages.putIfAbsent(message.getAckId(), ackHandler) != null) { + OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); + + if (this.exactlyOnceDeliveryEnabled.get()) { + outstandingReceipts.put( + message.getAckId(), new ReceiptCompleteData(outstandingMessage)); + } else if (pendingMessages.putIfAbsent(message.getAckId(), ackHandler) != null) { // putIfAbsent puts ackHandler if ackID isn't previously mapped, then return the // previously-mapped element. // If the previous element is not null, we already have the message and the new one is @@ -402,11 +402,6 @@ void processReceivedMessages(List messages) { // we want to eventually // totally expire so that pubsub service sends us the message again. continue; - } - OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); - if (this.exactlyOnceDeliveryEnabled.get()) { - outstandingReceipts.put( - message.getAckId(), new ReceiptCompleteData(outstandingMessage, false)); } else { outstandingBatch.add(outstandingMessage); } @@ -421,7 +416,7 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); // Setting to true means that the receipt is complete - receiptCompleteData.setReceiptComplete(true); + receiptCompleteData.notifyReceiptComplete(); List outstandingBatch = new ArrayList<>(); for (Iterator> it = @@ -429,11 +424,12 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { it.hasNext(); ) { Map.Entry receipt = it.next(); // If receipt is complete then add to outstandingBatch to process the batch - if (receipt.getValue().getReceiptComplete()) { + if (receipt.getValue().isReceiptComplete()) { it.remove(); - pendingMessages.putIfAbsent( - receipt.getKey(), receipt.getValue().getOutstandingMessage().ackHandler); - outstandingBatch.add(receipt.getValue().getOutstandingMessage()); + if (pendingMessages.putIfAbsent( + receipt.getKey(), receipt.getValue().getOutstandingMessage().ackHandler) == null) { //msg2, msg1, msg3 + outstandingBatch.add(receipt.getValue().getOutstandingMessage()); + } } else { break; } diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java similarity index 87% rename from google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java rename to google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java index 8c9641201..6a8a82e00 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageMatcher.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java @@ -19,11 +19,11 @@ import com.google.pubsub.v1.PubsubMessage; import org.mockito.ArgumentMatcher; -public class MessageMatcher implements ArgumentMatcher { +public class MessageDataMatcher implements ArgumentMatcher { private PubsubMessage message1; - public MessageMatcher(PubsubMessage message) { + public MessageDataMatcher(PubsubMessage message) { message1 = message; } diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index f1068a746..280e4f964 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -194,15 +194,15 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( - argThat(new MessageMatcher(TEST_MESSAGE3.getMessage())), + argThat(new MessageDataMatcher(TEST_MESSAGE3.getMessage())), any(AckReplyConsumerWithResponse.class)); verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( - argThat(new MessageMatcher(TEST_MESSAGE2.getMessage())), + argThat(new MessageDataMatcher(TEST_MESSAGE2.getMessage())), any(AckReplyConsumerWithResponse.class)); verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( - argThat(new MessageMatcher(TEST_MESSAGE1.getMessage())), + argThat(new MessageDataMatcher(TEST_MESSAGE1.getMessage())), any(AckReplyConsumerWithResponse.class)); } @@ -229,7 +229,7 @@ public void testReceiptModackForExactlyOnceDelivered() { verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( - argThat(new MessageMatcher(TEST_MESSAGE.getMessage())), + argThat(new MessageDataMatcher(TEST_MESSAGE.getMessage())), any(AckReplyConsumerWithResponse.class)); } From 1846eb6b1aa84f24d57bfa0b1b04dbc8be606208 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Wed, 23 Aug 2023 20:53:43 +0000 Subject: [PATCH 65/67] lint --- .../java/com/google/cloud/pubsub/v1/MessageDispatcher.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 3edaa6075..5792e718c 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -390,8 +390,7 @@ void processReceivedMessages(List messages) { OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { - outstandingReceipts.put( - message.getAckId(), new ReceiptCompleteData(outstandingMessage)); + outstandingReceipts.put(message.getAckId(), new ReceiptCompleteData(outstandingMessage)); } else if (pendingMessages.putIfAbsent(message.getAckId(), ackHandler) != null) { // putIfAbsent puts ackHandler if ackID isn't previously mapped, then return the // previously-mapped element. @@ -427,7 +426,8 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { if (receipt.getValue().isReceiptComplete()) { it.remove(); if (pendingMessages.putIfAbsent( - receipt.getKey(), receipt.getValue().getOutstandingMessage().ackHandler) == null) { //msg2, msg1, msg3 + receipt.getKey(), receipt.getValue().getOutstandingMessage().ackHandler) + == null) { // msg2, msg1, msg3 outstandingBatch.add(receipt.getValue().getOutstandingMessage()); } } else { From 3e45ca531be000c90588d3b91173d21d932272f4 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 24 Aug 2023 16:01:19 +0000 Subject: [PATCH 66/67] addressing review comments --- .../com/google/cloud/pubsub/v1/MessageDispatcher.java | 9 ++++----- .../com/google/cloud/pubsub/v1/MessageDataMatcher.java | 9 +++++---- .../google/cloud/pubsub/v1/MessageDispatcherTest.java | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java index 5792e718c..9556849bb 100644 --- a/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java +++ b/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java @@ -390,6 +390,8 @@ void processReceivedMessages(List messages) { OutstandingMessage outstandingMessage = new OutstandingMessage(message, ackHandler); if (this.exactlyOnceDeliveryEnabled.get()) { + // For exactly once deliveries we don't add to outstanding batch because we first + // process the receipt modack. If that is successful then we process the message. outstandingReceipts.put(message.getAckId(), new ReceiptCompleteData(outstandingMessage)); } else if (pendingMessages.putIfAbsent(message.getAckId(), ackHandler) != null) { // putIfAbsent puts ackHandler if ackID isn't previously mapped, then return the @@ -412,10 +414,7 @@ void processReceivedMessages(List messages) { synchronized void notifyAckSuccess(AckRequestData ackRequestData) { if (outstandingReceipts.containsKey(ackRequestData.getAckId())) { - ReceiptCompleteData receiptCompleteData = outstandingReceipts.get(ackRequestData.getAckId()); - OutstandingMessage outstandingMessage = receiptCompleteData.getOutstandingMessage(); - // Setting to true means that the receipt is complete - receiptCompleteData.notifyReceiptComplete(); + outstandingReceipts.get(ackRequestData.getAckId()).notifyReceiptComplete(); List outstandingBatch = new ArrayList<>(); for (Iterator> it = @@ -427,7 +426,7 @@ synchronized void notifyAckSuccess(AckRequestData ackRequestData) { it.remove(); if (pendingMessages.putIfAbsent( receipt.getKey(), receipt.getValue().getOutstandingMessage().ackHandler) - == null) { // msg2, msg1, msg3 + == null) { outstandingBatch.add(receipt.getValue().getOutstandingMessage()); } } else { diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java index 6a8a82e00..38bc8f24f 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java @@ -16,19 +16,20 @@ package com.google.cloud.pubsub.v1; +import com.google.protobuf.ByteString; import com.google.pubsub.v1.PubsubMessage; import org.mockito.ArgumentMatcher; public class MessageDataMatcher implements ArgumentMatcher { - private PubsubMessage message1; + private ByteString expectedData; - public MessageDataMatcher(PubsubMessage message) { - message1 = message; + public MessageDataMatcher(ByteString expectedData) { + this.expectedData = expectedData; } @Override public boolean matches(PubsubMessage message2) { - return (message1.getData() == message2.getData()); + return (expectedData == message2.getData()); } } diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java index 280e4f964..9321272b4 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDispatcherTest.java @@ -194,15 +194,15 @@ public void testReceiptModackWithOrderingForExactlyOnceDelivered() { verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( - argThat(new MessageDataMatcher(TEST_MESSAGE3.getMessage())), + argThat(new MessageDataMatcher(TEST_MESSAGE3.getMessage().getData())), any(AckReplyConsumerWithResponse.class)); verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( - argThat(new MessageDataMatcher(TEST_MESSAGE2.getMessage())), + argThat(new MessageDataMatcher(TEST_MESSAGE2.getMessage().getData())), any(AckReplyConsumerWithResponse.class)); verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( - argThat(new MessageDataMatcher(TEST_MESSAGE1.getMessage())), + argThat(new MessageDataMatcher(TEST_MESSAGE1.getMessage().getData())), any(AckReplyConsumerWithResponse.class)); } @@ -229,7 +229,7 @@ public void testReceiptModackForExactlyOnceDelivered() { verify(mockMessageReceiverWithAckResponse, times(1)) .receiveMessage( - argThat(new MessageDataMatcher(TEST_MESSAGE.getMessage())), + argThat(new MessageDataMatcher(TEST_MESSAGE.getMessage().getData())), any(AckReplyConsumerWithResponse.class)); } From a2336bd149e40e81c40f284976dece87de6c1e45 Mon Sep 17 00:00:00 2001 From: Maitri Mangal Date: Thu, 24 Aug 2023 16:30:40 +0000 Subject: [PATCH 67/67] addressing review comments --- .../java/com/google/cloud/pubsub/v1/MessageDataMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java index 38bc8f24f..745b18244 100644 --- a/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java +++ b/google-cloud-pubsub/src/test/java/com/google/cloud/pubsub/v1/MessageDataMatcher.java @@ -30,6 +30,6 @@ public MessageDataMatcher(ByteString expectedData) { @Override public boolean matches(PubsubMessage message2) { - return (expectedData == message2.getData()); + return (expectedData.equals(message2.getData())); } }