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

Support bucket rewrite relabel #3707

Merged
merged 3 commits into from
May 27, 2021
Merged

Conversation

yeya24
Copy link
Contributor

@yeya24 yeya24 commented Jan 8, 2021

Signed-off-by: yeya24 yb532204897@gmail.com

Fixes #3069

Add relabel support to bucket rewrite.

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

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:

  1. Relabeling might add new symbols so I have to modify the symbol set and sort it before returning it. Besides, symbols can be deleted because the whole series might be dropped via relabeling.
  2. Series are not ordered as relabeling might change the order of series.

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.

@yeya24 yeya24 mentioned this pull request Jan 9, 2021
2 tasks
Base automatically changed from master to main February 26, 2021 16:30
@yeya24
Copy link
Contributor Author

yeya24 commented May 3, 2021

I will close this pr as I cannot figure out a way to pass the relabel test case without #3710.
If anyone wants to continue this feature, feel free to pick it up.

@yeya24 yeya24 closed this May 3, 2021
@yeya24
Copy link
Contributor Author

yeya24 commented May 21, 2021

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.

@yeya24 yeya24 reopened this May 21, 2021
@yeya24 yeya24 marked this pull request as ready for review May 21, 2021 22:53
@yeya24
Copy link
Contributor Author

yeya24 commented May 21, 2021

This pr should be ready for review.
Note: Right now it only works for raw blocks. Not for downsampled blocks. I guess that's the same for bucket rewrite delete command.

pkg/compactv2/modifiers.go Outdated Show resolved Hide resolved
@yeya24 yeya24 force-pushed the bucket-rewrite-relabel branch 3 times, most recently from 16abe27 to 98a38dc Compare May 22, 2021 01:47
onprem
onprem previously approved these changes May 23, 2021
Copy link
Member

@onprem onprem left a 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.

pkg/compactv2/compactor_test.go Outdated Show resolved Hide resolved
onprem
onprem previously approved these changes May 25, 2021
Copy link
Member

@onprem onprem left a comment

Choose a reason for hiding this comment

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

LGTM!

@onprem
Copy link
Member

onprem commented May 25, 2021

@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>
Copy link
Member

@onprem onprem left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@onprem onprem merged commit d8b21e7 into thanos-io:main May 27, 2021
@yeya24 yeya24 deleted the bucket-rewrite-relabel branch May 27, 2021 17:53
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.

Tool: In-Place Relabelling for older data.
3 participants