internal/keyspan: rename from rangedel#1352
Conversation
|
This change is |
nicktrav
left a comment
There was a problem hiding this comment.
Naming is hard :)
Reviewed 36 of 36 files at r1, 13 of 25 files at r2, 3 of 5 files at r3, 2 of 2 files at r4, 3 of 3 files at r6, 5 of 5 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @sumeerbhola)
sumeerbhola
left a comment
There was a problem hiding this comment.
I didn't read very carefully since most of it looked mechanical -- let me know if there is something I should pay attention to.
Reviewed 3 of 36 files at r1, 12 of 25 files at r2, 3 of 5 files at r3, 2 of 2 files at r4, 3 of 3 files at r6, 4 of 5 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jbowens)
internal/keyspan/span.go, line 16 at r8 (raw file):
// Span is a span of key space. A spans covers all of the keys in the // range [start,end). Note that the start key is inclusive and the end // key is exclusive.
How about saying something like:
Span represents a key-kind over a span of user keys, along with the seqnum at which it was written. So conceptually it represents (key-kind, [start, end))#seqnum. Since we have an existing InternalKey struct that can represent (key-kind, user key, seqnum) we stuff the start user-key in there, which is the Start field, and the end user-key is in the End field. Currently the only supported key-kind is RANGEDEL.
Rename the internal/rangedel package to internal/keyspan. The rangedel.Tombstone type is renamed to keyspan.Span. This commit is in anticipation of additional kinds of keys that span a region of user key space (specifically, 'range keys' for modeling MVCC range deletes).
7811a6b to
e1aeb79
Compare
jbowens
left a comment
There was a problem hiding this comment.
TFTRs!
Reviewable status: 36 of 39 files reviewed, 1 unresolved discussion (waiting on @nicktrav and @sumeerbhola)
internal/keyspan/span.go, line 16 at r8 (raw file):
Previously, sumeerbhola wrote…
How about saying something like:
Span represents a key-kind over a span of user keys, along with the seqnum at which it was written. So conceptually it represents(key-kind, [start, end))#seqnum. Since we have an existingInternalKeystruct that can represent (key-kind, user key, seqnum) we stuff the start user-key in there, which is theStartfield, and the end user-key is in theEndfield. Currently the only supported key-kind is RANGEDEL.
Went with:
// Span represents a key-kind over a span of user keys, along with the
// sequence number at which it was written.
//
// Conceptually it represents (key-kind, [start, end))#seqnum. The key
// kind, start and sequence number are stored in a base.InternalKey
// struct in the Start field out of convenience. The end user key is
// stored separately as the End field.
//
// Note that the start user key is inclusive and the end user key is
// exclusive.
//
// Currently the only supported key kind is RANGEDEL.
This pull request renames the internal/rangedel package and its
Tombstone type to a new internal/keyspan and Span type in anticipation
of this package being used to represent key spans other than those
stemming from range deletion tombstones (specifically, range keys
#1341).
Nothing within this package deals with range tombstone idiosyncrasies
beyond representation and fragmentation of user key ranges with
inclusive start keys and exclusive end keys.
I structured this pull request as a series of small individual commits,
because I believe it makes it easier to see what's changed, especially
in the presence of file renames. I plan on squashing the commits before
merging.
I welcome alternative naming that avoids the stutter of
keyspan.Span.