[GitHub] [hudi] nsivabalan commented on a diff in pull request #5255: [HUDI-3798] Fixing ending of a transaction by different owner and removing some extraneous methods in trxn manager

[GitBox <gi...@apache.org>]

nsivabalan commented on code in PR #5255:
URL: https://github.com/apache/hudi/pull/5255#discussion_r846870445


##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestTransactionManager.java:
##########
@@ -152,6 +165,32 @@ public void testMultiWriterTransactions() {
     Assertions.assertTrue(writer2Completed.get());
   }
 
+  @Test
+  public void testEndTransactionByDiffOwner() throws InterruptedException {
+    // 1. Begin and end by the same transaction owner
+    Option<HoodieInstant> lastCompletedInstant = getInstant("0000001");
+    Option<HoodieInstant> newTxnOwnerInstant = getInstant("0000002");
+    transactionManager.beginTransaction(newTxnOwnerInstant, lastCompletedInstant);
+
+    CountDownLatch countDownLatch = new CountDownLatch(1);
+    // Another writer thread
+    Thread writer2 = new Thread(() -> {
+      Option<HoodieInstant> newTxnOwnerInstant1 = getInstant("0000003");
+      transactionManager.endTransaction(newTxnOwnerInstant1);
+      countDownLatch.countDown();
+    });
+
+    writer2.start();
+    countDownLatch.await(30, TimeUnit.SECONDS);
+    // should not have reset the state within transaction manager since the owner is different.
+    Assertions.assertTrue(transactionManager.getCurrentTransactionOwner().isPresent());

Review Comment:
   we usually don't use VisibleForTesting annotation. lets sync up on this. we might have tests in InProcessLockProvider to assert for the condition you are talking about



-- 
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: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org