Skip to content
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

expose number of evaluated query plan options #6765

Merged
merged 10 commits into from
Feb 11, 2025
16 changes: 16 additions & 0 deletions apollo-federation/src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,8 @@ pub enum SingleFederationError {
InterfaceKeyMissingImplementationType { message: String },
#[error("@defer is not supported on subscriptions")]
DeferredSubscriptionUnsupported,
#[error("{message}")]
QueryPlanComplexityExceeded { message: String },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this!

}

impl SingleFederationError {
Expand Down Expand Up @@ -487,6 +489,9 @@ impl SingleFederationError {
ErrorCode::InterfaceKeyMissingImplementationType
}
SingleFederationError::DeferredSubscriptionUnsupported => ErrorCode::Internal,
SingleFederationError::QueryPlanComplexityExceeded { .. } => {
ErrorCode::QueryPlanComplexityExceededError
}
}
}
}
Expand Down Expand Up @@ -1453,6 +1458,15 @@ static UNSUPPORTED_FEDERATION_DIRECTIVE: LazyLock<ErrorCodeDefinition> = LazyLoc
)
});

static QUERY_PLAN_COMPLEXITY_EXCEEDED: LazyLock<ErrorCodeDefinition> = LazyLock::new(|| {
ErrorCodeDefinition::new(
"QUERY_PLAN_COMPLEXITY_EXCEEDED".to_owned(),
"Indicates that provided query has too many possible ways to generate a plan and cannot be planned in a reasonable amount of time"
.to_owned(),
None,
)
});

#[derive(Debug, strum_macros::EnumIter)]
pub enum ErrorCode {
Internal,
Expand Down Expand Up @@ -1534,6 +1548,7 @@ pub enum ErrorCode {
InterfaceKeyMissingImplementationType,
UnsupportedFederationVersion,
UnsupportedFederationDirective,
QueryPlanComplexityExceededError,
}

impl ErrorCode {
Expand Down Expand Up @@ -1633,6 +1648,7 @@ impl ErrorCode {
}
ErrorCode::UnsupportedFederationVersion => &UNSUPPORTED_FEDERATION_VERSION,
ErrorCode::UnsupportedFederationDirective => &UNSUPPORTED_FEDERATION_DIRECTIVE,
ErrorCode::QueryPlanComplexityExceededError => &QUERY_PLAN_COMPLEXITY_EXCEEDED,
}
}
}
10 changes: 7 additions & 3 deletions apollo-federation/src/query_graph/graph_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use crate::display_helpers::DisplayOption;
use crate::display_helpers::DisplaySlice;
use crate::display_helpers::State as IndentedFormatter;
use crate::error::FederationError;
use crate::error::SingleFederationError;
use crate::is_leaf_type;
use crate::link::federation_spec_definition::get_federation_spec_definition_from_subgraph;
use crate::link::graphql_definition::BooleanOrVariable;
Expand Down Expand Up @@ -3734,9 +3735,12 @@ impl SimultaneousPaths {
product.saturating_mul(options.len())
});
if num_options > 1_000_000 {
return Err(FederationError::internal(format!(
"flat_cartesian_product: excessive number of combinations: {num_options}"
)));
return Err(SingleFederationError::QueryPlanComplexityExceeded {
message: format!(
"Excessive number of combinations for a given path: {num_options}"
),
}
.into());
}
let mut product = Vec::with_capacity(num_options);

Expand Down
1 change: 1 addition & 0 deletions apollo-federation/src/query_plan/query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ impl Default for QueryPlannerDebugConfig {
#[derive(Debug, PartialEq, Default, Serialize)]
pub struct QueryPlanningStatistics {
pub evaluated_plan_count: Cell<usize>,
pub evaluated_plan_paths: Cell<usize>,
}

#[derive(Debug, Default, Clone)]
Expand Down
20 changes: 15 additions & 5 deletions apollo-federation/src/query_plan/query_planning_traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use tracing::trace;
use super::fetch_dependency_graph::FetchIdGenerator;
use crate::ensure;
use crate::error::FederationError;
use crate::error::SingleFederationError;
use crate::operation::Operation;
use crate::operation::Selection;
use crate::operation::SelectionSet;
Expand Down Expand Up @@ -417,14 +418,23 @@ impl<'a: 'b, 'b> QueryPlanningTraversal<'a, 'b> {
no_followups = true;
break;
}

let evaluated_paths_count = &self.parameters.statistics.evaluated_plan_paths;
let simultaneous_indirect_path_count: usize =
followups_for_option.iter().map(|p| p.paths.0.len()).sum();
evaluated_paths_count
.set(evaluated_paths_count.get() + simultaneous_indirect_path_count);

duckki marked this conversation as resolved.
Show resolved Hide resolved
new_options.extend(followups_for_option);
if let Some(options_limit) = self.parameters.config.debug.paths_limit {
if new_options.len() > options_limit as usize {
// TODO: Create a new error code for this error kind.
return Err(FederationError::internal(format!(
"Too many options generated for {}, reached the limit of {}.",
selection, options_limit,
)));
return Err(SingleFederationError::QueryPlanComplexityExceeded {
message: format!(
"Too many options generated for {}, reached the limit of {}.",
selection, options_limit,
),
}
.into());
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions apollo-router/src/query_planner/query_planner_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ impl QueryPlannerService {
formatted_query_plan: Some(Arc::new(plan.to_string())),
query_plan_root_node: root_node.map(Arc::new),
evaluated_plan_count: plan.statistics.evaluated_plan_count.clone().into_inner() as u64,
evaluated_plan_paths: plan.statistics.evaluated_plan_paths.clone().into_inner() as u64,
})
}

Expand Down Expand Up @@ -294,6 +295,7 @@ impl QueryPlannerService {
query_plan_root_node,
formatted_query_plan,
evaluated_plan_count,
evaluated_plan_paths,
} = plan_result;

// If the query is filtered, we want to generate the signature using the original query and generate the
Expand Down Expand Up @@ -324,6 +326,11 @@ impl QueryPlannerService {
"Number of query plans evaluated for a query before choosing the best one",
evaluated_plan_count
);
u64_histogram!(
"apollo.router.query_planning.plan.evaluated_paths",
"Number of paths (including intermediate ones) considered to plan a query before starting to generate a plan",
evaluated_plan_paths
);

Ok(QueryPlannerContent::Plan {
plan: Arc::new(super::QueryPlan {
Expand Down Expand Up @@ -560,6 +567,7 @@ pub(crate) struct QueryPlanResult {
pub(super) formatted_query_plan: Option<Arc<String>>,
pub(super) query_plan_root_node: Option<Arc<PlanNode>>,
pub(super) evaluated_plan_count: u64,
pub(super) evaluated_plan_paths: u64,
}

pub(crate) fn metric_query_planning_plan_duration(planner: &'static str, elapsed: f64) {
Expand Down