Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bifrost] read correctly handles seal and tail instability #1725

Merged
merged 7 commits into from
Jul 23, 2024
Merged

Conversation

AhmedSoliman
Copy link
Contributor

@AhmedSoliman AhmedSoliman commented Jul 18, 2024

[Bifrost] read correctly handles seal and tail instability

  • Segments return the known tail_lsn which gets propagated to downstream components.
  • Renaming read_opt() to replace read() and making the non-blocking read() the de-facto way to read individual records. Readstream will be the only way for tailing readers.
  • Implements the intricate handling of read() during in-flight reconfiguration. This now requires bifrost to determine a safe known tail before allowing the read() to take place.
  • Testing read single record cross segments

Stack created with Sapling. Best reviewed with ReviewStack.

Copy link

github-actions bot commented Jul 18, 2024

Test Results

102 files  ±0  102 suites  ±0   22m 47s ⏱️ -13s
 84 tests ±0   84 ✅ ±0  0 💤 ±0  0 ❌ ±0 
217 runs  +2  217 ✅ +2  0 💤 ±0  0 ❌ ±0 

Results for commit 28d2239. ± Comparison against base commit bf9f0d1.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! LGTM. +1 for merging.

crates/bifrost/src/bifrost.rs Show resolved Hide resolved
crates/types/src/logs/metadata.rs Show resolved Hide resolved
A restructure the allows finer-grain control over what types loglet providers have access to. This will be exploited more in upcoming PRs.
- Major cleanup of loglet error handling, this removes the unnecessary leakage of internal loglet error types to bifrost.
- No more superflous `Arc<>` of non-Clone errors.
- FindTail on loglets cannot return Sealed error (enforcement via type-system)
This also adds an optional limit to loglet readstreams to enable creating a readstream with a pre-determined end offset.
In preparation to allow multi-segment chains, local loglets in single-node bootstrap now get unique random ids.
In preparation for the bifrost read_opt to be removed and replaced with `read()`
- Segments return the known tail_lsn which gets propagated to downstream components.
- Renaming `read_opt()` to replace `read()` and making the non-blocking `read()` the de-facto way to read individual records. Readstream will be the only way for tailing readers.
- Implements the intricate handling of `read()` during in-flight reconfiguration. This now requires bifrost to determine a safe known tail before allowing the `read()` to take place.
- Testing read single record cross segments
@AhmedSoliman AhmedSoliman merged commit 28d2239 into main Jul 23, 2024
9 checks passed
@AhmedSoliman AhmedSoliman deleted the pr1725 branch July 23, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants