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

rangedesciter: support scoped iteration #91330

Merged
merged 1 commit into from
Nov 23, 2022

Conversation

irfansharif
Copy link
Contributor

@irfansharif irfansharif commented Nov 4, 2022

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.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@irfansharif irfansharif changed the title rangedesciter: support bounding scans rangedesciter: support scoped iteration Nov 5, 2022
@irfansharif irfansharif marked this pull request as ready for review November 5, 2022 00:23
@irfansharif irfansharif requested a review from a team as a code owner November 5, 2022 00:23
@irfansharif irfansharif requested a review from a team November 5, 2022 00:23
@irfansharif irfansharif requested a review from a team as a code owner November 5, 2022 00:23
@irfansharif
Copy link
Contributor Author

(Bump.)

@ajwerner
Copy link
Contributor

@ecwall this may or may not interest you

Copy link
Collaborator

@arulajmani arulajmani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ecwall this may or may not interest you

+1. We might want to use this on the server side, instead of ScanMetaKVs, when we build out the new RPC to enable tenants to run SHOW RANGEs like we were talking about this morning.

Reviewed 16 of 16 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @irfansharif and @tbg)


pkg/util/rangedesciter/rangedesciter.go line 99 at r1 (raw file):

		init()

		// Bound the start key of the meta{1,2} scan as much as possible. If the

Could you instead do:

metaKey := keys.RangeMetaKey(key)
bounds, err := keys.MetaScanBounds(metaKey)
 metaScanStartKey := bounds.Key   
```?

Would this allow you to get rid of the special casing in this comment and the if condition in the for loop below explained by:
					// Keys in meta ranges are encoded using the end keys of
					// range descriptors. It's possible then the first
					// descriptor/key we read has the same (exclusive) end
					// key as our query start. We'll want to skip over this
					// descriptor.

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
Copy link
Contributor Author

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani and @tbg)


pkg/util/rangedesciter/rangedesciter.go line 99 at r1 (raw file):

Previously, arulajmani (Arul Ajmani) wrote…

Could you instead do:

metaKey := keys.RangeMetaKey(key)
bounds, err := keys.MetaScanBounds(metaKey)
 metaScanStartKey := bounds.Key   
```?

Would this allow you to get rid of the special casing in this comment and the if condition in the for loop below explained by:
					// Keys in meta ranges are encoded using the end keys of
					// range descriptors. It's possible then the first
					// descriptor/key we read has the same (exclusive) end
					// key as our query start. We'll want to skip over this
					// descriptor.

TIL about keys.MetaScanBounds. This stuff always melts my brain, but after staring at this for a while, and reading 1 (thanks for the pointer!), you're right. Done + changed some commentary.

// Bound the start key of the meta{1,2} scan as much as possible. If the
// start key < keys.Meta1KeyMax (we're also interested in the meta1
// range descriptor), start our scan at keys.MetaMin. If not, start it
// at the relevant range meta key -- in meta1 if the start key sits
// within meta2, in meta2 if the start key is an ordinary key.
//
// So what exactly is the "relevant range meta key"? Since keys in meta
// ranges are encoded using the end keys of range descriptors, we're
// looking for the lowest existing range meta key that's strictly
// greater than RangeMetaKey(start key).
rangeMetaKeyForStart := keys.RangeMetaKey(rspan.Key)
metaScanBoundsForStart, err := keys.MetaScanBounds(rangeMetaKeyForStart)
if err != nil {
  return err
}
metaScanStartKey := metaScanBoundsForStart.Key.AsRawKey()

@craig
Copy link
Contributor

craig bot commented Nov 23, 2022

Build succeeded:

@craig craig bot merged commit 14f61c5 into cockroachdb:master Nov 23, 2022
@irfansharif irfansharif deleted the 221104.rditer-span branch November 23, 2022 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants