-
Notifications
You must be signed in to change notification settings - Fork 988
Caching QueryPlan Results #3023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
85a5b9b to
819c3f1
Compare
|
@vdegans Wow! This is an impressive first contribution to Drill! Before we start review, would you please do a clean rebase on |
5ac8803 to
c49c581
Compare
|
@cgivre Thanks! I fixed the rebase. |
Thanks. Before I start review I had a few questions:
What I'm getting at here is would it make sense to have global settings which you already have, but then also give the user the ability to set custom settings for specific plugins if they wanted to do so. I genuinely don't know if that is worth the effort or not. I could imagine this being more of an issue with data where the schema could change--MongoDB or JSON files for instance--and queries like SELECT * might bring back different data every time you run them. |
|
Thanks, that’s a great point. I agree that giving users control over caching at the per-plugin level makes sense. Some plugins, especially ones where the underlying data might change frequently, like MongoDB or JSON files, could benefit from having caching disabled or customized independently from the global settings. I think supporting per-plugin cache configuration would give users the flexibility to optimize caching behavior for their specific use cases and improve the overall user experience. |
|
@vdegans Hi Vincent, Any update? |
|
Hi @cgivre, I was thinking about the suggestion for a setting to enable/disable caching per plugin and I got stuck on the idea of when to cache and when not to. I didn't get much time to look at the code yet, but I would like to hear your thoughts about the settings per plugin. |
The whole idea of partially cached query plans is extremely tricky. I think but could be wrong but there may have been some work on that from the Calcite team at one point. In any event, my suggestion would be to start simple. Let's get all the unit tests to pass and just start with simple caching. IE: exact query match. Once that's done and merged, we can iterate and find improvements. |
|
@vdegans You should rebase on current master. I think that will solve the size limit issue you're running into. |
|
I think caffeine might cause this, should I add caffeine to the exclude list? |
You can either exclude caffeine or bump up the max size. Either is fine. |
|
Locally this seems to have fixed the issue |
DRILL-8529: Caching QueryPlan Results
Description
Implements a caching mechanism for query plans and transformations, to shorten the prepare phase.
Documentation
The cache behavior can be customized via drill-override.conf
At runtime, caching can also be toggled with:
planner.cache.enable (true = enabled, false = disabled)Testing