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

outliers: rename to insights #84917

Merged
merged 1 commit into from
Jul 25, 2022
Merged

outliers: rename to insights #84917

merged 1 commit into from
Jul 25, 2022

Conversation

matthewtodd
Copy link
Contributor

@matthewtodd matthewtodd commented Jul 22, 2022

The UI design that includes outliers also features other insights about
sql execution. We will expand this outliers subsystem to support making
those insights as well (probably with more detectors, changing the
signature / return type of isOutlier). Renaming now is the first step
in that direction.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@matthewtodd matthewtodd marked this pull request as ready for review July 25, 2022 13:57
@matthewtodd matthewtodd requested a review from a team July 25, 2022 13:57
@matthewtodd matthewtodd requested review from a team as code owners July 25, 2022 13:57
@matthewtodd matthewtodd requested a review from a team July 25, 2022 13:57
@matthewtodd matthewtodd requested a review from a team as a code owner July 25, 2022 13:57
@matthewtodd matthewtodd requested a review from a team July 25, 2022 13:58
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

Just had one questions, otherwise :lgtm:

Reviewed 40 of 40 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @matthewtodd)


pkg/sql/sqlstats/insights/insights.go line 32 at r1 (raw file):

var LatencyThreshold = settings.RegisterDurationSetting(
	settings.TenantWritable,
	"sql.stats.insights.experimental.latency_threshold",

now that you made more tests, should we maybe remove the experimental ?

Copy link
Contributor Author

@matthewtodd matthewtodd left a comment

Choose a reason for hiding this comment

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

TFTR, @maryliag!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @maryliag)


pkg/sql/sqlstats/insights/insights.go line 32 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

now that you made more tests, should we maybe remove the experimental ?

Yes! I'll be doing that separately (and have more thoughts on naming queued up from Kevin), just wanted to get this mechanical renaming out there.

Copy link
Contributor

@j82w j82w left a comment

Choose a reason for hiding this comment

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

Reviewed 40 of 40 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @maryliag)

The UI design that includes outliers also features other insights about
sql execution. We will expand this outliers subsystem to support making
those insights as well (probably with more detectors, changing the
signature / return type of `isOutlier`). Renaming now is the first step
in that direction.

Release note: None
@matthewtodd
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 25, 2022

Build succeeded:

@craig craig bot merged commit 9e44f56 into cockroachdb:master Jul 25, 2022
@matthewtodd matthewtodd deleted the outliers-rename branch July 25, 2022 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants