[GitHub] [geode] pivotal-eshu commented on a change in pull request #6054: GEODE-8907: CI Failure: ClientServerTransactionCCEDUnitTest.testTxRemoveAll

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

pivotal-eshu commented on a change in pull request #6054:
URL: https://github.com/apache/geode/pull/6054#discussion_r586752182



##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java
##########
@@ -2076,10 +2078,11 @@ public Object call() throws Exception {
       }
     });
 
-    client.invoke(new SerializableCallable() {
+    TXId txid2 = (TXId) client.invoke(new SerializableCallable() {

Review comment:
       There is no need to have a new txId here, unless you want to assert this is the same as txId. There is only one transaction. Probably just ignore the return value here. And later resume the original txId instead of txid2.

##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/cache/RemoteTransactionDUnitTest.java
##########
@@ -3010,50 +3024,82 @@ public Object call() throws Exception {
       }
     });
 
-    SerializableCallable testFnOnDs = new SerializableCallable() {
-      @Override
-      public Object call() throws Exception {
-        TXManagerImpl mgr = getGemfireCache().getTXMgr();
-        PartitionedRegion pr = (PartitionedRegion) getGemfireCache().getRegion(CUSTOMER);
-        CustId keyOnDs = getKeyOnMember(pr.getMyId(), pr);
-        mgr.begin();
-        pr.get(keyOnDs);
-        Set filter = new HashSet();
-        filter.add(keyOnDs);
-        FunctionService.onRegion(pr).withFilter(filter).execute(TXFunction.id).getResult();
-        assertEquals(expectedCustomer, pr.get(expectedCustId));
-        TXStateProxy tx = mgr.pauseTransaction();
-        assertNull(pr.get(expectedCustId));
-        mgr.unpauseTransaction(tx);
-        return null;
-      }
-    };
-    SerializableCallable closeTx = new SerializableCallable() {
-      @Override
-      public Object call() throws Exception {
-        Region custRegion = getGemfireCache().getRegion(CUSTOMER);
-        CacheTransactionManager mgr = getGemfireCache().getTXMgr();
-        mgr.commit();
-        assertEquals(expectedCustomer, custRegion.get(expectedCustId));
-        custRegion.destroy(expectedCustId);
-        return null;
-      }
-    };
-
     if (owner.equals(ds1)) {
-      datastore1.invoke(testFnOnDs);
+      TXId dsTxId = (TXId) datastore1.invoke(

Review comment:
       Should not duplicate the code here, should reuse the existing callables.
   
   To pass in different txIds in the callable, you can see how DoOpsInTX is implemented.




----------------------------------------------------------------
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.

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