dlmarion commented on code in PR #5289:
URL: https://github.com/apache/accumulo/pull/5289#discussion_r1934588759
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/MinorCompactionTask.java:
##########
@@ -49,13 +55,32 @@ class MinorCompactionTask implements Runnable {
this.commitSession = commitSession;
this.flushId = flushId;
this.mincReason = mincReason;
+ this.pauseLimit =
tablet.getContext().getTableConfiguration(tablet.extent.tableId())
+ .getCount(Property.TABLE_FILE_PAUSE);
+ }
+
+ private static void checkMinorCompactionFiles(String tableId, long
pauseLimit,
+ long currentFileCount) throws AcceptableThriftTableOperationException {
+ if (pauseLimit > 0 && currentFileCount > pauseLimit) {
+ throw new AcceptableThriftTableOperationException(tableId, null,
TableOperation.COMPACT,
+ TableOperationExceptionType.OTHER, "Attempted to perform minor
compaction with "
+ + currentFileCount + " files, exceeding the configured limit of
" + pauseLimit);
+ }
}
@Override
public void run() {
tablet.minorCompactionStarted();
try {
Span span = TraceUtil.startSpan(this.getClass(), "minorCompaction");
+ try {
+ long currentFileCount = tablet.getTabletMemory().getNumEntries();
+ checkMinorCompactionFiles(tablet.extent.tableId().canonical(),
pauseLimit,
+ currentFileCount); // Add pause check here
+ } catch (AcceptableThriftTableOperationException e) {
+ log.warn("Minor compaction paused due to file count for tablet {}",
tablet.extent);
+ return;
Review Comment:
I don't think that returning here will cause a minor compaction to be
paused. I think the calling code will look at this as being completed
successfully, right? I'm not sure this is the right solution. @keith-turner -
what was your vision of this change from #5151 ?
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -392,7 +393,13 @@ DataFileValue minorCompact(InMemoryMap memTable,
ReferencedTabletFile tmpDatafil
Span span = TraceUtil.startSpan(this.getClass(), "minorCompact::write");
try (Scope scope = span.makeCurrent()) {
count = memTable.getNumEntries();
-
+ if (count > pauseLimit && pauseLimit > 0) {
Review Comment:
I don't think we want to pause minor compactions when the number of
**entries** in tablet exceeds the file count pause property. We only want to
pause minor compactions when the number of **files** in the tablet exceeds the
file count pause property. I think that the change in this file can be removed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
