You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2026/03/24 20:46:52 UTC

[PR] Removes the completed set from the coordinator [accumulo]

keith-turner opened a new pull request, #6241:
URL: https://github.com/apache/accumulo/pull/6241

   This change is made in support of #6217.  Successfully ran all ITs that were changed.


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Removes the completed set from the coordinator [accumulo]

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #6241:
URL: https://github.com/apache/accumulo/pull/6241#discussion_r2987748406


##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -247,9 +246,6 @@ public CompactionCoordinator(Manager manager,
 
     this.fateClients = fateClients;
 
-    completed = ctx.getCaches().createNewBuilder(CacheName.COMPACTIONS_COMPLETED, true)

Review Comment:
   Can `CacheName.COMPACTIONS_COMPLETED` be removed also?



##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -1052,29 +1045,6 @@ public TExternalCompactionMap getRunningCompactions(TInfo tinfo, TCredentials cr
     return result;
   }
 
-  /**
-   * Return information about recently completed compactions
-   *
-   * @param tinfo trace info
-   * @param credentials tcredentials object
-   * @return map of ECID to TExternalCompaction objects
-   * @throws ThriftSecurityException permission error
-   */
-  @Override
-  public TExternalCompactionMap getCompletedCompactions(TInfo tinfo, TCredentials credentials)

Review Comment:
   This was only used by tests in 4.0?



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Removes the completed set from the coordinator [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #6241:
URL: https://github.com/apache/accumulo/pull/6241


-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Removes the completed set from the coordinator [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #6241:
URL: https://github.com/apache/accumulo/pull/6241#discussion_r2988946366


##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -1052,29 +1045,6 @@ public TExternalCompactionMap getRunningCompactions(TInfo tinfo, TCredentials cr
     return result;
   }
 
-  /**
-   * Return information about recently completed compactions
-   *
-   * @param tinfo trace info
-   * @param credentials tcredentials object
-   * @return map of ECID to TExternalCompaction objects
-   * @throws ThriftSecurityException permission error
-   */
-  @Override
-  public TExternalCompactionMap getCompletedCompactions(TInfo tinfo, TCredentials credentials)

Review Comment:
   yeah it seems so



##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -247,9 +246,6 @@ public CompactionCoordinator(Manager manager,
 
     this.fateClients = fateClients;
 
-    completed = ctx.getCaches().createNewBuilder(CacheName.COMPACTIONS_COMPLETED, true)

Review Comment:
   probably, I will look into that



-- 
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: notifications-unsubscribe@accumulo.apache.org

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


Re: [PR] Removes the completed set from the coordinator [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #6241:
URL: https://github.com/apache/accumulo/pull/6241#discussion_r2984183282


##########
test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompactionTestUtils.java:
##########
@@ -162,7 +162,7 @@ public static void writeData(AccumuloClient client, String table1, int rows)
       }
     }
 
-    client.tableOperations().flush(table1);
+    client.tableOperations().flush(table1, null, null, true);

Review Comment:
   This was an existing test bug I ran into while running ITs.  Changed this code to wait for the flush instead of just initiate it.  One test was hanging because it was trying to create 4 files, but this would not always happen.



-- 
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: notifications-unsubscribe@accumulo.apache.org

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