Skip to content

Commit

Permalink
refactor: Use two instances of the query service for queries and http…
Browse files Browse the repository at this point in the history
…s outcall transform functions (#3992)

In a previous
[change](3760f0a),
the older `AnonymousQueryService` was dropped in order to re-use the
same `QueryService` for handling both regular queries and transform
functions from https outcalls.

While this change is a step in a good direction as we don't have mostly
duplicate code in `AnonymousQueryService`, it couples regular queries
with transform function executions. To provide better quality of
service, go back to using two instances of the same service. This way we
can get the best of both worlds: code reusability as well as isolation
between the two types of queries.
  • Loading branch information
dsarlis authored Feb 19, 2025
1 parent 20b0caf commit 075a364
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 8 deletions.
10 changes: 10 additions & 0 deletions rs/execution_environment/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ pub struct ExecutionServices {
pub ingress_history_writer: Arc<dyn IngressHistoryWriter<State = ReplicatedState>>,
pub ingress_history_reader: Box<dyn IngressHistoryReader>,
pub query_execution_service: QueryExecutionService,
pub https_outcalls_service: QueryExecutionService,
pub scheduler: Box<dyn Scheduler<State = ReplicatedState>>,
pub query_stats_payload_builder: QueryStatsPayloadBuilderParams,
}
Expand Down Expand Up @@ -176,6 +177,14 @@ impl ExecutionServices {
query_scheduler.clone(),
Arc::clone(&state_reader),
metrics_registry,
"regular",
);
let https_outcalls_service = HttpQueryHandler::new_service(
Arc::clone(&sync_query_handler) as Arc<_>,
query_scheduler.clone(),
Arc::clone(&state_reader),
metrics_registry,
"https_outcall",
);
let ingress_filter = IngressFilterServiceImpl::new_service(
query_scheduler.clone(),
Expand Down Expand Up @@ -203,6 +212,7 @@ impl ExecutionServices {
ingress_history_writer,
ingress_history_reader,
query_execution_service,
https_outcalls_service,
scheduler,
query_stats_payload_builder,
}
Expand Down
18 changes: 11 additions & 7 deletions rs/execution_environment/src/query_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use ic_types::{
messages::{Blob, Certificate, CertificateDelegation, Query},
CanisterId, NumInstructions, PrincipalId, SubnetId,
};
use prometheus::Histogram;
use prometheus::{histogram_opts, labels, Histogram};
use serde::Serialize;
use std::convert::Infallible;
use std::str::FromStr;
Expand Down Expand Up @@ -119,12 +119,15 @@ struct HttpQueryHandlerMetrics {
}

impl HttpQueryHandlerMetrics {
pub fn new(metrics_registry: &MetricsRegistry) -> Self {
pub fn new(metrics_registry: &MetricsRegistry, namespace: &str) -> Self {
Self {
height_diff_during_query_scheduling: metrics_registry.histogram(
"execution_query_height_diff_during_query_scheduling",
"The height difference between the latest certified height before query scheduling and state height used for execution",
vec![0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 20.0, 50.0, 100.0],
height_diff_during_query_scheduling: metrics_registry.register(
Histogram::with_opts(histogram_opts!(
"execution_query_height_diff_during_query_scheduling",
"The height difference between the latest certified height before query scheduling and state height used for execution",
vec![0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 20.0, 50.0, 100.0],
labels! {"query_type".to_string() => namespace.to_string()}
)).unwrap(),
),
}
}
Expand Down Expand Up @@ -349,12 +352,13 @@ impl HttpQueryHandler {
query_scheduler: QueryScheduler,
state_reader: Arc<dyn StateReader<State = ReplicatedState>>,
metrics_registry: &MetricsRegistry,
namespace: &str,
) -> QueryExecutionService {
BoxCloneService::new(Self {
internal,
state_reader,
query_scheduler,
metrics: Arc::new(HttpQueryHandlerMetrics::new(metrics_registry)),
metrics: Arc::new(HttpQueryHandlerMetrics::new(metrics_registry, namespace)),
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion rs/replica/src/setup_ic_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ pub fn construct_ic_stack(
rt_handle_main.clone(),
metrics_registry,
config.adapters_config,
execution_services.query_execution_service.clone(),
execution_services.https_outcalls_service,
max_canister_http_requests_in_flight,
log.clone(),
subnet_type,
Expand Down

0 comments on commit 075a364

Please sign in to comment.