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 2024/12/14 21:31:41 UTC

[PR] limits memory and cpu used by compaction reservation request [accumulo]

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

   Added threads pools to execute compaction reservation request in order to limit memory and cpu used by executing reservations.  Request queued up for the pool could still potentially use a lot of memory.  Did two things to control memory of things in the queue.  First only allow a compactor process to have one reservation processing at time.  Second made the data related to a resevation request a soft reference which should allow it be garbage collected if memory gets low while it sitting in the queue.  Once the request starts executing it obtains a strong refrence to the data so it can no longer be garbage collected.
   
   fixes #5177


-- 
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] limits memory and cpu used by compaction reservation request [accumulo]

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


##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -528,82 +551,138 @@ protected CompactionMetadata createExternalCompactionMetadata(CompactionJob job,
 
   }
 
-  protected CompactionMetadata reserveCompaction(CompactionJobQueues.MetaJob metaJob,
-      String compactorAddress, ExternalCompactionId externalCompactionId) {
+  private class ReserveCompactionTask implements Supplier<CompactionMetadata> {
+
+    // Use a soft reference for this in case free memory gets low while this is sitting in the queue
+    // waiting to process. This object can contain the tablets list of files and if there are lots
+    // of tablet with lots of files then that could start to cause memory problems.
+    private final SoftReference<CompactionJobQueues.MetaJob> metaJobRef;
+    private final String compactorAddress;
+    private final ExternalCompactionId externalCompactionId;
+
+    private ReserveCompactionTask(CompactionJobQueues.MetaJob metaJob, String compactorAddress,
+        ExternalCompactionId externalCompactionId) {
+      Preconditions.checkArgument(metaJob.getJob().getKind() == CompactionKind.SYSTEM
+          || metaJob.getJob().getKind() == CompactionKind.USER);
+      this.metaJobRef = new SoftReference<>(Objects.requireNonNull(metaJob));
+      this.compactorAddress = Objects.requireNonNull(compactorAddress);
+      this.externalCompactionId = Objects.requireNonNull(externalCompactionId);
+      Preconditions.checkState(activeCompactorReservationRequest.add(compactorAddress));
+    }
 
-    Preconditions.checkArgument(metaJob.getJob().getKind() == CompactionKind.SYSTEM
-        || metaJob.getJob().getKind() == CompactionKind.USER);
+    @Override
+    public CompactionMetadata get() {
+      try {
+        var metaJob = metaJobRef.get();
+        if (metaJob == null) {
+          LOG.warn("Compaction reservation request for {} {} was garbage collected.",
+              compactorAddress, externalCompactionId);
+          return null;
+        }
 
-    var tabletMetadata = metaJob.getTabletMetadata();
+        var tabletMetadata = metaJob.getTabletMetadata();
 
-    var jobFiles = metaJob.getJob().getFiles().stream().map(CompactableFileImpl::toStoredTabletFile)
-        .collect(Collectors.toSet());
+        var jobFiles = metaJob.getJob().getFiles().stream()
+            .map(CompactableFileImpl::toStoredTabletFile).collect(Collectors.toSet());
 
-    Retry retry = Retry.builder().maxRetries(5).retryAfter(Duration.ofMillis(100))
-        .incrementBy(Duration.ofMillis(100)).maxWait(Duration.ofSeconds(10)).backOffFactor(1.5)
-        .logInterval(Duration.ofMinutes(3)).createRetry();
+        Retry retry = Retry.builder().maxRetries(5).retryAfter(Duration.ofMillis(100))
+            .incrementBy(Duration.ofMillis(100)).maxWait(Duration.ofSeconds(10)).backOffFactor(1.5)
+            .logInterval(Duration.ofMinutes(3)).createRetry();
 
-    while (retry.canRetry()) {
-      try (var tabletsMutator = ctx.getAmple().conditionallyMutateTablets()) {
-        var extent = metaJob.getTabletMetadata().getExtent();
+        while (retry.canRetry()) {
+          try (var tabletsMutator = ctx.getAmple().conditionallyMutateTablets()) {
+            var extent = metaJob.getTabletMetadata().getExtent();
 
-        if (!canReserveCompaction(tabletMetadata, metaJob.getJob().getKind(), jobFiles, ctx,
-            manager.getSteadyTime())) {
-          return null;
-        }
+            if (!canReserveCompaction(tabletMetadata, metaJob.getJob().getKind(), jobFiles, ctx,
+                manager.getSteadyTime())) {
+              return null;
+            }
 
-        var ecm = createExternalCompactionMetadata(metaJob.getJob(), jobFiles, tabletMetadata,
-            compactorAddress, externalCompactionId);
-
-        // any data that is read from the tablet to make a decision about if it can compact or not
-        // must be checked for changes in the conditional mutation.
-        var tabletMutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation()
-            .requireFiles(jobFiles).requireNotCompacting(jobFiles);
-        if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
-          // For system compactions the user compaction requested column is examined when deciding
-          // if a compaction can start so need to check for changes to this column.
-          tabletMutator.requireSame(tabletMetadata, SELECTED, USER_COMPACTION_REQUESTED);
-        } else {
-          tabletMutator.requireSame(tabletMetadata, SELECTED);
-        }
+            var ecm = createExternalCompactionMetadata(metaJob.getJob(), jobFiles, tabletMetadata,
+                compactorAddress, externalCompactionId);
+
+            // any data that is read from the tablet to make a decision about if it can compact or
+            // not
+            // must be checked for changes in the conditional mutation.
+            var tabletMutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation()
+                .requireFiles(jobFiles).requireNotCompacting(jobFiles);
+            if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
+              // For system compactions the user compaction requested column is examined when
+              // deciding
+              // if a compaction can start so need to check for changes to this column.
+              tabletMutator.requireSame(tabletMetadata, SELECTED, USER_COMPACTION_REQUESTED);
+            } else {
+              tabletMutator.requireSame(tabletMetadata, SELECTED);
+            }
 
-        if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
-          var selectedFiles = tabletMetadata.getSelectedFiles();
-          var reserved = getFilesReservedBySelection(tabletMetadata, manager.getSteadyTime(), ctx);
-
-          // If there is a selectedFiles column, and the reserved set is empty this means that
-          // either no user jobs were completed yet or the selection expiration time has passed
-          // so the column is eligible to be deleted so a system job can run instead
-          if (selectedFiles != null && reserved.isEmpty()
-              && !Collections.disjoint(jobFiles, selectedFiles.getFiles())) {
-            LOG.debug("Deleting user compaction selected files for {} {}", extent,
-                externalCompactionId);
-            tabletMutator.deleteSelectedFiles();
-          }
-        }
+            if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
+              var selectedFiles = tabletMetadata.getSelectedFiles();
+              var reserved =
+                  getFilesReservedBySelection(tabletMetadata, manager.getSteadyTime(), ctx);
+
+              // If there is a selectedFiles column, and the reserved set is empty this means that
+              // either no user jobs were completed yet or the selection expiration time has passed
+              // so the column is eligible to be deleted so a system job can run instead
+              if (selectedFiles != null && reserved.isEmpty()
+                  && !Collections.disjoint(jobFiles, selectedFiles.getFiles())) {
+                LOG.debug("Deleting user compaction selected files for {} {}", extent,
+                    externalCompactionId);
+                tabletMutator.deleteSelectedFiles();
+              }
+            }
 
-        tabletMutator.putExternalCompaction(externalCompactionId, ecm);
-        tabletMutator.submit(tm -> tm.getExternalCompactions().containsKey(externalCompactionId));
+            tabletMutator.putExternalCompaction(externalCompactionId, ecm);
+            tabletMutator
+                .submit(tm -> tm.getExternalCompactions().containsKey(externalCompactionId));
 
-        var result = tabletsMutator.process().get(extent);
+            var result = tabletsMutator.process().get(extent);
 
-        if (result.getStatus() == Ample.ConditionalResult.Status.ACCEPTED) {
-          return ecm;
-        } else {
-          tabletMetadata = result.readMetadata();
+            if (result.getStatus() == Ample.ConditionalResult.Status.ACCEPTED) {
+              return ecm;
+            } else {
+              tabletMetadata = result.readMetadata();
+            }
+          }
+
+          retry.useRetry();
+          try {
+            retry.waitForNextAttempt(LOG,
+                "Reserved compaction for " + metaJob.getTabletMetadata().getExtent());
+          } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+          }
         }
-      }
 
-      retry.useRetry();
-      try {
-        retry.waitForNextAttempt(LOG,
-            "Reserved compaction for " + metaJob.getTabletMetadata().getExtent());
-      } catch (InterruptedException e) {
-        throw new RuntimeException(e);
+        return null;
+      } finally {
+        Preconditions.checkState(activeCompactorReservationRequest.remove(compactorAddress));
       }
     }
+  }
+
+  protected CompactionMetadata reserveCompaction(CompactionJobQueues.MetaJob metaJob,
+      String compactorAddress, ExternalCompactionId externalCompactionId) {
+
+    if (activeCompactorReservationRequest.contains(compactorAddress)) {

Review Comment:
   Yeah if something really unusual is going on that Precondition check would catch it.  Under normal case of compactor retrying the first check will catch it.  It would probably be good add some info to the preconditions check in case it does fire.



-- 
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] limits memory and cpu used by compaction reservation request [accumulo]

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


##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -528,82 +551,138 @@ protected CompactionMetadata createExternalCompactionMetadata(CompactionJob job,
 
   }
 
-  protected CompactionMetadata reserveCompaction(CompactionJobQueues.MetaJob metaJob,
-      String compactorAddress, ExternalCompactionId externalCompactionId) {
+  private class ReserveCompactionTask implements Supplier<CompactionMetadata> {
+
+    // Use a soft reference for this in case free memory gets low while this is sitting in the queue
+    // waiting to process. This object can contain the tablets list of files and if there are lots
+    // of tablet with lots of files then that could start to cause memory problems.
+    private final SoftReference<CompactionJobQueues.MetaJob> metaJobRef;
+    private final String compactorAddress;
+    private final ExternalCompactionId externalCompactionId;
+
+    private ReserveCompactionTask(CompactionJobQueues.MetaJob metaJob, String compactorAddress,
+        ExternalCompactionId externalCompactionId) {
+      Preconditions.checkArgument(metaJob.getJob().getKind() == CompactionKind.SYSTEM
+          || metaJob.getJob().getKind() == CompactionKind.USER);
+      this.metaJobRef = new SoftReference<>(Objects.requireNonNull(metaJob));
+      this.compactorAddress = Objects.requireNonNull(compactorAddress);
+      this.externalCompactionId = Objects.requireNonNull(externalCompactionId);
+      Preconditions.checkState(activeCompactorReservationRequest.add(compactorAddress));
+    }
 
-    Preconditions.checkArgument(metaJob.getJob().getKind() == CompactionKind.SYSTEM
-        || metaJob.getJob().getKind() == CompactionKind.USER);
+    @Override
+    public CompactionMetadata get() {
+      try {
+        var metaJob = metaJobRef.get();
+        if (metaJob == null) {
+          LOG.warn("Compaction reservation request for {} {} was garbage collected.",
+              compactorAddress, externalCompactionId);
+          return null;
+        }
 
-    var tabletMetadata = metaJob.getTabletMetadata();
+        var tabletMetadata = metaJob.getTabletMetadata();
 
-    var jobFiles = metaJob.getJob().getFiles().stream().map(CompactableFileImpl::toStoredTabletFile)
-        .collect(Collectors.toSet());
+        var jobFiles = metaJob.getJob().getFiles().stream()
+            .map(CompactableFileImpl::toStoredTabletFile).collect(Collectors.toSet());
 
-    Retry retry = Retry.builder().maxRetries(5).retryAfter(Duration.ofMillis(100))
-        .incrementBy(Duration.ofMillis(100)).maxWait(Duration.ofSeconds(10)).backOffFactor(1.5)
-        .logInterval(Duration.ofMinutes(3)).createRetry();
+        Retry retry = Retry.builder().maxRetries(5).retryAfter(Duration.ofMillis(100))
+            .incrementBy(Duration.ofMillis(100)).maxWait(Duration.ofSeconds(10)).backOffFactor(1.5)
+            .logInterval(Duration.ofMinutes(3)).createRetry();
 
-    while (retry.canRetry()) {
-      try (var tabletsMutator = ctx.getAmple().conditionallyMutateTablets()) {
-        var extent = metaJob.getTabletMetadata().getExtent();
+        while (retry.canRetry()) {
+          try (var tabletsMutator = ctx.getAmple().conditionallyMutateTablets()) {
+            var extent = metaJob.getTabletMetadata().getExtent();
 
-        if (!canReserveCompaction(tabletMetadata, metaJob.getJob().getKind(), jobFiles, ctx,
-            manager.getSteadyTime())) {
-          return null;
-        }
+            if (!canReserveCompaction(tabletMetadata, metaJob.getJob().getKind(), jobFiles, ctx,
+                manager.getSteadyTime())) {
+              return null;
+            }
 
-        var ecm = createExternalCompactionMetadata(metaJob.getJob(), jobFiles, tabletMetadata,
-            compactorAddress, externalCompactionId);
-
-        // any data that is read from the tablet to make a decision about if it can compact or not
-        // must be checked for changes in the conditional mutation.
-        var tabletMutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation()
-            .requireFiles(jobFiles).requireNotCompacting(jobFiles);
-        if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
-          // For system compactions the user compaction requested column is examined when deciding
-          // if a compaction can start so need to check for changes to this column.
-          tabletMutator.requireSame(tabletMetadata, SELECTED, USER_COMPACTION_REQUESTED);
-        } else {
-          tabletMutator.requireSame(tabletMetadata, SELECTED);
-        }
+            var ecm = createExternalCompactionMetadata(metaJob.getJob(), jobFiles, tabletMetadata,
+                compactorAddress, externalCompactionId);
+
+            // any data that is read from the tablet to make a decision about if it can compact or
+            // not
+            // must be checked for changes in the conditional mutation.
+            var tabletMutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation()
+                .requireFiles(jobFiles).requireNotCompacting(jobFiles);
+            if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
+              // For system compactions the user compaction requested column is examined when
+              // deciding
+              // if a compaction can start so need to check for changes to this column.
+              tabletMutator.requireSame(tabletMetadata, SELECTED, USER_COMPACTION_REQUESTED);
+            } else {
+              tabletMutator.requireSame(tabletMetadata, SELECTED);
+            }
 
-        if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
-          var selectedFiles = tabletMetadata.getSelectedFiles();
-          var reserved = getFilesReservedBySelection(tabletMetadata, manager.getSteadyTime(), ctx);
-
-          // If there is a selectedFiles column, and the reserved set is empty this means that
-          // either no user jobs were completed yet or the selection expiration time has passed
-          // so the column is eligible to be deleted so a system job can run instead
-          if (selectedFiles != null && reserved.isEmpty()
-              && !Collections.disjoint(jobFiles, selectedFiles.getFiles())) {
-            LOG.debug("Deleting user compaction selected files for {} {}", extent,
-                externalCompactionId);
-            tabletMutator.deleteSelectedFiles();
-          }
-        }
+            if (metaJob.getJob().getKind() == CompactionKind.SYSTEM) {
+              var selectedFiles = tabletMetadata.getSelectedFiles();
+              var reserved =
+                  getFilesReservedBySelection(tabletMetadata, manager.getSteadyTime(), ctx);
+
+              // If there is a selectedFiles column, and the reserved set is empty this means that
+              // either no user jobs were completed yet or the selection expiration time has passed
+              // so the column is eligible to be deleted so a system job can run instead
+              if (selectedFiles != null && reserved.isEmpty()
+                  && !Collections.disjoint(jobFiles, selectedFiles.getFiles())) {
+                LOG.debug("Deleting user compaction selected files for {} {}", extent,
+                    externalCompactionId);
+                tabletMutator.deleteSelectedFiles();
+              }
+            }
 
-        tabletMutator.putExternalCompaction(externalCompactionId, ecm);
-        tabletMutator.submit(tm -> tm.getExternalCompactions().containsKey(externalCompactionId));
+            tabletMutator.putExternalCompaction(externalCompactionId, ecm);
+            tabletMutator
+                .submit(tm -> tm.getExternalCompactions().containsKey(externalCompactionId));
 
-        var result = tabletsMutator.process().get(extent);
+            var result = tabletsMutator.process().get(extent);
 
-        if (result.getStatus() == Ample.ConditionalResult.Status.ACCEPTED) {
-          return ecm;
-        } else {
-          tabletMetadata = result.readMetadata();
+            if (result.getStatus() == Ample.ConditionalResult.Status.ACCEPTED) {
+              return ecm;
+            } else {
+              tabletMetadata = result.readMetadata();
+            }
+          }
+
+          retry.useRetry();
+          try {
+            retry.waitForNextAttempt(LOG,
+                "Reserved compaction for " + metaJob.getTabletMetadata().getExtent());
+          } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+          }
         }
-      }
 
-      retry.useRetry();
-      try {
-        retry.waitForNextAttempt(LOG,
-            "Reserved compaction for " + metaJob.getTabletMetadata().getExtent());
-      } catch (InterruptedException e) {
-        throw new RuntimeException(e);
+        return null;
+      } finally {
+        Preconditions.checkState(activeCompactorReservationRequest.remove(compactorAddress));
       }
     }
+  }
+
+  protected CompactionMetadata reserveCompaction(CompactionJobQueues.MetaJob metaJob,
+      String compactorAddress, ExternalCompactionId externalCompactionId) {
+
+    if (activeCompactorReservationRequest.contains(compactorAddress)) {

Review Comment:
   I was trying to think if there would be a race condition here if the thread pool was configured to be greater than 1 to handle multiple requests. I don't think there is because the same compactor should not be sending in multiple requests at the exact same time, so if there was multiple attempts and that leads to one of the Preconditions.checkState() checks on the add/remove failing then that is probably a good thing as it means there is an issue.



##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -528,82 +551,138 @@ protected CompactionMetadata createExternalCompactionMetadata(CompactionJob job,
 
   }
 
-  protected CompactionMetadata reserveCompaction(CompactionJobQueues.MetaJob metaJob,
-      String compactorAddress, ExternalCompactionId externalCompactionId) {
+  private class ReserveCompactionTask implements Supplier<CompactionMetadata> {
+
+    // Use a soft reference for this in case free memory gets low while this is sitting in the queue
+    // waiting to process. This object can contain the tablets list of files and if there are lots
+    // of tablet with lots of files then that could start to cause memory problems.
+    private final SoftReference<CompactionJobQueues.MetaJob> metaJobRef;

Review Comment:
   This is a neat way to help with the memory issue. I haven't looked but I am curious if there's some way to get statistics from the JVM to find out how often this reference is being freed that could be useful to monitor this. It may not be necessary though as the just watching total memory usage is probably good enough as this reference should only get cleared when low in memory.



-- 
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] limits memory and cpu used by compaction reservation request [accumulo]

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


-- 
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] limits memory and cpu used by compaction reservation request [accumulo]

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


##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -528,82 +551,138 @@ protected CompactionMetadata createExternalCompactionMetadata(CompactionJob job,
 
   }
 
-  protected CompactionMetadata reserveCompaction(CompactionJobQueues.MetaJob metaJob,
-      String compactorAddress, ExternalCompactionId externalCompactionId) {
+  private class ReserveCompactionTask implements Supplier<CompactionMetadata> {
+
+    // Use a soft reference for this in case free memory gets low while this is sitting in the queue
+    // waiting to process. This object can contain the tablets list of files and if there are lots
+    // of tablet with lots of files then that could start to cause memory problems.
+    private final SoftReference<CompactionJobQueues.MetaJob> metaJobRef;

Review Comment:
   This was trying to deal with a problem I have been seeing while testing using the [FlakyBulkBatchWriter](https://github.com/apache/accumulo-testing/blob/807b62c01c4aae422a155be549fbf19664157b9c/src/main/java/org/apache/accumulo/testing/continuous/FlakyBulkBatchWriter.java#L55) in accumulo-testing that will sometimes submits large numbers of files for a tablet.   This can cause lots of memory pressure on the manager when lots of tablet have lots of files and the manager is trying to keep a lot of that in memory.  For this case its really hard to reason about what will be on the thread pool queue and how much memory it is using.  So decided to use a soft refernce for now, but I am uncertain about it.   Opened #5188 and added comment pointing to that issue in this PR.  I think if #5188 were implemented that this soft reference could be removed.  Would still want to restrict compactors from having multiple request queued/running.  I think if compactors can only have one thing queued f
 or reservation and the queued data is only compaction jobs and no tablet metadata, then the memory usage would be unlikely to cause a problem.



-- 
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] limits memory and cpu used by compaction reservation request [accumulo]

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


##########
server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java:
##########
@@ -528,82 +551,138 @@ protected CompactionMetadata createExternalCompactionMetadata(CompactionJob job,
 
   }
 
-  protected CompactionMetadata reserveCompaction(CompactionJobQueues.MetaJob metaJob,
-      String compactorAddress, ExternalCompactionId externalCompactionId) {
+  private class ReserveCompactionTask implements Supplier<CompactionMetadata> {
+
+    // Use a soft reference for this in case free memory gets low while this is sitting in the queue
+    // waiting to process. This object can contain the tablets list of files and if there are lots
+    // of tablet with lots of files then that could start to cause memory problems.
+    private final SoftReference<CompactionJobQueues.MetaJob> metaJobRef;

Review Comment:
   This was trying to deal with a problem I have been seeing while testing using the [FlakyBulkBatchWriter](https://github.com/apache/accumulo-testing/blob/807b62c01c4aae422a155be549fbf19664157b9c/src/main/java/org/apache/accumulo/testing/continuous/FlakyBulkBatchWriter.java#L55) in accumulo-testing that will sometimes submit a large numbers of files for a tablet.   This can cause lots of memory pressure on the manager when lots of tablet have lots of files and the manager is trying to keep a lot of that in memory.  For this case its really hard to reason about what will be on the thread pool queue and how much memory it is using.  So decided to use a soft refernce for now, but I am uncertain about it.   Opened #5188 and added comment pointing to that issue in this PR.  I think if #5188 were implemented that this soft reference could be removed.  Would still want to restrict compactors from having multiple request queued/running.  I think if compactors can only have one thing queued 
 for reservation and the queued data is only compaction jobs and no tablet metadata, then the memory usage would be unlikely to cause a problem.



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