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

explain: remove Filter layer from OptimizerTrace stack #26679

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aalexandrov
Copy link
Contributor

@aalexandrov aalexandrov commented Apr 17, 2024

It's not clear why we need this, and it might be causing a regression in our optimization times.

This might be a root cause of MaterializeInc/database-issues#7881.

Motivation

  • This PR fixes a recognized bug.

Fixes MaterializeInc/database-issues#7881.

Tips for reviewer

I don't think we need the extra

.with(tracing::level_filters::LevelFilter::TRACE)

layer in OptimizerTrace::new(...) because the Filter implementation for PlanTrace will always report interest in target: "optimizer" spans:

impl<S, T> layer::Filter<S> for PlanTrace<T>
where
S: subscriber::Subscriber,
T: 'static + Clone,
{
fn enabled(&self, meta: &Metadata<'_>, _cx: &layer::Context<'_, S>) -> bool {
self.is_enabled(meta)
}
fn callsite_enabled(&self, meta: &'static Metadata<'static>) -> Interest {
if self.is_enabled(meta) {
Interest::always()
} else {
Interest::never()
}
}
}

/// Check if a subscriber layer of this kind will be interested in tracing
/// spans and events with the given metadata.
fn is_enabled(&self, meta: &Metadata<'_>) -> bool {
meta.is_span() && meta.target() == "optimizer"
}

Checklist

@aalexandrov aalexandrov self-assigned this Apr 17, 2024
@aalexandrov aalexandrov added A-optimization Area: query optimization and transformation A-compute Area: compute labels Apr 17, 2024
It's not clear why we need this, and it might be causing a regression
in our optimization times.
@aalexandrov aalexandrov marked this pull request as ready for review April 19, 2024 09:21
@aalexandrov aalexandrov requested a review from a team April 19, 2024 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compute Area: compute A-optimization Area: query optimization and transformation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant