-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[Bug][Compaction] Fix bug that output rowset is not deleted after compaction failure. #4964
Conversation
be/src/olap/reader.h
Outdated
@@ -128,7 +128,7 @@ class Reader { | |||
} | |||
|
|||
uint64_t filtered_rows() const { | |||
return _stats.rows_del_filtered; |
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.
Hi, is it compatible with segment v1?
In alpha_rowset_reader.h, filtered_rows() only calculates rows_del_filtered.
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.
Yes, it is compatible. Because rows_conditions_filtered is only used for SegmentV2, so it should always to 0 in Segment V1.
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.
LGTM
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.
+1
ec5186b
to
8ea8fa5
Compare
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.
+1
Proposed changes
This CL fix 2 bugs:
When the compaction fails, we must explicitly delete the output rowset,
otherwise the GC logic cannot process these rows.
Base compaction failed if compaction process include some delete version in SegmentV2,
Because the number of filtered rows is wrong.
Types of changes
Checklist