Skip to content

Conversation

@morningman
Copy link
Contributor

Proposed changes

When selecting candicate rowsets to do the cumulative compaction,
some rowsets may not be selected because the protection time has not expired.
Therefore, we need to find the current longest continuous version path in the candicate rowsets.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)

Checklist

When selecting candicate rowsets to do the cumulative compaction,
some rowsets may not be selected because the protection time has not expired.
Therefore, we need to find the current longest continuous version path in the candicate rowsets.
@morningman morningman added kind/fix Categorizes issue or PR as related to a bug. area/compact Issues or PRs related to the compact labels Sep 12, 2020
@morningman morningman self-assigned this Sep 12, 2020
@ZhangYu0123
Copy link
Contributor

LGTM

yangzhg
yangzhg previously approved these changes Oct 14, 2020
Copy link
Member

@yangzhg yangzhg left a comment

Choose a reason for hiding this comment

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

+1

@yangzhg yangzhg added the approved Indicates a PR has been approved by one committer. label Oct 14, 2020
for (; i < rowsets->size(); ++i) {
RowsetSharedPtr rowset = (*rowsets)[i];
if (rowset->start_version() != prev_rowset->end_version() + 1) {
LOG(WARNING) << "There are missed versions among rowsets. "
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it appropriate to print the warning log here?
I think this function only deals with specific logic, and the caller can decide whether to print the log. This allows this function to be used in more scenarios.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Co-authored-by: Zhao Chun <buaa.zhaoc@gmail.com>
Copy link
Member

@yangzhg yangzhg left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@imay imay left a comment

Choose a reason for hiding this comment

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

LGTM

@morningman morningman merged commit 588e5be into apache:master Oct 21, 2020
@yangzhg yangzhg mentioned this pull request Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. area/compact Issues or PRs related to the compact kind/fix Categorizes issue or PR as related to a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Routine Load][BE]Too many VersionCount cause by wrong compaction failure time

5 participants