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]