Skip to content

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 requested a review from guswynn April 17, 2024 09:15
@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
@aalexandrov aalexandrov removed their assignment Dec 13, 2024
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