cryptoe commented on code in PR #16328:
URL: https://github.com/apache/druid/pull/16328#discussion_r1579132807
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java:
##########
@@ -1523,4 +1523,10 @@ public CompactionTuningConfig
withPartitionsSpec(PartitionsSpec partitionsSpec)
);
}
}
+
+ @Override
+ public List<String> getLookupsToLoad()
+ {
+ return Collections.emptyList();
Review Comment:
We can use lookup in an aggregation function so I think this should not be
done.
##########
services/src/main/java/org/apache/druid/cli/CliPeon.java:
##########
@@ -356,6 +356,14 @@ public String getTaskIDFromTask(final Task task)
{
return task.getId();
}
+
+ @Provides
+ @LazySingleton
+ @Named(DataSourceTaskIdHolder.LOOKUPS_TO_LOAD_FOR_TASK)
+ public List<String> getLookupsToLoad(final Task task)
Review Comment:
How does it work for indexers and mm less task system. Does that code go via
cliPeon ? Maybe you can test locally and see how it goes.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/Task.java:
##########
@@ -331,4 +333,10 @@ static TaskInfo<TaskIdentifier, TaskStatus>
toTaskIdentifierInfo(TaskInfo<Task,
taskInfo.getTask().getMetadata()
);
}
+
+ @Nullable
+ default List<String> getLookupsToLoad()
Review Comment:
Keys with null will be difficult to pass via context.
Can we use a pojo instead here ?
##########
server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java:
##########
@@ -373,10 +375,18 @@ private void takeSnapshot(Map<String,
LookupExtractorFactoryContainer> lookupMap
}
}
- private void loadAllLookupsAndInitStateRef()
+ /**
+ * Load a set of lookups based on the injected value in {@link
DataSourceTaskIdHolder#getLookupsToLoad()}.
+ */
+ private void loadLookupsAndInitStateRef()
{
List<LookupBean> lookupBeanList = getLookupsList();
if (lookupBeanList != null) {
+ final List<String> lookupsToLoad =
lookupListeningAnnouncerConfig.getLookupsToLoad();
Review Comment:
This logic should be inside the Pojo which getLookupsToLoadReturns(). It
should tell you
* If I have lookups to load then load which lookups.
* I donot know, so load all of them
* Do not load lookups.
--
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]
