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

Broker recovery #1667

Merged
merged 3 commits into from
Feb 21, 2015
Merged

Broker recovery #1667

merged 3 commits into from
Feb 21, 2015

Conversation

benbjohnson
Copy link
Contributor

Overview

This pull request fixes the broker recovery so that it determines the last index from the various topic logs instead of persisting the snapshot on every message that comes in.

This commit fixes the broker recovery so that it determines the last index
from the various topic logs instead of persisting the snapshot on every
message that comes in.
@@ -57,6 +57,14 @@ func (b *Broker) metaPath() string {
return filepath.Join(b.path, "meta")
}

// Index returns the highest index seen by the broker.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: "seen" is a vague term. Could it be improved?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess somewhere we need to capture that the significance of this is that highest across all topics has been safely applied in the Raft log, if I remember correctly.

@otoolep
Copy link
Contributor

otoolep commented Feb 21, 2015

Some suggested changes, but this code makes sense to me. I would like to see the test tweaked.

+1

@@ -774,6 +791,33 @@ func (t *topic) Close() error {
return nil
}

// loadIndex reads the highest available index for a topic from disk.
func (t *topic) loadIndex() error {
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it's going to take a long time if there are quite a bit of topic logs backed up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we add truncation (and log segments) then it'll only have to traverse through the latest log segment for each topic.

@pauldix
Copy link
Member

pauldix commented Feb 21, 2015

LGTM, :shipit:

benbjohnson added a commit that referenced this pull request Feb 21, 2015
@benbjohnson benbjohnson merged commit 0be9a1f into master Feb 21, 2015
@benbjohnson benbjohnson deleted the broker-recovery branch February 21, 2015 22:31
mark-rushakoff pushed a commit that referenced this pull request Jan 11, 2019
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