cshannon commented on code in PR #4480:
URL: https://github.com/apache/accumulo/pull/4480#discussion_r1605548685
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/CompactionJobGenerator.java:
##########
@@ -205,9 +209,22 @@ private Collection<CompactionJob>
planCompactions(CompactionServiceId serviceId,
// remove any files that are in active compactions
tablet.getExternalCompactions().values().stream().flatMap(ecm ->
ecm.getJobFiles().stream())
.forEach(tmpFiles::remove);
- // remove any files that are selected
- if (tablet.getSelectedFiles() != null) {
- tmpFiles.keySet().removeAll(tablet.getSelectedFiles().getFiles());
+ // remove any files that are selected and the user compaction has
completed
+ // at least 1 job, otherwise we can keep the files
+ var selectedFiles = tablet.getSelectedFiles();
+
+ if (selectedFiles != null) {
+ long selectedExpirationDuration =
+
ConfigurationTypeHelper.getTimeInMillis(env.getConfiguration(tablet.getTableId())
+
.get(Property.TABLE_COMPACTION_SELECTION_EXPIRATION.getKey()));
+
+ // If jobs are completed, or selected time has not expired, the
remove
+ // from the candidate list otherwise we can cancel the selection
+ if (selectedFiles.getCompletedJobs() > 0 ||
(selectedFiles.getSelectedTime() != null
Review Comment:
Yeah it should not be null, I am wondering if we should add a notNull or
precondition check in the constructor for SelectedFiles
[here](https://github.com/apache/accumulo/pull/4480/files#diff-a1c7c614b8a2a9952143f2f70fd3ff199c433d8d0c429b834effb70f5c5326afR69)
I'm also wondering if we should validate anything else such as completedJobs
>= 0 and that FateId is not null
--
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]
