-
Notifications
You must be signed in to change notification settings - Fork 272
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
Distributed query plan cache keys include the Router version number #6406
Conversation
If you have enabled [Distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), starting with this release and going forward you should anticipate additional cache regeneration cost when updating between any Router versions. More often than not, an Apollo Router release may contain changes that affect what query plans are generated or how they’re represented. To avoid using outdated entries from distributed cache, the cache key includes a counter that was manually incremented with relevant data structure or algorithm changes. Instead the cache key now includes the Router version number, so that different versions will always use separate cache entries.
✅ Docs Preview ReadyNo new or changed pages found. |
This comment has been minimized.
This comment has been minimized.
CI performance tests
|
@@ -0,0 +1,7 @@ | |||
### Distributed query plan cache keys include the Router version number ([PR #6406](https://github.com/apollographql/router/pull/6406)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this (though filename prefix) in the "Configuration" section of the changelog, but none of the options in xtask changeset create
seemed like a great fit.
CHANGELOG.md
has some > [!IMPORTANT]
sections, is that done manually when editing for a release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do that manually in the release PR, since it has to go in the top of the changelog
@@ -0,0 +1,7 @@ | |||
### Distributed query plan cache keys include the Router version number ([PR #6406](https://github.com/apollographql/router/pull/6406)) | |||
|
|||
If you have enabled [Distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), starting with this release and going forward you should anticipate additional cache regeneration cost when updating between any Router versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this discuss the possibility a staged rollout to start warming up new cache entries while the previous version still serves most traffic? I didn’t want to sound like "draw the rest of the owl"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "drawing the rest of the owl" can be added to distributed query plan caching docs themselves. It will be a nice addition, and people can search the docs with a bit more ease than a changelog.
|
||
If you have enabled [Distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), starting with this release and going forward you should anticipate additional cache regeneration cost when updating between any Router versions. | ||
|
||
More often than not, an Apollo Router release may contain changes that affect what query plans are generated or how they’re represented. To avoid using outdated entries from distributed cache, the cache key includes a counter that was manually incremented with relevant data structure or algorithm changes. Instead the cache key now includes the Router version number, so that different versions will always use separate cache entries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this doesn’t need to mention CACHE_KEY_VERSION
v.s. FEDERATION_VERSION
so I tried to pick a relevant level of detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would put this paragraph first. The point is that almost every release has had a cache key change, which comes with the additional cache regeneration cost. If people read the first paragraph, they'll think this is more than what's currently happening, but it effectively isn't.
@@ -0,0 +1,7 @@ | |||
### Distributed query plan cache keys include the Router version number ([PR #6406](https://github.com/apollographql/router/pull/6406)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do that manually in the release PR, since it has to go in the top of the changelog
@@ -0,0 +1,7 @@ | |||
### Distributed query plan cache keys include the Router version number ([PR #6406](https://github.com/apollographql/router/pull/6406)) | |||
|
|||
If you have enabled [Distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), starting with this release and going forward you should anticipate additional cache regeneration cost when updating between any Router versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "drawing the rest of the owl" can be added to distributed query plan caching docs themselves. It will be a nice addition, and people can search the docs with a bit more ease than a changelog.
|
||
If you have enabled [Distributed query plan caching](https://www.apollographql.com/docs/router/configuration/distributed-caching/#distributed-query-plan-caching), starting with this release and going forward you should anticipate additional cache regeneration cost when updating between any Router versions. | ||
|
||
More often than not, an Apollo Router release may contain changes that affect what query plans are generated or how they’re represented. To avoid using outdated entries from distributed cache, the cache key includes a counter that was manually incremented with relevant data structure or algorithm changes. Instead the cache key now includes the Router version number, so that different versions will always use separate cache entries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would put this paragraph first. The point is that almost every release has had a cache key change, which comes with the additional cache regeneration cost. If people read the first paragraph, they'll think this is more than what's currently happening, but it effectively isn't.
@@ -51,7 +51,10 @@ async fn query_planner_cache() -> Result<(), BoxError> { | |||
} | |||
// If this test fails and the cache key format changed you'll need to update the key here. | |||
// Look at the top of the file for instructions on getting the new cache key. | |||
let known_cache_key = "plan:cache:1:federation:v2.9.3:8c0b4bfb4630635c2b5748c260d686ddb301d164e5818c63d6d9d77e13631676:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:8f8ce6ad09f15c3d567a05f1c3d7230ab71b3366fcaebc9cc3bbfa356d55ac12"; | |||
let known_cache_key = &format!( | |||
"plan:router:{}:8c0b4bfb4630635c2b5748c260d686ddb301d164e5818c63d6d9d77e13631676:opname:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:metadata:8f8ce6ad09f15c3d567a05f1c3d7230ab71b3366fcaebc9cc3bbfa356d55ac12", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
appreciate the format!
with env var so we don't have to update these tests with every release 🙏
If you have enabled Distributed query plan caching, starting with this release and going forward you should anticipate additional cache regeneration cost when updating between any Router versions.
More often than not, an Apollo Router release may contain changes that affect what query plans are generated or how they’re represented. To avoid using outdated entries from distributed cache, the cache key includes a counter that was manually incremented with relevant data structure or algorithm changes. Instead the cache key now includes the Router version number, so that different versions will always use separate cache entries.
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩