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

kv: phase out ScanMetaKVs in favor of IterateRangeDescriptors #87503

Open
tbg opened this issue Sep 7, 2022 · 2 comments
Open

kv: phase out ScanMetaKVs in favor of IterateRangeDescriptors #87503

tbg opened this issue Sep 7, 2022 · 2 comments
Labels
A-kv Anything in KV that doesn't belong in a more specific category. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-starter Might be suitable for a starter project for new employees or team members.

Comments

@tbg
Copy link
Member

tbg commented Sep 7, 2022 •
edited by cockroach-jira-scripts
Loading

We use ScanMetaKVs1 in a few places. This has problems:

  • it doesn't paginate, i.e. pulls potentially a lot of data into memory
  • it doesn't work properly with keyspans such as [k, k.Next()]2
  • it can end up emitting a second copy of r12

In other words, it's a bit janky. But we have IterateRangeDescriptors3 which is tucked away in the upgradecluster package. This one deals with the second copy of r1, and paginates. I am not sure if it has the second problem, as it doesn't take a keyspan but instead returns all descriptors.

We should move IterateRangeDescriptors out of the upgradecluster package and make it a kv-level util, teach it how to restrict to only the meta ranges touching a given Span, make sure that it doesn't pick up the middle problem along the way, and eliminate ScanMetaKVs from the codebase.

Jira issue: CRDB-19381

Footnotes

  1. https://github.com/cockroachdb/cockroach/blob/f4b475a97618755284e6a70d8626c8266ce145fd/pkg/kv/kvclient/scan_meta.go#L21-L40 ↩

  2. https://github.com/cockroachdb/cockroach/pull/87378 ↩ ↩2

  3. https://github.com/cockroachdb/cockroach/blob/32622e1b18030bd52529841ec1bb280a5683d5cb/pkg/upgrade/upgradecluster/cluster.go#L142-L192 ↩
@tbg tbg added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-starter Might be suitable for a starter project for new employees or team members. A-kv Anything in KV that doesn't belong in a more specific category. labels Sep 7, 2022
@irfansharif irfansharif mentioned this issue Oct 14, 2022
Merged
irfansharif added a commit to irfansharif/cockroach that referenced this issue Oct 14, 2022
@irfansharif
rangedesciter: carve out library for range desc iteration
c69227a 
Informs cockroachdb#87503; pure code-movement. Going to use it in future commits as
part of multi-tenant replication reports (cockroachdb#89987) where we'll need to
iterate over the set of range descriptors.

Release note: None
@irfansharif
Copy link
Contributor

There's also this but all this code is getting nuked soon:

cockroach/pkg/kv/kvserver/reports/reporter.go

Lines 655 to 657 in 4c43c2b

// meta2RangeIter is an implementation of RangeIterator that scans meta2 in a
// paginated way.
type meta2RangeIter struct {

irfansharif added a commit to irfansharif/cockroach that referenced this issue Oct 14, 2022
@irfansharif
rangedesciter: carve out library for range desc iteration
75688c2 
Informs cockroachdb#87503; pure code-movement. Going to use it in future commits as
part of multi-tenant replication reports (cockroachdb#89987) where we'll need to
iterate over the set of range descriptors.

Release note: None
craig bot pushed a commit that referenced this issue Oct 14, 2022
@stevendanna @nicktrav @irfansharif
Merge #87938 #89257 #89873 #89988
b0ed2bf 
87938: bazci: move the logic of posting github issues to bazci r=srosenberg a=healthy-pod

This code change moves the logic of filtering tests JSON output files and posting github issues to bazci.

Release note: None
Epic CRDB-15060

89257: upgrades: remove unused struct field r=stevendanna a=stevendanna

Epic: None

Release note: None

89873: cliccl: add `encryption-registry-list` command r=jbowenns a=nicktrav

The existing `enc_util` package contains a tool that could be used to dump the files in an encryption registry. This command has been broken since the file registry format was updated.

Add the `(*PebbleFileRegistry).List` function, that returns a map of files in the registry. Adapt existing test cases.

Add a `debug encryption-registry-list` command that will print all files contained in the registry of an encrypted store. This is useful for debugging which store / data key was used to encrypt each file, replacing the equivalent functionality in `enc_util`.

Touches: #89095.
Epic: None.

Release note (ops change): Adds a new command that can be used by an operator to list the files present in the Encryption-At-Rest file registry.

89988: rangedesciter: carve out library for range desc iteration r=irfansharif a=irfansharif

Informs #87503; pure code-movement. Going to use it in future commits as part of multi-tenant replication reports (#89987) where we'll need to iterate over the set of range descriptors.

Release note: None

Co-authored-by: healthy-pod <[email protected]>
Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Nick Travers <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
kvoli pushed a commit to kvoli/cockroach that referenced this issue Oct 17, 2022
@irfansharif @kvoli
rangedesciter: carve out library for range desc iteration
6153454 
Informs cockroachdb#87503; pure code-movement. Going to use it in future commits as
part of multi-tenant replication reports (cockroachdb#89987) where we'll need to
iterate over the set of range descriptors.

Release note: None
@irfansharif irfansharif mentioned this issue Nov 4, 2022
Merged
irfansharif added a commit to irfansharif/cockroach that referenced this issue Nov 5, 2022
@irfansharif
rangedesciter: support scoped iteration
b39e627 
Informs cockroachdb#87503; adds the ability to only paginate through range
descriptors in the system that overlap with some given span. We're going
to use it in future commits as part of multi-tenant replication reports
(cockroachdb#89987) where we'll need to iterate over the set of range descriptors
that pertain only to some span of interest (for a given index,
partition, tenant, etc.)

Release note: None
irfansharif added a commit to irfansharif/cockroach that referenced this issue Nov 23, 2022
@irfansharif
rangedesciter: support scoped iteration
98ab461 
Informs cockroachdb#87503; adds the ability to only paginate through range
descriptors in the system that overlap with some given span. We're going
to use it in future commits as part of multi-tenant replication reports
(cockroachdb#89987) where we'll need to iterate over the set of range descriptors
that pertain only to some span of interest (for a given index,
partition, tenant, etc.)

Release note: None
craig bot pushed a commit that referenced this issue Nov 23, 2022
@irfansharif @girishv-tw
Merge #91330 #92253
14f61c5 
91330: rangedesciter: support scoped iteration r=irfansharif a=irfansharif

Informs #87503; adds the ability to only paginate through range descriptors in the system that overlap with some given span. We're going to use it in future commits as part of multi-tenant replication reports (#89987) where we'll need to iterate over the set of range descriptors that pertain only to some span of interest (for a given index, partition, tenant, etc.)

Release note: None.

92253: sql/sem: add support for "pg_blocking_pids" builtin r=rafiss a=girishv-tw

added support pg_blocking_pids builtin. Needed for pgadmin.

Closes #91068

Release note (sql change): add support for pg_blocking_pids builtin function. It is hardcoded to return an empty array because CockroachDB has no equivalent concept of PIDs as in Postgres.

Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: GirishV <[email protected]>
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 8, 2024

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 23, 2024
@yuzefovich yuzefovich reopened this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-kv Anything in KV that doesn't belong in a more specific category. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) E-starter Might be suitable for a starter project for new employees or team members.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tbg @yuzefovich @irfansharif