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

[dbnode] Filter out corrupt commit log files that were not present before the node started in the commit log bootstrapper #1581

Merged
merged 4 commits into from
Apr 24, 2019

Conversation

richardartoul
Copy link
Contributor

What this PR does / why we need it:
Currently the commit log bootstrapper will only attempt to read commit log files that were present on disk before the node started to prevent it from trying to read the files that are currently being written to.

In addition to this check, we also need to add a guard against identifying the active file as corrupt. If the active file appears corrupt it will trigger logic in the commit log bootstrapper that makes configurable decisions about whether the commit log bootstrap should fail or whether the corrupt file should be ignored.

The existing code allowed callers to pass a predicate to the commit log iterator which could be used to filter out files that were not present before the node started, however, this filter was not being applied to files that appeared corrupt. In some rare scenarios, the active commit log file will appear corrupt because it hasn't been flushed yet and the commit log bootstrapper will thus think its encountered a corrupt file when in fact it should have never tried to read that file in the first place.

This P.R changes the iterator such that the predicate can be applied to files identified as corrupt as well, and modifies the commit log bootstrapper to take advantage of this new feature.
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:

None

Does this PR require updating code package or user-facing documentation?:

None

Copy link
Collaborator

@justinjc justinjc left a comment

Choose a reason for hiding this comment

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

LGTM

// which commitlogs the iterator should read from. If isCorrupt is true then f will contain
// a valid CommitLogFile, otherwise ErrorWithPath will contain an error and the path of the
// corrupt file.
type FileFilterPredicate func(isCorrupt bool, f persist.CommitLogFile, err ErrorWithPath) bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Can we join these fields into a single struct?

i.e.

type FileFilterInfo struct {
  File persist.CommitLogFile
  IsCorrupt bool
  Err ErrorWithPath
}

@richardartoul richardartoul force-pushed the ra/filter-corrupt-commitlog-files branch from 3d7f63d to 06708fe Compare April 24, 2019 14:06
@richardartoul richardartoul merged commit 00625a4 into master Apr 24, 2019
@richardartoul richardartoul deleted the ra/filter-corrupt-commitlog-files branch April 24, 2019 14:29
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.

3 participants