-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Support bucket rewrite relabel #3707
Conversation
c7181bc
to
73c34e6
Compare
I will close this pr as I cannot figure out a way to pass the relabel test case without #3710. |
After some investigation, I found the root cause and I am able to fix the problem. Reopen this pr and I will try to tackle this again. |
73c34e6
to
d027647
Compare
This pr should be ready for review. |
d027647
to
bdf1241
Compare
16abe27
to
98a38dc
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.
LGTM. Great work! Just a small nit.
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!
@yeya24 Maybe let's add a CHANGELOG entry for this before merge. This is too awesome to not have a CHANGELOG 😄 |
Signed-off-by: yeya24 <yb532204897@gmail.com>
Signed-off-by: yeya24 <yb532204897@gmail.com>
Signed-off-by: yeya24 <yb532204897@gmail.com>
5420332
to
f835ad2
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.
LGTM. Thanks!
Signed-off-by: yeya24 yb532204897@gmail.com
Fixes #3069
Add relabel support to bucket rewrite.
Changes
Add a
RelabelModifier
for blocks relabeling. Unlike the deletion modifier which is used in a streaming way,RelabelModifier
cannot do this and the reasons are:These requirements mean that I have to sort almost all the things in the constructor. It is inefficient but currently I don't find a better way to solve it.
Verification
Unit tests added and manual testing.