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.

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

Jump to bottom

Investigate adding sampling probability to statement bundle collection #92415

Closed
kevin-v-ngo opened this issue Nov 23, 2022 · 0 comments · Fixed by #101381
Closed

Investigate adding sampling probability to statement bundle collection #92415

kevin-v-ngo opened this issue Nov 23, 2022 · 0 comments · Fixed by #101381
Assignees
@maryliag
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@kevin-v-ngo
Copy link

kevin-v-ngo commented Nov 23, 2022 •
edited
Loading

This issue breaks down #90292 and #82896 (comment) in the context of statement bundles.

This issue tracks the investigation of surfacing the sampling probability introduced in (#82896) to statement bundles. This is an improvement to the existing conditional statement bundling experience. Although bundles (tracing) will not always be activated, this improvement will enable users to turn on statement bundles with higher confidence that bundles will not impact all statement fingerprint executions. This will help for known outlier scenarios.

A follow up issues has been created here: #92416 to define a retention and expiration policy.

Epic: CRDB-8649

Jira issue: CRDB-21754
@kevin-v-ngo kevin-v-ngo added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Nov 23, 2022
This was referenced Nov 23, 2022
Open
Closed
@kevin-v-ngo kevin-v-ngo mentioned this issue Feb 28, 2023
Open
@maryliag maryliag self-assigned this Apr 11, 2023
@maryliag maryliag mentioned this issue Apr 12, 2023
Merged
craig bot pushed a commit that referenced this issue Apr 18, 2023
@tbg @maryliag @rickystewart
Merge #98762 #101381 #101650 #101706 #101750
479ee62 
98762: kvserver: remove below-raft PreIngestDelay during SST application r=erikgrinaker a=tbg

**This is for 23.2 only**

We shouldn't artificially delay SST ingestion below raft because this
exacerbates memory pressure (#81834).

Anecdotally I see in my "typical experiment" (restores under I/O
bandwidth constraints) that `PreIngestDelay` is mostly fairly small
compared to the slowness that comes from write bandwidth overload
itself, so at least in those experiments removing this has little
to no effect.

As we are also working on replication admission control[^1] and are
looking into improving the memory footprint under unchecked overload[^2]
now's a good time to rip this out as we'll be in a good place to address
any detrimental fallout from doing so.

[^1]: #95563
[^2]: #98576

Touches #81834.
Fixes #57247.

Epic: CRDB-25503
Release note: None

101381: ui: add trace rate option to stmt diagnostics r=maryliag a=maryliag

Fixes #92415

Add option to select the trace rate for statement
diagnostics collection directly on the Console.

https://www.loom.com/share/beaf1ce16f7d4efc845056e33cb3bee1

<img width="587" alt="Screenshot 2023-04-12 at 4 09 50 PM" src="https://user-images.githubusercontent.com/1017486/231573175-e05ea6dd-d03d-4044-ae53-bb4cba55bb77.png">

<img width="591" alt="Screenshot 2023-04-12 at 4 10 00 PM" src="https://user-images.githubusercontent.com/1017486/231573195-5189fee3-8af2-4ada-af1b-8cc6fde5ceb2.png">


Release note (ui change): Add option to select the trace rate for statement diagnostics collection.

101650: tenantcostclient: mark test as being `no-remote` r=rail a=rickystewart

This test is broken under remote execution.

Epic: CRDB-17165
Release note: None

101706: ui: update side nav and titles to match r=maryliag a=maryliag

Previously the values for Advanced Debug (side nav) and Advanced Debugging (page title) were not matching. This commit uses the name "`Advanced Debug` for both.

Similarly, we were using Network Latency on the side nav and Network Diagnostics on the page title. Since we might want to show more than just latency, this commit updates the title to the more generic `Network`, to match how we name other pages (e.g. SQL Activity, Database, etc).

This commit also removed an extra space on the filter on the Network page.

Before
<img width="968" alt="Screenshot 2023-04-17 at 6 02 56 PM" src="https://user-images.githubusercontent.com/1017486/232657702-c399f8b0-9755-4aff-8043-c1753c7ad913.png">


After
<img width="967" alt="Screenshot 2023-04-17 at 10 43 18 PM" src="https://user-images.githubusercontent.com/1017486/232657728-7b907868-abc4-4926-80c8-5cf616ee2d58.png">


Epic: none

Release note (ui change): Update Network Latency side nav name and Network Diagnostics page title to `Network`. Update the Advanced Debugging page title to `Advanced Debug`.

101750: keyvisjob: mark the keyvis job as idle when it is not doing useful work r=zachlite a=zachlite

Resolves #101539
Epic: none
Release note: None

Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Zach Lite <[email protected]>
@craig craig bot closed this as completed in 8842795 Apr 18, 2023
@blathers-crl blathers-crl bot mentioned this issue Apr 18, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Apr 18, 2023
@maryliag
ui: add trace rate option to stmt diagnostics
babbbca 
Fixes #92415

Add option to select the trace rate for statement
diagnostics collection directly on the Console.

Release note (ui change): Add option to select the trace rate
for statement diagnostics collection.
@blathers-crl blathers-crl bot mentioned this issue Apr 18, 2023
Merged
blathers-crl bot pushed a commit that referenced this issue Apr 18, 2023
@maryliag
ui: add trace rate option to stmt diagnostics
2d9a547 
Fixes #92415

Add option to select the trace rate for statement
diagnostics collection directly on the Console.

Release note (ui change): Add option to select the trace rate
for statement diagnostics collection.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maryliag maryliag

Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ui: add trace rate option to stmt diagnostics maryliag/cockroach
2 participants
@maryliag @kevin-v-ngo