Skip to content

Add metrics for optimization notices #21868

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.

Sign up for GitHub

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

Merged
ggevay merged 2 commits into from
Sep 21, 2023
Merged

Add metrics for optimization notices #21868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9473a1d
Adjust dropped in-use index notice msg
ggevay Sep 21, 2023
168fa78
Add metrics for optimization notices
ggevay Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/adapter/src/coord/sequencer/inner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,10 @@ impl Coordinator {
}
for dropped_in_use_index in dropped_in_use_indexes {
session.add_notice(AdapterNotice::DroppedInUseIndex(dropped_in_use_index));
self.metrics
.optimization_notices
.with_label_values(&["DroppedInUseIndex"])
Copy link
Contributor

@aalexandrov aalexandrov Sep 21, 2023

Choose a reason for hiding this comment

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

I'm not sure if adding a value should be preferred over adding a separate metric per notice type. cc @teskje what would be better for minimizing the number of metrics?

Copy link
Contributor

@pH14 pH14 Sep 21, 2023

Choose a reason for hiding this comment

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

the # of time series will be the same either way (the name && labels together are uniquely identifying). what's here looks great

.inc_by(1);
}
Ok(ExecuteResponse::DroppedObject(object_type))
}
Expand Down Expand Up @@ -5224,6 +5228,10 @@ impl Coordinator {
let (notice, hint) = optimizer_notice.to_string(&humanizer);
session.add_notice(AdapterNotice::OptimizerNotice { notice, hint });
}
self.metrics
.optimization_notices
.with_label_values(&[optimizer_notice.metric_label()])
.inc_by(1);
}
}
}
6 changes: 6 additions & 0 deletions src/adapter/src/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub struct Metrics {
pub statement_logging_unsampled_bytes: IntCounterVec,
pub statement_logging_actual_bytes: IntCounterVec,
pub slow_message_handling: HistogramVec,
pub optimization_notices: IntCounterVec,
}

impl Metrics {
Expand Down Expand Up @@ -114,6 +115,11 @@ impl Metrics {
var_labels: ["message_kind"],
buckets: histogram_seconds_buckets(0.128, 32.0),
)),
optimization_notices: registry.register(metric!(
name: "mz_optimization_notices",
help: "Number of optimization notices per notice type.",
var_labels: ["notice_type"],
)),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/adapter/src/notice.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ impl fmt::Display for AdapterNotice {
index_name,
dependant_objects,
}) => {
write!(f, "The dropped index {index_name} is being used by the following objects: {}. The index will be dropped from the catalog, but it will continue to be maintained and take up resources!", separated(", ", dependant_objects))
write!(f, "The dropped index {index_name} is being used by the following objects: {}. The index is now dropped from the catalog, but it will continue to be maintained and take up resources until all dependent objects are dropped, altered, or Materialize is restarted!", separated(", ", dependant_objects))
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions src/transform/src/optimizer_notices.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,17 @@ impl OptimizerNotice {
OptimizerNotice::IndexKeyEmpty => true,
}
}

/// A notice name, which will be applied as the label on the metric that is counting notices
/// labelled by notice type.
pub fn metric_label(&self) -> &str {
match self {
OptimizerNotice::IndexTooWideForLiteralConstraints(..) => {
"IndexTooWideForLiteralConstraints"
}
OptimizerNotice::IndexKeyEmpty => "IndexKeyEmpty",
}
}
}

#[derive(Debug)]
Expand Down
6 changes: 3 additions & 3 deletions test/pgtest-mz/notice.pt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ CommandComplete {"tag":"CREATE INDEX"}
ReadyForQuery {"status":"I"}
CommandComplete {"tag":"DROP INDEX"}
ReadyForQuery {"status":"I"}
NoticeResponse {"fields":[{"typ":"S","value":"NOTICE"},{"typ":"C","value":"01000"},{"typ":"M","value":"The dropped index materialize.public.t_idx_a is being used by the following objects: materialize.public.mv1. The index will be dropped from the catalog, but it will continue to be maintained and take up resources!"}]}
NoticeResponse {"fields":[{"typ":"S","value":"NOTICE"},{"typ":"C","value":"01000"},{"typ":"M","value":"The dropped index materialize.public.t_idx_a is being used by the following objects: materialize.public.mv1. The index is now dropped from the catalog, but it will continue to be maintained and take up resources until all dependent objects are dropped, altered, or Materialize is restarted!"}]}
CommandComplete {"tag":"DROP INDEX"}
ReadyForQuery {"status":"I"}
CommandComplete {"tag":"DROP MATERIALIZED VIEW"}
Expand All @@ -72,14 +72,14 @@ CommandComplete {"tag":"CREATE INDEX"}
ReadyForQuery {"status":"I"}
CommandComplete {"tag":"CREATE MATERIALIZED VIEW"}
ReadyForQuery {"status":"I"}
NoticeResponse {"fields":[{"typ":"S","value":"NOTICE"},{"typ":"C","value":"01000"},{"typ":"M","value":"The dropped index materialize.public.t_idx_a is being used by the following objects: materialize.public.mv2. The index will be dropped from the catalog, but it will continue to be maintained and take up resources!"}]}
NoticeResponse {"fields":[{"typ":"S","value":"NOTICE"},{"typ":"C","value":"01000"},{"typ":"M","value":"The dropped index materialize.public.t_idx_a is being used by the following objects: materialize.public.mv2. The index is now dropped from the catalog, but it will continue to be maintained and take up resources until all dependent objects are dropped, altered, or Materialize is restarted!"}]}
CommandComplete {"tag":"DROP INDEX"}
ReadyForQuery {"status":"I"}
CommandComplete {"tag":"CREATE INDEX"}
ReadyForQuery {"status":"I"}
CommandComplete {"tag":"CREATE INDEX"}
ReadyForQuery {"status":"I"}
NoticeResponse {"fields":[{"typ":"S","value":"NOTICE"},{"typ":"C","value":"01000"},{"typ":"M","value":"The dropped index materialize.public.t_idx_a is being used by the following objects: materialize.public.v1_idx_a. The index will be dropped from the catalog, but it will continue to be maintained and take up resources!"}]}
NoticeResponse {"fields":[{"typ":"S","value":"NOTICE"},{"typ":"C","value":"01000"},{"typ":"M","value":"The dropped index materialize.public.t_idx_a is being used by the following objects: materialize.public.v1_idx_a. The index is now dropped from the catalog, but it will continue to be maintained and take up resources until all dependent objects are dropped, altered, or Materialize is restarted!"}]}
CommandComplete {"tag":"DROP INDEX"}
ReadyForQuery {"status":"I"}
CommandComplete {"tag":"DROP INDEX"}
Expand Down