milleruntime commented on a change in pull request #1899:
URL: https://github.com/apache/accumulo/pull/1899#discussion_r569678771



##########
File path: 
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
##########
@@ -556,14 +557,15 @@ private void manageMemory() {
                 continue;
               }
               Tablet tablet = tabletReport.getTablet();
-              if 
(!tablet.initiateMinorCompaction(MinorCompactionReason.SYSTEM)) {
-                if (tablet.isClosed()) {
+              var state = 
context.getTableManager().getTableState(tablet.getExtent().tableId());
+              if 
(!tablet.initiateMinorCompaction(MinorCompactionReason.SYSTEM, state)) {
+                if (tablet.isClosed() || state.equals(TableState.DELETING)) {

Review comment:
       Yeah. I haven't seen the MinC max threads 
(tserver.compaction.minor.concurrent.max=4) consumed by 4 deleted tablets after 
removing the report. But I think it is still possible.
   
   I am wondering now if I should move the check to inside of 
`LargestFirstMemoryManager.tabletsToMinorCompact()` so that the deleted tablets 
will be skipped and not returned in `tabletsToMinorCompact`. This would allow 
flushes up to `tserver.compaction.minor.concurrent.max` without having to make 
another pass. Right now the `tabletsToMinorCompact` won't utilize all of the 
MinC threads if some tablets are being deleted. If all large tablets are being 
deleted, then no MinC will run during that pass.




----------------------------------------------------------------
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:
[email protected]


Reply via email to