-
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
compact: retry on sync metas error #5865
Conversation
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.
Thanks, this makes sense to me. I just wonder if we should have some backoff or keep retrying forever.
As we are exposing the metric for it (retries) and the interval is not low by default, I think it's better to leave it to users by configuring the |
As SyncMetas is surrounded by Repeat func and can return retry errors, in some cases (like S3) errors (network issue, timeout, etc.) can be retried. Signed-off-by: Seena Fallah <seenafallah@gmail.com>
450d29d
to
48dece5
Compare
@fpetkovski I also add this change to the changelog, so users will inform about this new behavior. |
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.
Looks good! Thanks.
As SyncMetas is surrounded by Repeat func and can return retry errors, in some cases (like S3) errors (network issue, timeout, etc.) can be retried. Signed-off-by: Seena Fallah <seenafallah@gmail.com> Signed-off-by: Seena Fallah <seenafallah@gmail.com>
Changes
As SyncMetas is surrounded by Repeat func and can return retry errors, in some cases (like S3) errors (network issue, timeout, etc.) can be retried.
Signed-off-by: Seena Fallah seenafallah@gmail.com
Verification
Currently in case of any errs on
SyncMetas
it will gracefully shutdown the HTTP server, It can be verified by e.g. issuing timeout on SyncMetas func