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

[kv store] retry unprocessed items #14453

Merged
merged 1 commit into from
Oct 26, 2023
Merged

Conversation

phoenix-o
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Oct 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mysten-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 26, 2023 5:20pm
sui-typescript-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 26, 2023 5:20pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Oct 26, 2023 5:20pm
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Oct 26, 2023 5:20pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Oct 26, 2023 5:20pm

@vercel
Copy link

vercel bot commented Oct 26, 2023

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

@phoenix-o phoenix-o requested a review from sadhansood October 26, 2023 14:40
@vercel
Copy link

vercel bot commented Oct 26, 2023

@phoenix-o is attempting to deploy a commit to the Mysten Labs Team on Vercel.

A member of the Team first needs to authorize it.

if let Some(response) = response.unprocessed_items {
if let Some(unprocessed) = response.into_iter().next() {
if !unprocessed.1.is_empty() {
queue.push_back(unprocessed.1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if we want to add backoff here

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah backoff would be great, I wonder if there are settings in dynamodb client which let us configure retries and backoff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIU there's not, it's indeed a little bit confusing to not at least return the error
the reason I'm slightly hesitant about adding backoff is because of the concurrency model in the KV backfiller
we spawn N concurrent actors to process checkpoints, but store one single progress int value in "meta store" for the job
so it's updated only once there's continuation of progress from multiple actors
thus one slow actor can have impact on that metric. I guess we can try it anyway and see if there's any issue with this and it needs to be adjusted

if let Some(response) = response.unprocessed_items {
if let Some(unprocessed) = response.into_iter().next() {
if !unprocessed.1.is_empty() {
queue.push_back(unprocessed.1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah backoff would be great, I wonder if there are settings in dynamodb client which let us configure retries and backoff

@vercel vercel bot temporarily deployed to Preview – mysten-ui October 26, 2023 17:20 Inactive
@phoenix-o phoenix-o merged commit 141e83c into MystenLabs:main Oct 26, 2023
32 checks passed
jonas-lj pushed a commit to jonas-lj/sui that referenced this pull request Nov 2, 2023
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