RFC for log segmentation - #32
Conversation
agavra
left a comment
There was a problem hiding this comment.
Mostly LGTM, some food for thought before the green check though. Biggest concern is adding 8 bytes to each key.
|
|
||
| ``` | ||
| Log Entry: | ||
| | version (u8) | type (u8) | segment_id (u64 BE) | key (TerminatedBytes) | sequence (u64 BE) | |
There was a problem hiding this comment.
do we need a full u64 for segment id? u32 would allow 4B segments. adding this to every key seems pretty significant for a log structure. There are even varint encodings that maintain lexicographical ordering (see https://github.com/khonsulabs/ordered-varint for example, which we could easily vendor)
There was a problem hiding this comment.
alternatively, we can make sequence a u32 and make the total ordering segment + sequence so that we have u64 in total (sequences are unique within a segment)
There was a problem hiding this comment.
I agree 8 bytes is a lot. Another idea I was considering is to add an attribute flag packed into the record type to specify the segment_id size. We could use 4 bytes by default, but switch to 8 bytes if needed. We probably wouldn't need to implement it until we need it. Just need to know it is possible.
There was a problem hiding this comment.
I do like the idea to consider segment + sequence as a composite. One thing that bothers me a little bit is that we would have to bump when the u32 sequence is exhausted. Perhaps that's not a big deal, but I did kind of like the idea of representing "semantic segments" as we have with timeseries (each bucket represents an hour of time). It would be annoying to have to handle overflow segments. Perhaps we could consider using varlength u64 for the sequence number which always resets to 0? So u32 segment_id and varlength u64 sequence. This option wasn't possible until we had the fancypants delimiter for the key.
There was a problem hiding this comment.
Is the concern the storage overhead? I would imagine that gets optimized away pretty well by the prefix encoding in the SSTs
There was a problem hiding this comment.
that + memory overhead, I guess prefix compression would basically eliminate storage overhead.
There was a problem hiding this comment.
I implemented something close to what we discussed. First, I switched to a u32 segment encoding, Second, I changed the sequence number to be a varlength u64 which is defined relative to the base sequence from the SegmentMetadata.
There was a problem hiding this comment.
that's perfect since we'll get great prefix compression from the segment in slate but actually pretty bad compression from the sequence number so this may turn out to be better overall than the original design 👍
|
|
||
| 1. **On open**: When a new segment is created, a `SegmentMeta` record is written with `start_seq`, `start_time_ms`, and empty `user_meta`. This ensures the segment is immediately discoverable. | ||
|
|
||
| 2. **On seal**: When `seal_segment()` is called with user metadata, the `SegmentMeta` record is overwritten to include the user-provided bytes. If no user metadata is provided, the record is left unchanged. |
There was a problem hiding this comment.
we probably need to track the current segment id and increment it somewhere, right?
There was a problem hiding this comment.
I think we can just read the metadata in reverse order to find the last segment.
There was a problem hiding this comment.
I kind of expected we would keep the segment metadata in memory. It's tiny.
There was a problem hiding this comment.
makes sense, I had a brain fart that we could just compute it from the latest SST last key on restart
rodesai
left a comment
There was a problem hiding this comment.
This makes sense to me. One high level thought is whether the notion of a segment is something we should just bake into slatedb directly. It feels very similar to what we'd want with twcs. Need to think more about what that would look like.
|
|
||
| ``` | ||
| Log Entry: | ||
| | version (u8) | type (u8) | segment_id (u64 BE) | key (TerminatedBytes) | sequence (u64 BE) | |
There was a problem hiding this comment.
Is the concern the storage overhead? I would imagine that gets optimized away pretty well by the prefix encoding in the SSTs
|
|
||
| ``` | ||
| SegmentMeta Record: | ||
| Key: | version (u8) | type (u8=0x03) | segment_id (u64 BE) | |
There was a problem hiding this comment.
one thought: does it make sense to include the notion of a segment key and a segment id, and use the id as the key for internally generated segments? The idea being that when we support user-generated segments they can select their own keys that can be looked up later
There was a problem hiding this comment.
That's a nice idea. Do you think this could be added through a version bump later? Another interesting idea I was considering is to allow segments to be defined at prefix/range granularity. That might open the door to enforcing different retention semantics for different keys. I do think there might an interesting application design space here.
There was a problem hiding this comment.
I think these are neat ideas but I'm +1 to keeping things as simple as possible and only tinroducing those concepts if necessary later to avoid complicating things
|
|
||
| 1. **On open**: When a new segment is created, a `SegmentMeta` record is written with `start_seq`, `start_time_ms`, and empty `user_meta`. This ensures the segment is immediately discoverable. | ||
|
|
||
| 2. **On seal**: When `seal_segment()` is called with user metadata, the `SegmentMeta` record is overwritten to include the user-provided bytes. If no user metadata is provided, the record is left unchanged. |
There was a problem hiding this comment.
I think we can just read the metadata in reverse order to find the last segment.
@rodesai I agree that is the interesting question. I was wondering if the API in slatedb could work with minimal assumptions about key structure. For example, maybe we could have an API to mark a range of keys as "sealed" which makes those keys immutable and stops compaction? I think we need a design in slate sooner than later. Not sure this work needs to block behind it though? |
Totally agree. We can move forward with the approach here and revisit as we understand the problem better. |
|
If there are no objections, I'll merge this. |

This patch adds a logical segmentation notion which is similar to the way that time buckets work in timeseries. Segments can be created based on time-based boundaries, such as hourly like with timeseries. The user can also seal a segment based on their own application logic. This provides a useful foundation for cross-key operations. For example, when reading from a range of keys, we can use the segment to position reads across all keys in the range. It could also be used as a basis for performing cross-key retention semantics (e.g. drop all segments older than one day).