feat: add time-based rotation to RotatingWriter (#136)#179
Merged
Conversation
RotatingWriter now rotates segments based on wall-clock time in addition
to file size. Rotation triggers when *either* condition is met:
- File exceeds `max_file_size` bytes (existing behavior)
- A wall-clock-aligned time boundary is crossed (new)
The default rotation period is 60 seconds, aligned to round minute
boundaries (e.g. if the writer starts at 14:03:22, the first rotation
fires at 14:04:00). This is configurable via the builder:
RotatingWriter::builder()
.rotation_period(Duration::from_secs(300)) // every 5 min
Time-aligned segments produce clean S3 key paths when using the
`worker-s3` feature (keys already use `{YYYY-MM-DD}/{HHMM}` bucketing).
Edge cases:
- Time rotation is skipped when no real events have been written
(avoids empty sealed segments)
- `single_file()` disables time rotation (Duration::MAX)
- Eviction budget is respected with frequent time rotations
Closes #136
rcoh
force-pushed
the
feature/time-based-rotation
branch
from
c110043 to
ec49afb
Compare
RotatingWriter now rotates segments based on wall-clock time in addition
to file size. Rotation triggers when *either* condition is met:
- File exceeds `max_file_size` bytes (existing behavior)
- A wall-clock-aligned time boundary is crossed (new)
The default rotation period is 60 seconds, aligned to round minute
boundaries (e.g. if the writer starts at 14:03:22, the first rotation
fires at 14:04:00). This is configurable via the builder:
RotatingWriter::builder()
.rotation_period(Duration::from_secs(300)) // every 5 min
Time-aligned segments produce clean S3 key paths when using the
`worker-s3` feature (keys already use `{YYYY-MM-DD}/{HHMM}` bucketing).
Edge cases:
- Time rotation is skipped when no real events have been written
(avoids empty sealed segments)
- `single_file()` disables time rotation (Duration::MAX)
- Eviction budget is respected with frequent time rotations
Closes #136
jlizen
approved these changes
Apr 10, 2026
| max_total_size: u64, | ||
| /// How often to rotate based on wall-clock time. `Duration::MAX` disables | ||
| /// time-based rotation (used by `single_file()`). | ||
| rotation_period: Duration, |
Member
There was a problem hiding this comment.
We should reject 0 seconds. Currently it rotates every batch.
yulnr
approved these changes
Apr 10, 2026
Comment on lines
+518
to
+521
| if !self.has_real_events && time_source().system_time() >= self.next_rotation_time { | ||
| self.next_rotation_time = | ||
| Self::next_boundary(time_source().system_time().as_std(), self.rotation_period); | ||
| } |
Collaborator
There was a problem hiding this comment.
very nit: these two system_time calls should logically be the same one, could capture once:
Suggested change
| if !self.has_real_events && time_source().system_time() >= self.next_rotation_time { | |
| self.next_rotation_time = | |
| Self::next_boundary(time_source().system_time().as_std(), self.rotation_period); | |
| } | |
| let now = time_source().system_time(); | |
| if !self.has_real_events && now >= self.next_rotation_time { | |
| self.next_rotation_time = | |
| Self::next_boundary(now.as_std(), self.rotation_period); | |
| } |
don't think it matters much in practice in this case though, just how it reads
… feature/time-based-rotation
Contributor
|
Bencher Report| Branch | feature/time-based-rotation |
| Testbed | ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| decode_1M_events | 📈 view plot 🚷 view threshold | 162.82 ms(-0.17%)Baseline: 163.09 ms | 203.86 ms (79.87%) |
| decode_1M_events_ref | 📈 view plot 🚷 view threshold | 139.22 ms(+15.67%)Baseline: 120.36 ms | 150.45 ms (92.54%) |
| decode_1M_events_visit | 📈 view plot 🚷 view threshold | 34.98 ms(-0.43%)Baseline: 35.14 ms | 43.92 ms (79.66%) |
| encode_1M_events | 📈 view plot 🚷 view threshold | 29.65 ms(+0.76%)Baseline: 29.43 ms | 36.79 ms (80.61%) |
Contributor
|
| Branch | feature/time-based-rotation |
| Testbed | ubuntu-latest |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) | Throughput | Benchmark Result operations / second (ops/s) x 1e3 (Result Δ%) | Lower Boundary operations / second (ops/s) x 1e3 (Limit %) |
|---|---|---|---|---|---|---|
| e2e::throughput_rps | 📈 view plot 🚷 view threshold | 23.83 ops/s x 1e3(-0.22%)Baseline: 23.89 ops/s x 1e3 | 17.91 ops/s x 1e3 (75.16%) | |||
| e2e::wall_time_ns | 📈 view plot 🚷 view threshold | 167.83 ms(+0.22%)Baseline: 167.46 ms | 209.33 ms (80.18%) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RotatingWriter now rotates segments based on wall-clock time in addition to file size. Rotation triggers when either condition is met:
max_file_sizebytes (existing behavior)The default rotation period is 60 seconds, aligned to round minute boundaries (e.g. if the writer starts at 14:03:22, the first rotation fires at 14:04:00). This is configurable via the builder:
Time-aligned segments produce clean S3 key paths when using the
worker-s3feature (keys already use{YYYY-MM-DD}/{HHMM}bucketing).Edge cases:
single_file()disables time rotation (Duration::MAX)Closes #136