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] rename bifrost read functions #1722

Merged
merged 6 commits into from
Jul 23, 2024
Merged

[Bifrost] rename bifrost read functions #1722

merged 6 commits into from
Jul 23, 2024

Conversation

Copy link

github-actions bot commented Jul 18, 2024

Test Results

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

Results for commit a4af555. ± 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.

LGTM. +1 for merging.

from: Lsn,
) -> Result<Option<LogRecord>> {
self.inner.read_next_single_opt(log_id, from).await
pub async fn read_opt(&self, log_id: LogId, from: Lsn) -> Result<Option<LogRecord>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we going to remove this method as stated in the commit message?

Copy link
Contributor

Choose a reason for hiding this comment

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

The next commit answers this question. Yes this method will get removed.

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()`
@AhmedSoliman AhmedSoliman merged commit a4af555 into main Jul 23, 2024
9 checks passed
@AhmedSoliman AhmedSoliman deleted the pr1722 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