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

compact: remove cancel on SyncMetas errors #5923

Merged
merged 1 commit into from
Dec 21, 2022

Conversation

clwluvw
Copy link
Contributor

@clwluvw clwluvw commented Nov 28, 2022

Changes

A follow-up on #5922
In a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah seenafallah@gmail.com

@fpetkovski
Copy link
Contributor

is cleanPartialMarked used somewhere else? And should we call cancel for non-retryable errors now?

@clwluvw
Copy link
Contributor Author

clwluvw commented Nov 28, 2022

is cleanPartialMarked used somewhere else? And should we call cancel for non-retryable errors now?

I think I need to edit my commit message, cleanPartialMarked is basically returning err but the ones surrounded by Repeat will return nil if it was a retriable error.
And by returning err on non-repeated scenarios (like !wait) and non-retriable errors, it will call the cancel in a favour of run.Group interrupt function:

}, func(error) {
	cancel()
})

in a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
@fpetkovski
Copy link
Contributor

This makes sense to me, but maybe someone with more knowledge around the compactor can take a look.

@clwluvw
Copy link
Contributor Author

clwluvw commented Nov 29, 2022

This makes sense to me, but maybe someone with more knowledge around the compactor can take a look.

@fpetkovski Can you request a review or mention them?

@clwluvw
Copy link
Contributor Author

clwluvw commented Dec 2, 2022

@bwplotka @GiedriusS Can you take a look, please?

@clwluvw
Copy link
Contributor Author

clwluvw commented Dec 16, 2022

ping @GiedriusS @bwplotka

@clwluvw
Copy link
Contributor Author

clwluvw commented Dec 21, 2022

@GiedriusS @bwplotka As a part of this change has been merged into master - if this doesn't come with that release, I think we will going to have a broken release for this part of the change. Can you please take a look before the new release?

Indeed it will leads to have non-healthy probs with reason=null

@bwplotka
Copy link
Member

Agree, can you base that PR to merge to release-0.30 branch please?

@clwluvw
Copy link
Contributor Author

clwluvw commented Dec 21, 2022

Agree, can you base that PR to merge to release-0.30 branch please?

@bwplotka I can cherry-pick the commit. But I think it needs to be on the master branch as well, shouldn't it?

@bwplotka bwplotka mentioned this pull request Dec 21, 2022
@bwplotka bwplotka changed the base branch from main to release-0.30 December 21, 2022 15:50
@bwplotka
Copy link
Member

We merge release branch to master later on, so all good. I changed the base branch

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks!

@bwplotka bwplotka merged commit 6e71787 into thanos-io:release-0.30 Dec 21, 2022
@clwluvw
Copy link
Contributor Author

clwluvw commented Jan 9, 2023

@bwplotka Just out of curiosity, when will you merge that to the main branch?:)

@GiedriusS
Copy link
Member

@clwluvw I'll merge this into main today 😄

GiedriusS pushed a commit that referenced this pull request Jan 17, 2023
in a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
saswatamcode pushed a commit that referenced this pull request Jan 18, 2023
* compact: remove cancel on SyncMetas errors (#5923)

in a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

* Cut v0.30.0-rc.0 (#5992)

* Cut v0.30.0-rc.0

Signed-off-by: bwplotka <bwplotka@gmail.com>

* mdox fix.

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Cut 0.30.0 (#6011)

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* *: cut 0.30.1 (#6017)

* fix duplicate metrics registration in redis client (#6009)

* fix duplicate metrics registration in redis client

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* fixed test

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* *: cut 0.30.1

Add CHANGELOG entry.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Tracing: Fix sampler defaults (#5887)

* Fix sampler defaults

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Add CHANGELOG

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Replace checkout with git-shallow-clone (#5829)

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* CHANGELOG: fix

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
Co-authored-by: Seena Fallah <seenafallah@gmail.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
Kartik-Garg added a commit to infracloudio/thanos that referenced this pull request Jan 18, 2023
Added re-try mechanism for store inital sync, where if the initial sync fails, it tries to do the initial sync again every 5 seconds for 15 seconds duration (total 3 re-tries for initial sync of store).

Signed-off-by: Kartik-Garg <kartik.garg@infracloud.io>

Store: Make initial sync more robust

Added re-try mechanism for store inital sync, where if the initial sync fails, it tries to do the initial sync again every 5 seconds for 15 seconds duration (total 3 re-tries for initial sync of store).

Signed-off-by: Kartik-Garg <kartik.garg@infracloud.io>

Merge release 0.30 into main (thanos-io#6041)

* compact: remove cancel on SyncMetas errors (thanos-io#5923)

in a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

* Cut v0.30.0-rc.0 (thanos-io#5992)

* Cut v0.30.0-rc.0

Signed-off-by: bwplotka <bwplotka@gmail.com>

* mdox fix.

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Cut 0.30.0 (thanos-io#6011)

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* *: cut 0.30.1 (thanos-io#6017)

* fix duplicate metrics registration in redis client (thanos-io#6009)

* fix duplicate metrics registration in redis client

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* fixed test

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* *: cut 0.30.1

Add CHANGELOG entry.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Tracing: Fix sampler defaults (thanos-io#5887)

* Fix sampler defaults

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Add CHANGELOG

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Replace checkout with git-shallow-clone (thanos-io#5829)

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* CHANGELOG: fix

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
Co-authored-by: Seena Fallah <seenafallah@gmail.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
sshantel pushed a commit to sshantel/thanos that referenced this pull request Jan 28, 2023
* compact: remove cancel on SyncMetas errors (thanos-io#5923)

in a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

* Cut v0.30.0-rc.0 (thanos-io#5992)

* Cut v0.30.0-rc.0

Signed-off-by: bwplotka <bwplotka@gmail.com>

* mdox fix.

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Cut 0.30.0 (thanos-io#6011)

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* *: cut 0.30.1 (thanos-io#6017)

* fix duplicate metrics registration in redis client (thanos-io#6009)

* fix duplicate metrics registration in redis client

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* fixed test

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* *: cut 0.30.1

Add CHANGELOG entry.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Tracing: Fix sampler defaults (thanos-io#5887)

* Fix sampler defaults

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Add CHANGELOG

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Replace checkout with git-shallow-clone (thanos-io#5829)

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* CHANGELOG: fix

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
Co-authored-by: Seena Fallah <seenafallah@gmail.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request Apr 17, 2023
* compact: remove cancel on SyncMetas errors (thanos-io#5923)

in a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

* Cut v0.30.0-rc.0 (thanos-io#5992)

* Cut v0.30.0-rc.0

Signed-off-by: bwplotka <bwplotka@gmail.com>

* mdox fix.

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Cut 0.30.0 (thanos-io#6011)

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* *: cut 0.30.1 (thanos-io#6017)

* fix duplicate metrics registration in redis client (thanos-io#6009)

* fix duplicate metrics registration in redis client

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* fixed test

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* *: cut 0.30.1

Add CHANGELOG entry.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Tracing: Fix sampler defaults (thanos-io#5887)

* Fix sampler defaults

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Add CHANGELOG

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Replace checkout with git-shallow-clone (thanos-io#5829)

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* CHANGELOG: fix

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
Co-authored-by: Seena Fallah <seenafallah@gmail.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request Apr 17, 2023
* compact: remove cancel on SyncMetas errors (thanos-io#5923)

in a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

* Cut v0.30.0-rc.0 (thanos-io#5992)

* Cut v0.30.0-rc.0

Signed-off-by: bwplotka <bwplotka@gmail.com>

* mdox fix.

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Cut 0.30.0 (thanos-io#6011)

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* *: cut 0.30.1 (thanos-io#6017)

* fix duplicate metrics registration in redis client (thanos-io#6009)

* fix duplicate metrics registration in redis client

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* fixed test

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>

* *: cut 0.30.1

Add CHANGELOG entry.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* Tracing: Fix sampler defaults (thanos-io#5887)

* Fix sampler defaults

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Add CHANGELOG

Signed-off-by: Matej Gera <matejgera@gmail.com>

* Replace checkout with git-shallow-clone (thanos-io#5829)

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>

Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

* CHANGELOG: fix

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
Signed-off-by: bwplotka <bwplotka@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
Signed-off-by: Kama Huang <kamatogo13@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
Co-authored-by: Seena Fallah <seenafallah@gmail.com>
Co-authored-by: Kama Huang <121007071+kama910@users.noreply.github.com>
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request May 18, 2023
in a favour of 86b4039 SyncMetas will retry if it's retriable.
Also, the cleanPartialMarked calls are surrounded by runutil.Repeat() will be repeated,
the ones not and are not retriable will throw an interrupt to run.Group() by returning err
and Group will call cancel() as it's configured for its interrupt func.

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

Signed-off-by: Seena Fallah <seenafallah@gmail.com>
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.

4 participants