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

Remove MessageLogIter, use commit_log::Iter instead #272

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

kim
Copy link
Contributor

@kim kim commented Sep 11, 2023

Depends on #265

@kim kim requested review from cloutiertyler and aasoni September 11, 2023 12:55
for message in message_log.iter() {
for commit in commit_log::Iter::from(message_log.segments()) {
let commit = commit?;

segment_index += 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aasoni Unsure why this is named segment_index when it counts commits (i.e. messages). If it is supposed to count commits, this could be further simplified via .enumerate().

Copy link
Contributor

Choose a reason for hiding this comment

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

this may be a question for @cloutiertyler, I've only rearranged things in this function and added debug logging. Not sure why the original naming decision

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay... I guess I'll just leave it like that, then. Although it looks to me like last_commit_offset would suffice to compute the percentage.

@cloutiertyler cloutiertyler merged commit ba98666 into kim/log-access Oct 3, 2023
5 checks passed
cloutiertyler pushed a commit that referenced this pull request Oct 3, 2023
* core: Provide read access to commit/message log and odb

This is a first approximation to provide data access for Kafka-style
replication. It punts on notifications of segment rotation (which
could also be solved via filesystem events).

ObjectDB access is fairly crude, and will likely require it's own
replication subsystem.

* fixup! Merge remote-tracking branch 'origin/master' into kim/log-access

* Add some tests

Also make max segment size configurable, so tests don't have to write
>= 1GiB worth of data.

* Abide to the Rust naming conventions

* Add test for commit iter

* Remove `MessageLogIter`, use `commit_log::Iter` instead (#272)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants