eolivelli commented on a change in pull request #11305:
URL: https://github.com/apache/pulsar/pull/11305#discussion_r668795163



##########
File path: 
pulsar-broker/src/test/java/org/apache/pulsar/broker/transaction/buffer/TransactionBufferClientTest.java
##########
@@ -311,10 +231,26 @@ public void testTransactionBufferChannelUnActive() {
     }
 
     @Test
-    public void testTransactionBufferLookUp() throws ExecutionException, 
InterruptedException {
+    public void testTransactionBufferLookUp() throws Exception {
         String topic = "persistent://" + namespace + 
"/testTransactionBufferLookUp";
-        tbClient.abortTxnOnSubscription(topic + "_abort_sub", "test", 1L, 1L, 
-1L).get();
-        tbClient.commitTxnOnSubscription(topic + "_commit_sub", "test", 1L, 
1L, -1L).get();
+
+        Awaitility.await().until(() -> {

Review comment:
       why do we have to use this loop ?
   in this test the command is supposed to work well
   which kind of error may happen ?

##########
File path: 
pulsar-broker/src/test/java/org/apache/pulsar/broker/transaction/buffer/TransactionBufferClientTest.java
##########
@@ -195,43 +152,6 @@ public void testAbortOnSubscription() throws 
ExecutionException, InterruptedExce
         }
     }
 
-    @Test
-    public void testTransactionBufferOpFail() throws InterruptedException, 
ExecutionException {

Review comment:
       why are we dropping this test ?

##########
File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1995,12 +1995,9 @@ protected void 
handleEndTxnOnSubscription(CommandEndTxnOnSubscription command) {
 
                 Subscription subscription = 
optionalTopic.get().getSubscription(subName);
                 if (subscription == null) {
-                    log.error("Topic {} subscription {} is not exist.", 
optionalTopic.get().getName(), subName);
-                    ctx.writeAndFlush(Commands.newEndTxnOnSubscriptionResponse(
-                            requestId, txnidLeastBits, txnidMostBits,
-                            ServerError.ServiceNotReady,
-                            "Topic " + optionalTopic.get().getName()
-                                    + " subscription " + subName + " is not 
exist."));
+                    log.warn("Topic {} subscription {} is not exist.", 
optionalTopic.get().getName(), subName);

Review comment:
       we can use this opportunity to fix the typo and use this message: `Topic 
{} subscription {} does not exist`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to