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

avoid unnecessary live file query and fix double referencing moved file #240

Closed
wants to merge 6 commits into from

Conversation

tabokie
Copy link
Member

@tabokie tabokie commented Jun 25, 2021

Signed-off-by: tabokie xy.tao@outlook.com

Obsolete file candidates consist of obsolete_files_ that are no longer referenced by any version in VersionSet, and all files in DB directory if doing_full_scan is true. By design, obsolete_files_ is guaranteed to be obsolete and thus no need to be checked against live files list. This PR avoids the unnecessary live file query when not doing full scan.

The optimization depends on the assumption that each physical SST file is represented by an unique FileMetaData structure, which is actually broken in existing codebase: A trivial file move will create a new file meta for the same file number. This patch also fix this issue.

Signed-off-by: tabokie <xy.tao@outlook.com>
Signed-off-by: tabokie <xy.tao@outlook.com>
@yiwu-arbug
Copy link
Collaborator

Sorry for delay, will get to this today or tomorrow.

void Ref() { ++refs; }

bool Unref() {
--refs;
Copy link
Member

Choose a reason for hiding this comment

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

How about making ref be private?

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

db/version_builder.cc Show resolved Hide resolved
Copy link
Collaborator

@yiwu-arbug yiwu-arbug left a comment

Choose a reason for hiding this comment

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

Add more comments overall?

db/db_impl/db_impl_files.cc Outdated Show resolved Hide resolved
db/version_builder.cc Show resolved Hide resolved
db/version_builder.cc Outdated Show resolved Hide resolved
Signed-off-by: tabokie <xy.tao@outlook.com>
Signed-off-by: tabokie <xy.tao@outlook.com>
@yiwu-arbug
Copy link
Collaborator

Looking good overall. Let's send to upstream and see?

@yiwu-arbug
Copy link
Collaborator

duplicated with #250

@yiwu-arbug yiwu-arbug closed this Jul 22, 2021
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