You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "pranavbhole (via GitHub)" <gi...@apache.org> on 2023/08/10 16:36:24 UTC
[I] Lookup updates that leads inconsistencies (druid)
pranavbhole opened a new issue, #14796:
URL: https://github.com/apache/druid/issues/14796
Please provide a detailed title (e.g. "Broker crashes when using TopN query with Bound filter" instead of just "Broker crashes").
### Affected Version: Master branch
The Druid version where the problem was encountered.
### Description
Lookup instantiation mainly works on broadcasting two notice, AddNotice, DropNotice. When we create fresh lookup, we issue AddNotice and when we update the existing lookup we issue the AddNotice and DropNotice.
We have been seeing the inconsistent lookup state esp with JDBC in the cluster that caused by the following scenario, I was able to reproduce this in local as well.
1. Post create jdbc lookup request, assuming that jdbc server is consistent, it loads the lookup and ready to serve.
2. Next pooling fails due to jdbc server not available/ some issues with lookup jdbc connection but old lookup is still serving good.
3. Druid User tries to update the lookup json and post update request and old good lookup is also killed and query fails with cache state CACHE_NOT_INITIALIZED.
Bug is the behavior of Step 3, currently we blindly issue the [AddNotice for new lookup and DropNotice](https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/query/lookup/LookupListeningResource.java#L96) for old lookup without making sure that new lookup cache population is successful.
There is another bug at Step 2 where we do have resiliency in dealing with JDBC handle lookups, [we do not retry handle on transient errors](https://github.com/apache/druid/blob/master/extensions-core/lookups-cached-global/src/main/java/org/apache/druid/server/lookup/namespace/JdbcCacheGenerator.java#L134C18-L134C27). If transient error occurs then we need to wait for next pooling period to reach and populate lookups. In this time, lookup's state remains CACHE_NOT_INITIALIZED if we have no successful load previously.
**Proposals for the addressing Step 2 and Step 3 bugs:**
1. Step 2: Create resilient handle and retry on transient error.
2. Step 3: Delay the Drop notice execution until AddNotice loads the lookup on the current node, and make sure that we have one latest lookup loaded successfully and good to drop the previous one. This can be done by starting the schedule executor thread here https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java#L675 that can execute it after Delay D, N times and it can get the latest stateRef from LookupReferenceManager and make sure that latest Ref cache is loaded successfully (Also make sure that LookupExtractorFactoryContainer that we are trying to remove is not same as current stateRef container).
--
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: commits-unsubscribe@druid.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [I] Lookup updates that leads inconsistencies (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis closed issue #14796: Lookup updates that leads inconsistencies
URL: https://github.com/apache/druid/issues/14796
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [I] Lookup updates that leads inconsistencies (druid)
Posted by "pranavbhole (via GitHub)" <gi...@apache.org>.
pranavbhole commented on issue #14796:
URL: https://github.com/apache/druid/issues/14796#issuecomment-1673957032
Thank you @clintropolis, Currently, Load Notice Handler tries to instantiate and start the new LookupExtractorFactoryContainer and start the extractor factory. Start is non blocking call, it just start the scheduler today and does not wait for the actual cache to load.
Idea it to change the LoadNotice and wait for actual first cache to load using firstCacheLoadLatch in CacheScheduler,
LoadNotice Handler:
1. If loadNotice is first time load then just go ahead and start container and do not wait for first cache load.
1. If it is Load Notice with update call, it will instantiate the new LookupExtractorFactoryContainer and wait for firstCacheLoadLatch (Need to expose the method at LookupExtractorFactory), We can not indefinitely to load the firstCache, we need new namespace config with default value would be 10 mins , `lookupLoadTimeout = 10` mins.
2. After cache load is successful, it will update the stateRef with new lookup state and with new container.
Drop Notice Handler:
1. If drop notice is final lookup drop call then go ahead drop it directly
2. Once we receive the drop notice from update flow, we will peek into reference manager's stateRef and make sure that corresponding new container for given lookup is loaded (using wrapper method that check firstCacheLoadLatch in Cache Scheduler)
3. After new container is loaded successfully then we will go ahead remove the old container from lookupMap and clean up old container
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [I] Lookup updates that leads inconsistencies (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on issue #14796:
URL: https://github.com/apache/druid/issues/14796#issuecomment-1673821303
Seems reasonable to make sure the new lookup is good before dropping the old one, though I'm not entirely certain how to check this.. what do you have in mind? It would be nice to make this more "proposal-ish" and include what changes are actually needed once you have a design. As for ideas (in case you need any) i guess we could make sure the `LookupExtractorFactory` started successfully, or maybe does it need a new method added to the interface like an `isInitialized` or something else? Also, if the lookup implementation doesn't load everything in heap like the built-in ones do, how do we know its in a good state in that case? With a new method I suppose those lookups could just do whatever made sense to make sure they can do lookup stuff.
>Step 3: Delay the Drop notice execution until AddNotice loads the lookup on the current node, and make sure that we have one latest lookup loaded successfully and good to drop the previous one. This can be done by starting the schedule executor thread here https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java#L675 that can execute it after Delay D, N times and it can get the latest stateRef from LookupReferenceManager and make sure that latest Ref cache is loaded successfully (Also make sure that LookupExtractorFactoryContainer that we are trying to remove is not same as current stateRef container).
Will we need to split up the 'drop because i got a new version' from the 'drop because we no longer want this lookup loaded anywhere' cases? Or how do you envision that working?
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [I] Lookup updates that leads inconsistencies (druid)
Posted by "sixtus (via GitHub)" <gi...@apache.org>.
sixtus commented on issue #14796:
URL: https://github.com/apache/druid/issues/14796#issuecomment-1739083428
Just wondering what the status of this is. As I am seeing a LOT of CACHE_NOT_INITIALIZED errors in my logs, I was hoping this is the solution, however it's far from applying cleanly to 27.0.0.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org