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

check for partition being synced before starting errgroup #1401

Merged
merged 10 commits into from
Mar 7, 2024

Conversation

heavycrystal
Copy link
Contributor

No description provided.

@heavycrystal heavycrystal requested a review from serprex February 29, 2024 09:58
@serprex
Copy link
Contributor

serprex commented Feb 29, 2024

Why is this necessary since Sync still checks this?

@@ -81,3 +81,10 @@ func (c *S3Connector) SetupQRepMetadataTables(_ context.Context, config *protos.
c.logger.Info("QRep metadata setup not needed for S3.")
return nil
}

// S3 doesn't appear to check if the partition already exists
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment should be updated with reality. It doesn't check. Instead, S3 will always overwrite an existing partition

@heavycrystal heavycrystal enabled auto-merge (squash) March 7, 2024 17:58
@heavycrystal heavycrystal merged commit 0496eb5 into main Mar 7, 2024
7 checks passed
@heavycrystal heavycrystal deleted the qrep-syncedpart-handling branch March 7, 2024 18:05
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.

2 participants