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

sql: support collecting a statement bundle for a specific gist #103018

Closed
rytaft opened this issue May 10, 2023 · 2 comments
Closed

sql: support collecting a statement bundle for a specific gist #103018

rytaft opened this issue May 10, 2023 · 2 comments
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) C-escalation-improvement Having this feature would have made an escalation easier O-support Originated from a customer ui-design-needed Issues that require a UX/UI design for it

Comments

@rytaft
Copy link
Collaborator

rytaft commented May 10, 2023

Is your feature request related to a problem? Please describe.
A particular statement fingerprint often has a few different query plans that are used at execution time. In the DB Console, these are represented by different gists. When debugging an issue, sometimes we have reason to believe that a particular query plan is problematic, so it would be nice to selectively collect a statement bundle for a particular gist.

We already have the ability to conditionally collect statement bundles for high-latency query executions, but the target gist may not necessarily have higher latency. In that case, it's difficult to get a bundle for a very rare gist.

Describe the solution you'd like
Make it possible to collect a statement bundle for a specific gist.

Jira issue: CRDB-27794

@rytaft rytaft added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label May 10, 2023
@rytaft rytaft added this to Triage in Cluster Observability via automation May 10, 2023
@rytaft rytaft added this to Triage in SQL Queries via automation May 10, 2023
@maryliag maryliag added the ui-design-needed Issues that require a UX/UI design for it label May 10, 2023
@maryliag maryliag moved this from Triage to 23.2 in Cluster Observability May 15, 2023
@cucaroach cucaroach removed this from Triage in SQL Queries May 23, 2023
@cucaroach
Copy link
Contributor

Leaving this with cluster obs team, reach out to SQL Queries if any help is needed.

@maryliag maryliag added C-escalation-improvement Having this feature would have made an escalation easier T-cluster-observability O-support Originated from a customer and removed T-sql-observability labels Jul 6, 2023
@exalate-issue-sync exalate-issue-sync bot added T-cluster-observability observability-escalation-follow-up O-support Originated from a customer C-escalation-improvement Having this feature would have made an escalation easier and removed O-support Originated from a customer T-cluster-observability C-escalation-improvement Having this feature would have made an escalation easier observability-escalation-follow-up labels Jul 11, 2023
craig bot pushed a commit that referenced this issue Aug 5, 2023
105477: sql: add plan gist matching to stmt diagnostics feature r=yuzefovich a=yuzefovich

This commit extends the stmt diagnostics feature to add optional
plan-gist-based matching. Previously, we filtered statements based only
on the fingerprint but now we can optionally ask for a particular plan
(by specifying the target plan gist). All other aspects of the feature
(minimum execution latency, sampling probability) are unaffected.

The caveat to the implementation is that the plan gist of the running
statement is available after the optimizer has done its part, so
whenever plan-gist-based matching is desired, the trace will not
include the optimizer part as well as the plan string won't be
available.

This commit also made a minor change to always store the memo and the
opt planning catalog in `planTop`. Previously, this was stored only when
the bundle collection is enabled, but we now can enable it after the
optimizer, at which point the memo and the catalog might be lost. The
optimizer now stores it unconditionally, but then if we choose to not
collect the bundle once the plan gist is available, we release these
things. This allows us to still get `opt` files in the bundle.

Epic: None
Addresses: #96765.
Addresses: #103018.

Release note (sql change): Statement diagnostics feature has been
extended to support collecting a bundle for a particular plan. Namely,
the existing fingerprint-based matching has been extended to also
include plan-gist-based matching. Such bundle will miss a couple of
things: `plan.txt` file as well as the tracing of the optimizer. At
the moment, the feature is only exposed via an overload to
`crdb_internal.request_statement_bundle` builtin function. We now also
support "anti-match" - i.e. collecting a bundle for any plan other than
the provided plan gist.

108139: sql: fix logic to collect stats on system.jobs r=rytaft a=rytaft

This commit fixes an oversight in #102637 which intended to enable stats collection on the jobs table, but was not successful.

I've manually confirmed that stats are now collected on the jobs table in a local cluster:
```
  888074673664065537 | AUTO CREATE STATS               | Table statistics refresh for system.public.jobs                         | CREATE STATISTICS __auto__ FROM [15] WITH OPTIONS THROTTLING 0.9 AS OF SYSTEM TIME '-30s'  | root      | succeeded | NULL           | 2023-08-03 19:01:22.343
```

Informs #107405

Release note (performance improvement): We now automatically collect table statistics on the `system.jobs` table, which will enable the optimizer to produce better query plans for internal queries that access the `system.jobs` table. This may result in better performance of the system. Note: a previous attempt to enable stats on `system.jobs` starting in 23.1.0 was unsuccessful, but we have now fixed the oversight.

Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
maryliag added a commit to maryliag/cockroach that referenced this issue Sep 20, 2023
Part Of cockroachdb#103018

This commit adds an option to collect statement bundle
based on a specific plan gist.

Release note (ui change): Add option to filter out by specific
plan gist when collecting a statement bundle.
craig bot pushed a commit that referenced this issue Sep 20, 2023
110931: ui: add plan gist as option on bundle collection r=maryliag a=maryliag

Note to reviewers: There is another option to get any plan _except_ from the selected gist, but that is not part of this PR. Once a design is created, this option can be added.

---
Part Of #103018

This commit adds an option to collect statement bundle based on a specific plan gist.

<img width="578" alt="Screenshot 2023-09-19 at 3 18 01 PM" src="https://github.com/cockroachdb/cockroach/assets/1017486/5ab807b7-08f4-49e7-b540-2dadface766d">


https://www.loom.com/share/59335438f0884b75a7d163d96effe5a8

Release note (ui change): Add option to filter out by specific plan gist when collecting a statement bundle.

110976: syntheticprivilege: admin always has ALL global privileges r=rafiss a=rafiss

### syntheticprivilege: admin always has ALL global privileges

As we move away from requiring the admin role to perform cluster
debug/repair operations, we want to use a privilege instead. To
facilitate that, the admin role now implicitly has ALL global
privileges. The privilege for admins is not revokeable.

---

### sql: use better error message for missing system privilege

Since we document privileges on the GlobalPrivilegeObject using the
phrase "system privilege", we should make the error message say that
too.

informs #109814
Release note: None

110979: roachtest: use correct format directive for job ID r=yuzefovich a=yuzefovich

`catpb.JobID` doesn't implement `fmt.Stringer`.

Touches: #110782.

Epic: None

Release note: None

Co-authored-by: maryliag <marylia@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
@maryliag
Copy link
Contributor

maryliag commented Nov 3, 2023

Work completed

@maryliag maryliag closed this as completed Nov 3, 2023
Cluster Observability automation moved this from 24.1 to Done Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) C-escalation-improvement Having this feature would have made an escalation easier O-support Originated from a customer ui-design-needed Issues that require a UX/UI design for it
Projects
No open projects
Development

No branches or pull requests

3 participants