-
Notifications
You must be signed in to change notification settings - Fork 886
Add GetSubrange for LittDB #3815
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
base: main
Are you sure you want to change the base?
Changes from all commits
42b25f4
1e4d704
193af35
cdb21b2
c9e9140
5f6715f
6eae178
8d8f4b3
0ae8ed3
78ad13d
219d259
f0a4720
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,6 +113,74 @@ func (c *cachedTable) Get(key []byte) (value []byte, exists bool, err error) { | |
| return value, exists, nil | ||
| } | ||
|
|
||
| // GetSubrange reads only the [offset, offset+length) byte range of the value stored under key. | ||
| // | ||
| // Cache policy — subrange reads consult both caches, but never populate them: | ||
| // | ||
| // - Populating would be unsafe. Both caches are keyed by the full key and hold the full value, so | ||
| // storing a sub-range under that key would hand a later Get a partial value it is entitled to treat | ||
| // as the whole thing. | ||
| // - Consulting is safe, because every path agrees that a key's offsets are relative to that key's own | ||
| // logical value. Put stores value[sk.Offset:sk.Offset+sk.Length] under a secondary key, the base table | ||
| // addresses that secondary at the same aliased region on disk, and readCache holds whatever Get | ||
| // returned. A cache hit therefore slices exactly the bytes the base table would have read. | ||
| // - Consulting is also strictly cheaper. A hit means the value is already materialized in memory, so | ||
| // slicing it costs no I/O at all, where the base table would pay a keymap lookup plus a disk read. | ||
| // Bypassing the caches would make GetSubrange slower than plain Get for precisely the hot, | ||
| // recently-written values a subrange read is meant to serve. | ||
| // | ||
| // Because hits come from the same caches Get uses, the two agree on whether a key exists — including for | ||
| // a key already reclaimed from the base table by GC or TTL expiry but still resident in a cache. | ||
| // | ||
| // Caching sub-ranges themselves, under specially structured keys, would speed up misses as well, but that | ||
| // adds complexity we do not yet know we need. Revisit if profiling shows subrange misses are hot enough to | ||
| // justify it. | ||
| func (c *cachedTable) GetSubrange(key []byte, offset uint32, length uint32) (value []byte, exists bool, err error) { | ||
| // hot tracks whether the value was served from one of this table's caches (a "hot" read) for metrics. | ||
| var hot bool | ||
| if c.metrics != nil { | ||
| start := time.Now() | ||
| defer func() { | ||
| if exists && value != nil { | ||
| c.metrics.ReportReadOperation(c.Name(), time.Since(start), uint64(len(value)), hot) | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| stringKey := util.UnsafeBytesToString(key) | ||
|
|
||
| if cached, ok := c.writeCache.Get(stringKey); ok { | ||
| // The value was recently written. | ||
| hot = true | ||
| return subrangeOf(cached, offset, length) | ||
| } | ||
| if cached, ok := c.readCache.Get(stringKey); ok { | ||
| // The value was recently read in full. | ||
| hot = true | ||
| return subrangeOf(cached, offset, length) | ||
| } | ||
|
|
||
| value, exists, err = c.base.GetSubrange(key, offset, length) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] This fall-through (and the
This comment was marked as low quality.
Sorry, something went wrong. |
||
| if err != nil { | ||
| return value, exists, fmt.Errorf("failed to get subrange from base table: %w", err) | ||
| } | ||
| return value, exists, nil | ||
| } | ||
|
|
||
| // subrangeOf slices a cached full value, applying the same bounds check (and reporting it the same way) | ||
| // as the base table, so a cache hit and a cache miss are indistinguishable to the caller. | ||
| func subrangeOf(value []byte, offset uint32, length uint32) ([]byte, bool, error) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] This bounds check + capped three-index slice is now duplicated verbatim in three places (here, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Two small things here:
|
||
| end := uint64(offset) + uint64(length) | ||
| if end > uint64(len(value)) { | ||
| return nil, false, fmt.Errorf( | ||
| "subrange [%d, %d) is out of bounds for value of length %d", offset, end, len(value)) | ||
| } | ||
| // Capped (three-index) slice: without it the sub-range would carry spare capacity reaching into the | ||
| // rest of the cached value, and an append by the caller would silently overwrite the bytes that | ||
| // follow — corrupting the entry for every later reader. Capping forces such an append to allocate. | ||
| return value[offset:end:end], true, nil | ||
| } | ||
|
|
||
| func (c *cachedTable) Exists(key []byte) (exists bool, err error) { | ||
| _, exists = c.writeCache.Get(util.UnsafeBytesToString(key)) | ||
| if exists { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package disktable | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/sei-protocol/sei-chain/sei-db/db_engine/litt/types" | ||
| "github.com/sei-protocol/sei-chain/sei-db/db_engine/litt/util" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestGetSubrange exercises the GetSubrange read path across every disk-table implementation, both before | ||
| // a flush (served from the in-memory unflushed data cache) and after (served from the keymap + a bounded | ||
| // segment read). It checks that a sub-range read returns exactly value[offset:offset+length], that it | ||
| // composes with secondary keys (which alias a sub-range of the value), that a zero-length read is valid, | ||
| // that an out-of-bounds range errors, and that a missing key reports not-found without an error. | ||
| func TestGetSubrange(t *testing.T) { | ||
| t.Parallel() | ||
| for _, tb := range tableBuilders { | ||
| t.Run(tb.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| rand := util.NewTestRandom() | ||
| directory := t.TempDir() | ||
| tableName := rand.String(8) | ||
| table, err := tb.builder(time.Now, tableName, []string{directory}) | ||
| require.NoError(t, err) | ||
|
|
||
| // 0 1 2 3 4 | ||
| // 0123456789012345678901234567890123456789012 | ||
| value := []byte("the quick brown fox jumps over the lazy dog") | ||
| primary := []byte("primary") | ||
| // A secondary aliasing the strict sub-range "brown fox", to prove GetSubrange composes with a | ||
| // secondary key (its address already points at a sub-range of the value's bytes). | ||
| sk := &types.SecondaryKey{Key: []byte("brown-fox"), Offset: 10, Length: 9} | ||
| require.NoError(t, table.Put(primary, value, sk)) | ||
|
|
||
| valueLen := uint32(len(value)) | ||
|
|
||
| verify := func(stage string) { | ||
| t.Helper() | ||
|
|
||
| // The full range equals a plain Get. | ||
| got, ok, err := table.GetSubrange(primary, 0, valueLen) | ||
| require.NoError(t, err, stage) | ||
| require.True(t, ok, stage) | ||
| require.Equal(t, value, got, stage) | ||
|
|
||
| // Assorted sub-ranges, including zero-length reads in the middle and at the very end. | ||
| ranges := []struct{ off, length uint32 }{ | ||
| {0, 3}, // "the" | ||
| {4, 5}, // "quick" | ||
| {valueLen - 3, 3}, // "dog" | ||
| {10, 0}, // zero-length in the middle | ||
| {valueLen, 0}, // zero-length at the very end | ||
| } | ||
| for _, r := range ranges { | ||
| got, ok, err := table.GetSubrange(primary, r.off, r.length) | ||
| require.NoError(t, err, stage) | ||
| require.True(t, ok, stage) | ||
| require.NotNil(t, got, stage) | ||
| require.Equal(t, value[r.off:r.off+r.length], got, stage) | ||
| // The sub-range may alias the whole value in memory, so its capacity must stop at its | ||
| // length: otherwise an append by the caller would overwrite the bytes that follow. | ||
| require.Equal(t, len(got), cap(got), stage) | ||
| } | ||
|
|
||
| // A sub-range read of a secondary key stays within the secondary's aliased region. | ||
| got, ok, err = table.GetSubrange(sk.Key, 0, sk.Length) | ||
| require.NoError(t, err, stage) | ||
| require.True(t, ok, stage) | ||
| require.Equal(t, value[sk.Offset:sk.Offset+sk.Length], got, stage) | ||
|
|
||
| got, ok, err = table.GetSubrange(sk.Key, 6, 3) // "fox" within "brown fox" | ||
| require.NoError(t, err, stage) | ||
| require.True(t, ok, stage) | ||
| require.Equal(t, value[sk.Offset+6:sk.Offset+6+3], got, stage) | ||
|
|
||
| // A range that runs past the end of the value is an error. | ||
| _, _, err = table.GetSubrange(primary, valueLen-1, 5) | ||
| require.Error(t, err, stage) | ||
| _, _, err = table.GetSubrange(primary, valueLen+1, 0) | ||
| require.Error(t, err, stage) | ||
|
|
||
| // A missing key reports not-found with no error. | ||
| _, ok, err = table.GetSubrange([]byte("does-not-exist"), 0, 1) | ||
| require.NoError(t, err, stage) | ||
| require.False(t, ok, stage) | ||
| } | ||
|
|
||
| verify("before flush") | ||
| require.NoError(t, table.Flush()) | ||
| verify("after flush") | ||
|
|
||
| require.NoError(t, table.Drop()) | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -714,6 +714,60 @@ func (s *Segment) Read(key []byte, dataAddress types.Address) ([]byte, error) { | |
| return s.maybeDecompress(value) | ||
| } | ||
|
|
||
| // ReadSubrange fetches only the [offset, offset+length) byte range of the value identified by dataAddress, | ||
| // avoiding a read of the full value. | ||
| // | ||
| // For an uncompressed segment this issues a bounded read of exactly length bytes, seeking to the value's | ||
| // on-disk offset plus the requested sub-offset — so the I/O cost scales with length, not the full value. | ||
| // For a compressed segment the on-disk blob is a single compressed unit that cannot be sliced, so this | ||
| // falls back to reading and decompressing the whole value (via Read) and then slicing out the requested | ||
| // range. Either way the returned bytes are the plaintext value[offset:offset+length]. | ||
| // | ||
| // The requested range must lie within the value; otherwise an error is returned. | ||
| // | ||
| // It is only thread safe to read from a segment if the key being read has previously been flushed to disk. | ||
| func (s *Segment) ReadSubrange(key []byte, dataAddress types.Address, offset uint32, length uint32) ([]byte, error) { | ||
| // A compressed value cannot be partially read from disk: decompress the whole thing, then slice. The | ||
| // bounds check is against the decompressed length, since dataAddress.ValueSize() is the compressed size. | ||
| if s.IsCompressed() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] The compressed fallback is correct, but note the cost profile for the intended use case: reading N transactions out of one block means N full reads + N decompressions of the same value, i.e. strictly worse than one |
||
| value, err := s.Read(key, dataAddress) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| end := uint64(offset) + uint64(length) | ||
| if end > uint64(len(value)) { | ||
| return nil, fmt.Errorf("subrange [%d, %d) is out of bounds for value of length %d", | ||
| offset, end, len(value)) | ||
| } | ||
| // Capped (three-index) slice, so that every GetSubrange path returns a sub-range whose capacity | ||
| // stops at its length. The decompression buffer sliced here is not shared with another reader | ||
| // today, but keeping the guarantee uniform means callers never have to know which path served | ||
| // them before deciding whether an append is safe. | ||
| return value[offset:end:end], nil | ||
| } | ||
|
|
||
| // For an uncompressed value, dataAddress.ValueSize() is the exact value length, so we can bound the | ||
| // range and read only the requested bytes directly from disk. | ||
| end := uint64(offset) + uint64(length) | ||
| if end > uint64(dataAddress.ValueSize()) { | ||
| return nil, fmt.Errorf("subrange [%d, %d) is out of bounds for value of length %d", | ||
| offset, end, dataAddress.ValueSize()) | ||
| } | ||
|
|
||
| values, err := s.shardForAddress(dataAddress) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to resolve shard for read: %w", err) | ||
| } | ||
|
|
||
| // The value starts at dataAddress.Offset(); the sub-range starts offset bytes further in. Both operands | ||
| // and their sum are bounded by the value file size (< 2^32), so the uint32 addition cannot overflow. | ||
| value, err := values.read(dataAddress.Offset()+offset, length) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] The no-overflow argument in the comment holds for every case that matters, but not unconditionally: when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] |
||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read value subrange: %w", err) | ||
| } | ||
| return value, nil | ||
| } | ||
|
|
||
| // maybeDecompress decodes an on-disk value from a compressed segment (stripping the per-value algorithm | ||
| // tag and decompressing the body; see types.EncodeValue), or returns it unchanged if the segment is not | ||
| // compressed. All value reads (Segment.Read and SegmentReader.Read) pass through here so the on-disk | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion] The consult-but-never-populate policy is well argued for safety, but note the perf consequence for the motivating workload: extracting N transactions from one block on a cache-enabled table costs N keymap lookups + N file opens, where a single
Get+ N in-memory slices would cost one. SoGetSubrangeis pessimal exactly where the caller reads several ranges from the same value. Moot today (DefaultTableConfigsets both cache sizes to 0), and the doc already flags subrange-keyed caching as future work — but worth a line in that comment saying multi-range callers should prefer oneGetwhile the caches can't help them, so the follow-up producer doesn't loop this per tx.