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 1 commit 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 |
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.