-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
cleanWal by commitLogId #4015
cleanWal by commitLogId #4015
Conversation
@@ -1133,7 +1133,7 @@ void NebulaStore::cleanWAL() { | |||
auto& part = partEntry.second; | |||
if (part->needToCleanWal()) { | |||
// clean wal by expired time | |||
part->wal()->cleanWAL(); | |||
part->cleanWal(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe unify all usage, either implement cleanWAL()
in Part
and Listener
, or just call part->wal()->cleanWAL(id)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM
// clean wal by log id | ||
listener->wal()->cleanWAL(listener->getApplyId()); | ||
// clean wal by commit log id | ||
listener->cleanWal(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listener
need to override it, because it use apply id, not commitId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job
// remove wal file that lastId is less than target | ||
auto iter = walFiles_.begin(); | ||
while (iter != walFiles_.end()) { | ||
if (iter->second->lastId() < id) { | ||
if (iter->second->lastId() < id && index < size - 2 && (now - iter->second->mtime() > walTTL)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do not use walTTL to limit the cleanWAL, what will happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then no wal would be deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good Job
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Description:
How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: