From d279aaba73ab3f23f4fda5937a9e51072692a391 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 10 Sep 2024 15:23:43 -0700 Subject: [PATCH] Allow disabling PQ-based QP cache rewarm when reloading schemas In #3829 we taught router to use the PQ list to prewarm the query planner cache when reloading its schema. In #5340 we gave it the experimental option to do the same thing at startup. Now we are allowing you to turn off the on-reload prewarm. Because the PQ list does not have metadata to tell the Router which PQs have been used recently, if you have a large PQ list then the on-reload prewarm can slow down schema updates noticeably, and it may not provide much value over prewarming based on the contents of the in-memory QP cache itself. We keep the same defaults, but let you turn off the on-reload prewarm with: ``` persisted_queries: experimental_prewarm_query_plan_cache: on_reload: false ``` Previously, `persisted_queries.experimental_prewarm_query_plan_cache` was a boolean; a migration moves this boolean to `persisted_queries.experimental_prewarm_query_plan_cache.on_startup`. (`on_startup` defaults to false and `on_reload` defaults to true.) This migration uses a `type: change` action. No such actions existed in the codebase and the implementation appeared to be buggy: there is no top-level `==` operator (or top-level operators at all) in JSONPath. This PR fixes the implementation of `type: change` to use the `[?(@.x == y)]` filter syntax which does appear to work (as tested by the migration tests for the new migration). --- .changesets/exp_qp_cache_prewarm_on_reload.md | 20 ++++++++++++ .../0028-pq-prewarm-to-on_startup.yaml | 12 +++++++ apollo-router/src/configuration/mod.rs | 1 + .../src/configuration/persisted_queries.rs | 32 ++++++++++++++----- ...nfiguration__tests__schema_generation.snap | 22 +++++++++++-- ...d-queries-prewarm-already-object.yaml.snap | 9 ++++++ ...@persisted-queries-prewarm-false.yaml.snap | 9 ++++++ ...n@persisted-queries-prewarm-true.yaml.snap | 9 ++++++ ...sisted-queries-prewarm-already-object.yaml | 4 +++ .../persisted-queries-prewarm-false.yaml | 3 ++ .../persisted-queries-prewarm-true.yaml | 3 ++ apollo-router/src/configuration/upgrade.rs | 2 +- .../query_planner/caching_query_planner.rs | 8 +++-- apollo-router/src/router_factory.rs | 4 +-- .../persisted_queries/manifest_poller.rs | 18 +++++------ .../src/services/supergraph/service.rs | 3 +- .../configuration/in-memory-caching.mdx | 6 ++-- .../configuration/persisted-queries.mdx | 6 ++-- 18 files changed, 138 insertions(+), 33 deletions(-) create mode 100644 .changesets/exp_qp_cache_prewarm_on_reload.md create mode 100644 apollo-router/src/configuration/migrations/0028-pq-prewarm-to-on_startup.yaml create mode 100644 apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-already-object.yaml.snap create mode 100644 apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-false.yaml.snap create mode 100644 apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-true.yaml.snap create mode 100644 apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-already-object.yaml create mode 100644 apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-false.yaml create mode 100644 apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-true.yaml diff --git a/.changesets/exp_qp_cache_prewarm_on_reload.md b/.changesets/exp_qp_cache_prewarm_on_reload.md new file mode 100644 index 00000000000..56f87a70e7c --- /dev/null +++ b/.changesets/exp_qp_cache_prewarm_on_reload.md @@ -0,0 +1,20 @@ +### Allow disabling persisted-queries-based query plan cache prewarm on schema reload + +In Router v1.31.0, we started including operations from persisted query lists when Router pre-warms the query plan cache when loading a new schema. + +In Router v1.49.0, we let you also pre-warm the query plan cache from the persisted query list during Router startup by setting `persisted_queries.experimental_prewarm_query_plan_cache` to true. + +We now allow you to disable the original feature, so that Router will only pre-warm recent operations from the query planning cache when loading a new schema (and not the persisted query list as well), by setting `persisted_queries.experimental_prewarm_query_plan_cache.on_reload` to `false`. + +The option added in v1.49.0 has been renamed from `persisted_queries.experimental_prewarm_query_plan_cache` to `persisted_queries.experimental_prewarm_query_plan_cache.on_startup`. Existing configuration files will keep working as before, but with a warning that can be resolved by updating your config file: + +```diff + persisted_queries: + enabled: true +- experimental_prewarm_query_plan_cache: true ++ experimental_prewarm_query_plan_cache: ++ on_startup: true +``` + + +By [@glasser](https://github.com/glasser) in https://github.com/apollographql/router/pull/5990 \ No newline at end of file diff --git a/apollo-router/src/configuration/migrations/0028-pq-prewarm-to-on_startup.yaml b/apollo-router/src/configuration/migrations/0028-pq-prewarm-to-on_startup.yaml new file mode 100644 index 00000000000..0c6c7557115 --- /dev/null +++ b/apollo-router/src/configuration/migrations/0028-pq-prewarm-to-on_startup.yaml @@ -0,0 +1,12 @@ +description: Experimental persisted query prewarm query plan cache can now be configured separately for `on_startup` and `on_reload`; the previous value now means `on_startup` +actions: + - type: change + path: persisted_queries.experimental_prewarm_query_plan_cache + from: true + to: + on_startup: true + - type: change + path: persisted_queries.experimental_prewarm_query_plan_cache + from: false + to: + on_startup: false diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 6fc59167d0c..7837d8e4dcf 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -15,6 +15,7 @@ use displaydoc::Display; use itertools::Itertools; use once_cell::sync::Lazy; pub(crate) use persisted_queries::PersistedQueries; +pub(crate) use persisted_queries::PersistedQueriesPrewarmQueryPlanCache; #[cfg(test)] pub(crate) use persisted_queries::PersistedQueriesSafelist; use regex::Regex; diff --git a/apollo-router/src/configuration/persisted_queries.rs b/apollo-router/src/configuration/persisted_queries.rs index e1ae76e0e00..2b395b79ff1 100644 --- a/apollo-router/src/configuration/persisted_queries.rs +++ b/apollo-router/src/configuration/persisted_queries.rs @@ -16,7 +16,7 @@ pub struct PersistedQueries { pub safelist: PersistedQueriesSafelist, /// Experimental feature to prewarm the query plan cache with persisted queries - pub experimental_prewarm_query_plan_cache: bool, + pub experimental_prewarm_query_plan_cache: PersistedQueriesPrewarmQueryPlanCache, /// Enables using a local copy of the persisted query manifest to safelist operations pub experimental_local_manifests: Option>, @@ -30,7 +30,7 @@ impl PersistedQueries { enabled: Option, log_unknown: Option, safelist: Option, - experimental_prewarm_query_plan_cache: Option, + experimental_prewarm_query_plan_cache: Option, experimental_local_manifests: Option>, ) -> Self { Self { @@ -38,7 +38,7 @@ impl PersistedQueries { safelist: safelist.unwrap_or_default(), log_unknown: log_unknown.unwrap_or_else(default_log_unknown), experimental_prewarm_query_plan_cache: experimental_prewarm_query_plan_cache - .unwrap_or_else(default_prewarm_query_plan_cache), + .unwrap_or_default(), experimental_local_manifests, } } @@ -67,13 +67,24 @@ impl PersistedQueriesSafelist { } } +/// Persisted Queries (PQ) query plan cache prewarm configuration +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields, default)] +pub struct PersistedQueriesPrewarmQueryPlanCache { + /// Enabling this field uses the persisted query list to pre-warm the query planner cache on startup (disabled by default) + pub on_startup: bool, + + /// Enabling this field uses the persisted query list to pre-warm the query planner cache on schema and config changes (enabled by default) + pub on_reload: bool, +} + impl Default for PersistedQueries { fn default() -> Self { Self { enabled: default_pq(), safelist: PersistedQueriesSafelist::default(), log_unknown: default_log_unknown(), - experimental_prewarm_query_plan_cache: default_prewarm_query_plan_cache(), + experimental_prewarm_query_plan_cache: PersistedQueriesPrewarmQueryPlanCache::default(), experimental_local_manifests: None, } } @@ -88,6 +99,15 @@ impl Default for PersistedQueriesSafelist { } } +impl Default for PersistedQueriesPrewarmQueryPlanCache { + fn default() -> Self { + Self { + on_startup: false, + on_reload: true, + } + } +} + const fn default_pq() -> bool { false } @@ -103,7 +123,3 @@ const fn default_require_id() -> bool { const fn default_log_unknown() -> bool { false } - -const fn default_prewarm_query_plan_cache() -> bool { - false -} diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 87898400745..4eba0206d0a 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -4119,9 +4119,8 @@ expression: "&schema" "type": "array" }, "experimental_prewarm_query_plan_cache": { - "default": false, - "description": "Experimental feature to prewarm the query plan cache with persisted queries", - "type": "boolean" + "$ref": "#/definitions/PersistedQueriesPrewarmQueryPlanCache", + "description": "#/definitions/PersistedQueriesPrewarmQueryPlanCache" }, "log_unknown": { "default": false, @@ -4135,6 +4134,23 @@ expression: "&schema" }, "type": "object" }, + "PersistedQueriesPrewarmQueryPlanCache": { + "additionalProperties": false, + "description": "Persisted Queries (PQ) query plan cache prewarm configuration", + "properties": { + "on_reload": { + "default": true, + "description": "Enabling this field uses the persisted query list to pre-warm the query planner cache on schema and config changes (enabled by default)", + "type": "boolean" + }, + "on_startup": { + "default": false, + "description": "Enabling this field uses the persisted query list to pre-warm the query planner cache on startup (disabled by default)", + "type": "boolean" + } + }, + "type": "object" + }, "PersistedQueriesSafelist": { "additionalProperties": false, "description": "Persisted Queries (PQ) Safelisting configuration", diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-already-object.yaml.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-already-object.yaml.snap new file mode 100644 index 00000000000..8898a55cc99 --- /dev/null +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-already-object.yaml.snap @@ -0,0 +1,9 @@ +--- +source: apollo-router/src/configuration/tests.rs +expression: new_config +--- +--- +persisted_queries: + enabled: true + experimental_prewarm_query_plan_cache: + on_reload: false diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-false.yaml.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-false.yaml.snap new file mode 100644 index 00000000000..3d45bd15ac1 --- /dev/null +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-false.yaml.snap @@ -0,0 +1,9 @@ +--- +source: apollo-router/src/configuration/tests.rs +expression: new_config +--- +--- +persisted_queries: + enabled: true + experimental_prewarm_query_plan_cache: + on_startup: false diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-true.yaml.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-true.yaml.snap new file mode 100644 index 00000000000..ef598f91d29 --- /dev/null +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__upgrade_old_configuration@persisted-queries-prewarm-true.yaml.snap @@ -0,0 +1,9 @@ +--- +source: apollo-router/src/configuration/tests.rs +expression: new_config +--- +--- +persisted_queries: + enabled: true + experimental_prewarm_query_plan_cache: + on_startup: true diff --git a/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-already-object.yaml b/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-already-object.yaml new file mode 100644 index 00000000000..a6647b3a042 --- /dev/null +++ b/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-already-object.yaml @@ -0,0 +1,4 @@ +persisted_queries: + enabled: true + experimental_prewarm_query_plan_cache: + on_reload: false diff --git a/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-false.yaml b/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-false.yaml new file mode 100644 index 00000000000..704865131f7 --- /dev/null +++ b/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-false.yaml @@ -0,0 +1,3 @@ +persisted_queries: + enabled: true + experimental_prewarm_query_plan_cache: false diff --git a/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-true.yaml b/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-true.yaml new file mode 100644 index 00000000000..3a1fafa87d5 --- /dev/null +++ b/apollo-router/src/configuration/testdata/migrations/persisted-queries-prewarm-true.yaml @@ -0,0 +1,3 @@ +persisted_queries: + enabled: true + experimental_prewarm_query_plan_cache: true diff --git a/apollo-router/src/configuration/upgrade.rs b/apollo-router/src/configuration/upgrade.rs index 0056801e86c..d925ed7354c 100644 --- a/apollo-router/src/configuration/upgrade.rs +++ b/apollo-router/src/configuration/upgrade.rs @@ -144,7 +144,7 @@ fn apply_migration(config: &Value, migration: &Migration) -> Result { - if !jsonpath_lib::select(config, &format!("$.{path} == {from}")) + if !jsonpath_lib::select(config, &format!("$[?(@.{path} == {from})]")) .unwrap_or_default() .is_empty() { diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index e2a4053d96d..bee5c9a8b55 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -28,6 +28,7 @@ use crate::cache::estimate_size; use crate::cache::storage::InMemoryCache; use crate::cache::storage::ValueType; use crate::cache::DeduplicatingCache; +use crate::configuration::PersistedQueriesPrewarmQueryPlanCache; use crate::error::CacheResolverError; use crate::error::QueryPlannerError; use crate::plugins::authorization::AuthorizationPlugin; @@ -167,7 +168,7 @@ where previous_cache: Option, count: Option, experimental_reuse_query_plans: bool, - experimental_pql_prewarm: bool, + experimental_pql_prewarm: &PersistedQueriesPrewarmQueryPlanCache, ) { let _timer = Timer::new(|duration| { ::tracing::info!( @@ -223,8 +224,9 @@ where cache_keys.shuffle(&mut thread_rng()); - let should_warm_with_pqs = - (experimental_pql_prewarm && previous_cache.is_none()) || previous_cache.is_some(); + let should_warm_with_pqs = (experimental_pql_prewarm.on_startup + && previous_cache.is_none()) + || (experimental_pql_prewarm.on_reload && previous_cache.is_some()); let persisted_queries_operations = persisted_query_layer.all_operations(); let capacity = if should_warm_with_pqs { diff --git a/apollo-router/src/router_factory.rs b/apollo-router/src/router_factory.rs index bb4c4a15224..fa2bde177fd 100644 --- a/apollo-router/src/router_factory.rs +++ b/apollo-router/src/router_factory.rs @@ -253,7 +253,7 @@ impl YamlRouterFactory { .supergraph .query_planning .experimental_reuse_query_plans, - configuration + &configuration .persisted_queries .experimental_prewarm_query_plan_cache, ) @@ -269,7 +269,7 @@ impl YamlRouterFactory { .supergraph .query_planning .experimental_reuse_query_plans, - configuration + &configuration .persisted_queries .experimental_prewarm_query_plan_cache, ) diff --git a/apollo-router/src/services/layers/persisted_queries/manifest_poller.rs b/apollo-router/src/services/layers/persisted_queries/manifest_poller.rs index c957b086e07..7051ec97be3 100644 --- a/apollo-router/src/services/layers/persisted_queries/manifest_poller.rs +++ b/apollo-router/src/services/layers/persisted_queries/manifest_poller.rs @@ -686,7 +686,6 @@ mod tests { use super::*; use crate::configuration::Apq; use crate::configuration::PersistedQueries; - use crate::configuration::PersistedQueriesSafelist; use crate::test_harness::mocks::persisted_queries::*; use crate::uplink::Endpoints; @@ -783,15 +782,14 @@ mod tests { let manifest_manager = PersistedQueryManifestPoller::new( Configuration::fake_builder() .apq(Apq::fake_new(Some(false))) - .persisted_query(PersistedQueries::new( - Some(true), - Some(false), - Some(PersistedQueriesSafelist::default()), - Some(false), - Some(vec![ - "tests/fixtures/persisted-queries-manifest.json".to_string() - ]), - )) + .persisted_query( + PersistedQueries::builder() + .enabled(true) + .experimental_local_manifests(vec![ + "tests/fixtures/persisted-queries-manifest.json".to_string(), + ]) + .build(), + ) .build() .unwrap(), ) diff --git a/apollo-router/src/services/supergraph/service.rs b/apollo-router/src/services/supergraph/service.rs index d06edb7092a..136f4e90635 100644 --- a/apollo-router/src/services/supergraph/service.rs +++ b/apollo-router/src/services/supergraph/service.rs @@ -28,6 +28,7 @@ use tracing_futures::Instrument; use crate::batching::BatchQuery; use crate::configuration::Batching; +use crate::configuration::PersistedQueriesPrewarmQueryPlanCache; use crate::context::OPERATION_NAME; use crate::error::CacheResolverError; use crate::graphql; @@ -945,7 +946,7 @@ impl SupergraphCreator { previous_cache: Option, count: Option, experimental_reuse_query_plans: bool, - experimental_pql_prewarm: bool, + experimental_pql_prewarm: &PersistedQueriesPrewarmQueryPlanCache, ) { self.query_planner_service .warm_up( diff --git a/docs/source/configuration/in-memory-caching.mdx b/docs/source/configuration/in-memory-caching.mdx index 000fade2e5a..e90f6c1d3a0 100644 --- a/docs/source/configuration/in-memory-caching.mdx +++ b/docs/source/configuration/in-memory-caching.mdx @@ -48,9 +48,7 @@ supergraph: When loading a new schema, a query plan might change for some queries, so cached query plans cannot be reused. -To prevent increased latency upon query plan cache invalidation, the router precomputes query plans for: -* The most used queries from the cache. -* The entire list of persisted queries. +To prevent increased latency upon query plan cache invalidation, the router precomputes query plans for the most used queries from the cache when a new schema is loaded. Precomputed plans will be cached before the router switches traffic over to the new schema. @@ -63,6 +61,8 @@ supergraph: warmed_up_queries: 100 ``` +(In addition, the router can use the contents of the [persisted query list](./persisted-queries) to prewarm the cache. By default, it does this when loading a new schema but not on startup; you can [configure](./persisted-queries#persisted-queries#experimental_prewarm_query_plan_cache) it to change either of these defaults.) + To get more information on the planning and warm-up process use the following metrics (where `` can be `redis` for distributed cache or `memory`): * counters: diff --git a/docs/source/configuration/persisted-queries.mdx b/docs/source/configuration/persisted-queries.mdx index bb2ba65d5e6..0caabc7d1d6 100644 --- a/docs/source/configuration/persisted-queries.mdx +++ b/docs/source/configuration/persisted-queries.mdx @@ -76,12 +76,14 @@ If used with the [`safelist`](#safelist) option, the router logs unregistered an -Adding `experimental_prewarm_query_plan_cache: true` to `persisted_queries` configures the router to prewarm the query plan cache on startup using the persisted query list. All subsequent requests benefit from the warmed cache, reducing latency and enhancing performance. +By default, the router [prewarms the query plan cache](./in-memory-caching#cache-warm-up) using all operations on the PQL when a new schema is loaded, but not at startup. Using the `experimental_prewarm_query_plan_cache` option, you can tell the router to prewarm the cache using the PQL on startup as well, or tell it not to prewarm the cache when reloading the schema. (This does not affect whether the router prewarms the query plan cache with recently-used operations from its in-memory cache.) Prewarming the cache means can reduce request latency by ensuring that operations are pre-planned when requests are received, but can make startup or schema reloads slower. ```yaml title="router.yaml" persisted_queries: enabled: true - experimental_prewarm_query_plan_cache: true # default: false + experimental_prewarm_query_plan_cache: + on_startup: true # default: false + on_reload: false # default: true ``` #### `experimental_local_manifests`