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

kvserver/rangefeed: track targeted follow ups for unbuffered reg reviews #135332

Open
1 of 12 tasks
wenyihu6 opened this issue Nov 15, 2024 · 1 comment
Open
1 of 12 tasks
Assignees
Labels
A-kv Anything in KV that doesn't belong in a more specific category. A-kv-rangefeed Rangefeed infrastructure, server+client branch-master Failures and bugs on the master branch. T-kv KV Team

Comments

@wenyihu6
Copy link
Contributor

wenyihu6 commented Nov 15, 2024

Describe the problem

Jira issue: CRDB-44454

@wenyihu6 wenyihu6 added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Nov 15, 2024
Copy link

blathers-crl bot commented Nov 15, 2024

Hi @wenyihu6, please add branch-* labels to identify which branch(es) this C-bug affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@wenyihu6 wenyihu6 added the A-kv-rangefeed Rangefeed infrastructure, server+client label Nov 15, 2024
@wenyihu6 wenyihu6 self-assigned this Nov 15, 2024
@wenyihu6 wenyihu6 added A-kv Anything in KV that doesn't belong in a more specific category. T-kv KV Team labels Nov 15, 2024
@nicktrav nicktrav added branch-master Failures and bugs on the master branch. and removed C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. labels Nov 20, 2024
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Dec 10, 2024
This patch metamorphically enables the cluster setting
RangefeedUseBufferedSender.

Release note: none
Part of: cockroachdb#135332
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Dec 12, 2024
This patch metamorphically enables the cluster setting
RangefeedUseBufferedSender.

Release note: none
Part of: cockroachdb#135332
craig bot pushed a commit that referenced this issue Dec 12, 2024
135924: sql: label latency histograms with statement fingerprint r=MattWhelan a=MattWhelan

Previously, we collected a single histogram for all statements. While
this sort of metric may be useful in overall database operations,
application developers need more detail, to know how their code changes
impact their query latencies.

By switching to HistogramVec, we can now offer per-statement-fingerprint
latency metrics.

This feature is disabled by default. To know whether its safe to enable,
we also track a cardinality estimate of the unique statement
fingerprints that we see.

TREQ: https://cockroachlabs.atlassian.net/browse/TREQ-703

Epic: none

Release note (ops change):
Introduces a new `sql.exec.latency.detail` histogram metric. This metric
is labeled with its statement fingerprint. Enable this feature using the
`sql.stats.detailed_latency_metrics.enabled` application setting.

The `sql.query.unique.count` metric is a new count metric that estimates
the cardinality of the set of all statement fingerprints. For most
workloads, this ranges from dozens to hundreds. For workloads with over
a couple thousand fingerprints, we advise caution in enabling
`sql.stats.detailed_latency_metrics.enabled`.

Benchmark results:

```
# Baseline (master branch)
_elapsed_______tpmC____efc__avg(ms)__p50(ms)__p90(ms)__p95(ms)__p99(ms)_pMax(ms)
 1800.0s     6314.1  98.2%     31.2     21.0     52.4     88.1    209.7    570.4

# PR, setting disabled
_elapsed_______tpmC____efc__avg(ms)__p50(ms)__p90(ms)__p95(ms)__p99(ms)_pMax(ms)
 1800.0s     6315.6  98.2%     24.2     19.9     33.6     48.2    117.4    805.3

# PR, setting enabled
_elapsed_______tpmC____efc__avg(ms)__p50(ms)__p90(ms)__p95(ms)__p99(ms)_pMax(ms)
 1800.0s     6309.1  98.1%     24.2     19.9     33.6     48.2    113.2    671.1
```

136995: kvserver/rangefeed: metamorphically enable RangefeedUseBufferedSender r=stevendanna a=wenyihu6

**kvserver/rangefeed: rename .buffered_stream_sender. to .buffered_sender.**

This patch renames kv.rangefeed.buffered_stream_sender.enabled to
kv.rangefeed.buffered_sender.enabled to align with variable and
struct names better.

Epic: none
Release: none

---

**kvserver/rangefeed: metamorphically enable RangefeedUseBufferedSender**

This patch metamorphically enables the cluster setting
RangefeedUseBufferedSender.

Release note: none
Part of: #135332

137288: roachtest: make disk bandwidth test manual only r=aadityasondhi a=aadityasondhi

This test is a little too noisy until we make further improvements to the disk bandwidth limiter.

Fixes: #136064.

Release note: None

Co-authored-by: Matt Whelan <[email protected]>
Co-authored-by: Wenyi Hu <[email protected]>
Co-authored-by: Aaditya Sondhi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv Anything in KV that doesn't belong in a more specific category. A-kv-rangefeed Rangefeed infrastructure, server+client branch-master Failures and bugs on the master branch. T-kv KV Team
Projects
None yet
Development

No branches or pull requests

3 participants