osiris_log: Fix infinite recursion with segment size smaller than header - #217
osiris_log: Fix infinite recursion with segment size smaller than header#217the-mikedavis wants to merge 2 commits into
Conversation
The minimum size of any segment is always the header size (8 bytes, MAGIC plus 32 bit version). When setting the segment size bytes to a single byte, the transition to the second segment would always recurse indefinitely because transitioning to the next segment could never succeed. This change bails out and allows a 1-byte max segment size to mean a single chunk per segment.
There was a problem hiding this comment.
Pull request overview
Fixes an infinite recursion in osiris_log:write_chunk/6 that occurred when max_segment_size_bytes was set to a value smaller than or equal to the segment file header size (8 bytes). Because a freshly opened segment already accounts for ?LOG_HEADER_SIZE bytes, max_segment_size_reached/1 would always return true, causing write_chunk → open_new_segment → write_chunk to loop forever.
Changes:
- Guard the byte-based segment-roll predicate with
MaxSizeBytes > ?LOG_HEADER_SIZE, effectively disabling bytes-based rolling for tiny limits. - Add a regression test that writes two chunks with
max_segment_size_bytes => 1.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/osiris_log.erl | Skip byte-size segment roll when MaxSizeBytes is at or below the segment header size, preventing infinite recursion. |
| test/osiris_log_SUITE.erl | New write_with_small_max_segment_size test case covering the edge case. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
How does this fix interact with tracking snapshots whcih are written by the log itself? Really we should accept a tracking snapshot + 1 user chunk for every stream |
|
Hmm yeah, with a tiny max segment size plus tracking data, the tracking data would end up in one segment and the user chunk in the next. The simplest way to enforce this might be to set a minimum chunk count of 2 per segment. Not very fancy but seems like an easy minumum. |
This ensures that, no matter the max-segment-size-bytes configuration, a segment will always contain a tracking snapshot plus one user chunk. (I.e. if there is tracking data, the current segment will always start with a tracking snapshot.)
The minimum size of any segment is always the header size (8 bytes, MAGIC plus 32 bit version). When setting the segment size bytes to a single byte, the transition to the second segment would always recurse indefinitely because transitioning to the next segment could never succeed. This change bails out and allows a 1-byte max segment size to mean a single chunk per segment.
This is a total edge-case and not probably worth worrying about, but this is possible because the header
x-stream-max-segment-size-bytesdoes not check against a minimum. It is only checked to be non-negative and under a maximum.