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

Replicate: Add flag to mark blocks for deletion after replication #7366

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Hatry1337
Copy link

@Hatry1337 Hatry1337 commented May 16, 2024

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

Changes

  • Added flag --mark-after to thanos tools bucket replicate
  • Changed replicationScheme.fromBkt type from InstrumentedBucketReader to InstrumentedBucket
  • Added forcing --ignore-marked-for-deletion if --mark-after set

Verification

  • Successfully built project
  • Ran replication between 2 buckets with --mark-after flag set
  • Monitored bucket contents for deletion marks
  • Made sure compactor removed marked blocks
  • Made sure blocks replicated successfully

Signed-off-by: Thomas Sidereal <53402621+Hatry1337@users.noreply.github.com>
Hatry1337 and others added 2 commits May 16, 2024 19:12
Signed-off-by: Thomas Sidereal <53402621+Hatry1337@users.noreply.github.com>
Signed-off-by: Thomas Sidereal <53402621+Hatry1337@users.noreply.github.com>

if !deletionMarkExists {
level.Debug(rs.logger).Log("msg", "marking block for deletion as already replicated", "block_uuid", blockID)
if err := block.MarkForDeletion(ctx, rs.logger, rs.fromBkt, id, "marked for deletion by thanos bucket replicate", promauto.With(nil).NewCounter(prometheus.CounterOpts{})); err != nil {
Copy link
Member

@GiedriusS GiedriusS May 20, 2024

Choose a reason for hiding this comment

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

Why do we have promauto.With(nil).NewCounter(prometheus.CounterOpts{})) here instead of adding something to replicationScheme.reg?

gotOrigin := len(originBucket.Objects())
gotTarget := len(targetBucket.Objects())

if gotOrigin != expectedOrigin {
Copy link
Member

Choose a reason for hiding this comment

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

We can use testutil.Equal()

@@ -274,6 +298,23 @@ func (rs *replicationScheme) ensureBlockIsReplicated(ctx context.Context, id uli
return errors.Wrap(err, "upload meta file")
}

if rs.markAfter {
deletionMarkFile := path.Join(id.String(), metadata.DeletionMarkFilename)
Copy link
Member

Choose a reason for hiding this comment

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

MarkForDeletion already checks for this, no? It could get quite expensive to check twice if one is replicating, let's say, 10000 blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants