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

spanconfig/rangefeed: investigate microbenchmark regression #79602

Closed
Tracked by #81009
irfansharif opened this issue Apr 7, 2022 · 3 comments
Closed
Tracked by #81009

spanconfig/rangefeed: investigate microbenchmark regression #79602

irfansharif opened this issue Apr 7, 2022 · 3 comments
Labels
A-kv Anything in KV that doesn't belong in a more specific category. A-zone-configs C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. no-issue-activity X-stale

Comments

@irfansharif
Copy link
Contributor

irfansharif commented Apr 7, 2022

Describe the problem

One of our microbenchmarks show additional memory usage and was bisected to #74555. See #74555 (comment).

To Reproduce

I ran the following and used go tool pprof -http localhost:8082 -diff_base=mem-old.out -normalize mem-new.out to see where the differences are coming from.

dev bench pkg/bench -f='BenchmarkSQL/MultinodeCockroach/InsertSecondaryIndex/count=1000' --count=10 --bench-mem --timeout=24h -v --test-args '-test.memprofile=mem.out'

image

Looks like it's additional rangefeed event processing. I'm not sure off-hand why; I suspect it's got something to do with ranges having to enable rangefeeds by default when no targeted config is available.

Expected resolution

Pin down exactly why we're doing more processing, and whether it's expected.

Jira issue: CRDB-14939

@irfansharif irfansharif added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-zone-configs A-kv Anything in KV that doesn't belong in a more specific category. labels Apr 7, 2022
@nvanbenschoten
Copy link
Member

We return true from Replica.isRangefeedEnabled when !r.mu.spanConfigExplicitlySet:

if !r.mu.spanConfigExplicitlySet {
return true
}

Is this unintentionally enabling rangefeed and MVCCLogicalOp publication on most ranges?

@irfansharif
Copy link
Contributor Author

Discussed internally: spanConfigExplicitlySet is not whether a span config exists for the range or not, it’s whether the current replica’s span config has been applied from a rangefeed on the spanconfig table. So we're not enabling rangefeed + MVCCLogicalOp publication on most ranges. That said, this logic does have weird artifacts in test code that use scratch ranges for ex. where unless the ConfigureScratchRange testing knob is set to true, we don’t use span configs in that portion of the keyspan. So automatically have rangefeed enabled.

Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 1, 2024
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-zone-configs C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. no-issue-activity X-stale
Projects
None yet
Development

No branches or pull requests

3 participants