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

Retry reads for blobs and trees if we can retry because of skeleton snapshots. #4427

Merged
merged 23 commits into from
Dec 3, 2020

Conversation

jatgarg
Copy link
Contributor

@jatgarg jatgarg commented Nov 20, 2020

Related to issue: #3963
With odsp driver moving to skeleton snapshots, we won't fetch compete snapshot as trees latest call and we don't do that already for r11s. So runtime should retry blob/tree read on retriable failures.
Provide the utility to retry on failures.

@jatgarg jatgarg added this to the November 2020 milestone Nov 20, 2020
@jatgarg jatgarg requested a review from vladsud November 20, 2020 07:46
@jatgarg jatgarg self-assigned this Nov 20, 2020
@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Nov 20, 2020

@fluidframework/base-host: +1.37 KB
Metric NameBaseline SizeCompare SizeSize Diff
main.js 163.1 KB 164.48 KB +1.37 KB
Total Size 163.1 KB 164.48 KB +1.37 KB
@fluid-example/bundle-size-tests: No change
Metric NameBaseline SizeCompare SizeSize Diff
container.js 184.32 KB 184.32 KB No change
map.js 45.19 KB 45.19 KB No change
matrix.js 143.79 KB 143.79 KB No change
odspDriver.js 193.53 KB 193.53 KB No change
sharedString.js 157.29 KB 157.29 KB No change
Total Size 724.12 KB 724.12 KB No change

Baseline commit: c03a8b2

Generated by 🚫 dangerJS against 5492291

@github-actions github-actions bot requested a review from vladsud November 23, 2020 20:20
@github-actions github-actions bot requested a review from vladsud December 1, 2020 05:49
@vladsud
Copy link
Contributor

vladsud commented Dec 4, 2020

Adding a bit more background about the defect:
this.maxThrottlingDelay in DeltaManager.ts tracks time when we expect to be fully online. I.e. as we get 429 error from the server to come back in 10 minutes, this.maxThrottlingDelay will be 10 min. The problem is that as time goes by, this value no longer reflects the reality - we need to track point in time, not duration, when it's over.

@jatgarg jatgarg deleted the blobretry branch December 4, 2020 21:39
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.

4 participants