yuanlihan commented on a change in pull request #10421:
URL: https://github.com/apache/druid/pull/10421#discussion_r509937891
##########
File path:
server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java
##########
@@ -587,6 +594,43 @@ private StringFullResponseHolder
fetchLookupsForTier(String tier) throws Interru
void handle(Map<String, LookupExtractorFactoryContainer> lookupMap) throws
Exception;
}
+ private static class StatusNotice implements Notice
+ {
+ private final LookupReferencesManager manager;
+ private final String lookupName;
+ private final LookupExtractorFactoryContainer
lookupExtractorFactoryContainer;
+
+ public StatusNotice(
+ LookupReferencesManager manager,
+ String lookupName,
+ LookupExtractorFactoryContainer lookupExtractorFactoryContainer
+ )
+ {
+ this.manager = manager;
+ this.lookupName = lookupName;
+ this.lookupExtractorFactoryContainer = lookupExtractorFactoryContainer;
+ }
+
+ @Override
+ public void handle(Map<String, LookupExtractorFactoryContainer> lookupMap)
+ {
+ if
(lookupExtractorFactoryContainer.getLookupExtractorFactory().isReady()) {
+ LookupExtractorFactoryContainer old = lookupMap.put(lookupName,
lookupExtractorFactoryContainer);
+
+ LOG.info("Loaded lookup [%s] with spec [%s].", lookupName,
lookupExtractorFactoryContainer);
+
+ if (old != null) {
+ if (!old.getLookupExtractorFactory().destroy()) {
+ throw new ISE("destroy method returned false for lookup
[%s]:[%s]", lookupName, old);
+ }
+ }
+ } else {
+ LOG.info("Loading lookup [%s] with spec [%s].", lookupName,
lookupExtractorFactoryContainer);
+ manager.addNotice(new StatusNotice(manager, lookupName,
lookupExtractorFactoryContainer));
Review comment:
If there is a new cached lookup being initialised, then this lookup will
not be visible to queries. And queries will failed with `Lookup [xxx] not
found` error, see
https://github.com/apache/druid/blob/1b9a8c46874520d22b65573eeabfa3a62e16a5e0/processing/src/main/java/org/apache/druid/query/lookup/RegisteredLookupExtractionFn.java#L151
This PR only tries to improve the use case of updating an existing lookup
with higher version. And I agree that this
[proposal](https://github.com/apache/druid/issues/10294) will improve these use
cases on a general level.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
