gianm commented on code in PR #19074:
URL: https://github.com/apache/druid/pull/19074#discussion_r2900980148
##########
multi-stage-query/src/test/java/org/apache/druid/msq/indexing/DurableStorageCleanerTest.java:
##########
@@ -127,10 +172,109 @@ public void testGetSchedule()
DurableStorageCleanerConfig cleanerConfig = new
DurableStorageCleanerConfig();
cleanerConfig.delaySeconds = 10L;
cleanerConfig.enabled = true;
- DurableStorageCleaner durableStorageCleaner = new
DurableStorageCleaner(cleanerConfig, (temp) ->
NilStorageConnector.getInstance(), null);
+ DurableStorageCleaner durableStorageCleaner = new DurableStorageCleaner(
+ cleanerConfig,
+ (temp) -> NilStorageConnector.getInstance(),
+ null,
+ null
+ );
DutySchedule schedule = durableStorageCleaner.getSchedule();
Assert.assertEquals(cleanerConfig.delaySeconds * 1000,
schedule.getPeriodMillis());
Assert.assertEquals(cleanerConfig.delaySeconds * 1000,
schedule.getInitialDelayMillis());
}
+
+ public static class TestTask implements Task
Review Comment:
Could this be a NoopTask?
##########
processing/src/main/java/org/apache/druid/frame/util/DurableStorageUtils.java:
##########
@@ -52,6 +52,23 @@ private static String
getQueryResultsControllerDirectory(final String controller
);
}
+ @Nullable
+ private static String getControllerTaskId(final String path)
+ {
+ Iterator<String> elementsIterator = SPLITTER.split(path).iterator();
+ List<String> elements = ImmutableList.copyOf(elementsIterator);
+ if (elements.size() < 2) {
+ return null;
+ }
+ if (!DurableStorageUtils.QUERY_RESULTS_DIR.equals(elements.get(0))) {
+ return null;
+ }
+ if (!elements.get(1).startsWith("controller_")) {
+ return null;
+ }
+ return elements.get(1).substring(11);
Review Comment:
Better to clarify what `11` means, such as by using a constant like `private
static final int CONTROLLER_PREFIX_LENGTH = "controller_".length()`
##########
multi-stage-query/src/main/java/org/apache/druid/msq/indexing/cleaner/DurableStorageCleaner.java:
##########
@@ -52,17 +55,20 @@ public class DurableStorageCleaner implements OverlordDuty
private final DurableStorageCleanerConfig config;
private final StorageConnector storageConnector;
private final Provider<TaskMaster> taskMasterProvider;
+ private final TaskStorage taskStorage;
@Inject
public DurableStorageCleaner(
final DurableStorageCleanerConfig config,
final @MultiStageQuery StorageConnectorProvider storageConnectorProvider,
- @JacksonInject final Provider<TaskMaster> taskMasterProvider
+ @JacksonInject final Provider<TaskMaster> taskMasterProvider,
Review Comment:
I don't think the `@JacksonInject` is doing anything here, since this is not
read using Jackson. I see it was already there, but since you're editing the
constructor anyway, see if things still work if you remove it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
