[LogServer][Bifrost][compat] Native support for HLC timestamps#4173
Merged
AhmedSoliman merged 1 commit intomainfrom Jan 13, 2026
Merged
[LogServer][Bifrost][compat] Native support for HLC timestamps#4173AhmedSoliman merged 1 commit intomainfrom
AhmedSoliman merged 1 commit intomainfrom
Conversation
Test Results 7 files + 2 7 suites +2 3m 11s ⏱️ + 1m 54s Results for commit 9fed329. ± Comparison against base commit 96bd772. This pull request removes 34 and adds 47 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
AhmedSoliman
force-pushed
the
pr4173
branch
3 times, most recently
from
5925c06 to
8ec68cd
Compare
tillrohrmann
approved these changes
Jan 12, 2026
Contributor
tillrohrmann
left a comment
tillrohrmann
left a comment
There was a problem hiding this comment.
Thanks for creating this PR @AhmedSoliman. The changes look good to me. I only had some clarifying questions. +1 for merging.
Comment on lines
+49
to
+65
| #[derive(Debug, Clone, Default)] | ||
| #[repr(transparent)] | ||
| pub struct RecordFlags(u16); | ||
| bitflags! { | ||
| impl RecordFlags: u16 { | ||
| /// Record timestamp is in HLC format (UniqueTimestamp) | ||
| /// If unset, the timestamp is in NanosSinceEpoch format. | ||
| const HlcTimestamp = 0b00000000_00000001; | ||
| /// This flag indicates that instead of reading the payload directly from its position | ||
| /// in the message buffer, we should expect a u8 field that contains the number of extra | ||
| /// bytes the header occupies before the payload. | ||
| /// | ||
| /// This allows future versions to add extra header fields without breaking backwards | ||
| /// compatibility. | ||
| const ExtendedHeader = 0b00000000_00000010; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Why adding this to the local loglet? Are we planning on keeping the local loglet for the future?
Contributor
Author
There was a problem hiding this comment.
Until we completely remove it, I'd like to keep it compatible. Note that we currently have it as default in restate-lite which we might change, but until then, let's keep them in sync (loosely)
| /// compatibility. | ||
| const ExtendedHeader = 0b00000000_00000010; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Could we share the RecordFlags between the local loglet and the log-server?
Contributor
Author
There was a problem hiding this comment.
I don't like to create direct coupling between the two, let's keep them roughly+manually in sync until we have a clear decision on the future of local loglet.
Adds support for HLC timestamps and extended headers in the record format for both log-server and local_loglet storage layers. Note that in this commit we don't actually write the HLC timestamp nor the extended header, but we handle decoding them correctly for forward compatibility. Additionally, the new flag for ExtendedHeader allows arbitrary header fields to be added (based on flags) in the future. Fixes #4149 - Add RecordFlags bitflags with HlcTimestamp and ExtendedHeader flags - Decode HLC timestamps (UniqueTimestamp) when HlcTimestamp flag is set - Skip extended header bytes when ExtendedHeader flag is set (forward compat) - Add test cases for decoding records with various flag combinations - Fix missing ExtendedHeader handling in bifrost decode_custom_encoded_record
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Adds support for HLC timestamps and extended headers in the record format
for both log-server and local_loglet storage layers.
Note that in this commit we don't actually write the HLC timestamp nor the extended header, but we handle decoding them correctly for forward compatibility. Additionally,
the new flag for ExtendedHeader allows arbitrary header fields to be added (based on flags) in the future.
Fixes #4149
Stack created with Sapling. Best reviewed with ReviewStack.