clintropolis commented on code in PR #14806:
URL: https://github.com/apache/druid/pull/14806#discussion_r1295112280
##########
server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java:
##########
@@ -638,18 +664,44 @@ public void handle(Map<String,
LookupExtractorFactoryContainer> lookupMap) throw
e -> true,
startRetries
);
-
- old = lookupMap.put(lookupName, lookupExtractorFactoryContainer);
-
- LOG.debug("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);
- }
+ if
(lookupExtractorFactoryContainer.getLookupExtractorFactory().isCacheLoaded()) {
+ old = lookupMap.put(lookupName, lookupExtractorFactoryContainer);
+ LOG.debug("Loaded lookup [%s] with spec [%s].", lookupName,
lookupExtractorFactoryContainer);
+ manager.dropContainer(old, lookupName);
+ return;
}
+ manager.submitAsyncLookupTask(() -> {
+ try {
+ /*
+ Retry startRetries times and wait for first cache to load for new
container,
+ if loaded then kill old container and start serving from new one.
+ If new lookupExtractorFactoryContainer has errors in loading, kill the
new container and do not remove the old container
+ */
+ RetryUtils.retry(() -> {
+
lookupExtractorFactoryContainer.getLookupExtractorFactory().awaitToInitialise();
+ return null;
+ }, e -> true,
Review Comment:
i dont' think this needs to be a blocker, but it would be nice if we could
distinguish errors which are retryable from errors which are not, but i imagine
there is too much variety in the errors which could be thrown for us to do this
and we would probably need to standardize the error states for lookup
implementations to do this effectively.
--
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]
