Use exponential backoff with jitter #2419

j0holo · 2023-04-12T20:25:52Z

Description

Closes #1720.

I have added ctxutil.SleepWithJitter based on https://aws.amazon.com/ru/blogs/architecture/exponential-backoff-and-jitter/ (FullJitter).

I did not complete all tasks in the issue for the following reason:

time.Sleep is not used
context.WithTimeout is only used for actual timeout from what I'm reading. It does not make sense to add jitter to context timeouts if FerretDB does not retry something based on the context timing out.
ctxutil.Sleep has been replaced with a version that adds jitter with one exception: reporter.Run has a reportInternal duration that has no need for jitter.
All other ctxutil.Sleeps where located in for loops where the intention is clear that FerretDB is retrying some task. Adding jitter makes sense in these cases.

For the three unit tests I have added I had difficulty with making sure my tests are correct because SleepWithJitter uses rand.Int63n which makes it difficult to validate testing boundaries.

Readiness checklist

I added/updated unit tests.
I added/updated integration/compatibility tests.
I added/updated comments and checked rendering.
I made spot refactorings.
I updated user documentation.
I ran task all, and it passed. <-- returns package.txt: open package.txt: no such file or directory
I ensured that PR title is good enough for the changelog.
(for maintainers only) I set Reviewers (@FerretDB/core), Assignee, Labels, Project and project's Sprint fields.
I marked all done items in this checklist.

CLAassistant · 2023-04-12T20:25:59Z

All committers have signed the CLA.

AlekSi · 2023-04-12T20:32:42Z

@j0holo could you please squash your commits so there is only one with a email address that matches your GitHub email address?

codecov · 2023-04-12T20:37:44Z

Codecov Report

Merging #2419 (05a39a1) into main (486709f) will decrease coverage by 37.42%.
The diff coverage is 85.00%.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2419       +/-   ##
===========================================
- Coverage   64.32%   26.91%   -37.42%     
===========================================
  Files         392      392               
  Lines       19234    19251       +17     
===========================================
- Hits        12373     5182     -7191     
- Misses       5962    13514     +7552     
+ Partials      899      555      -344

Impacted Files	Coverage Δ
internal/handlers/tigris/tigrisdb/databases.go	`53.70% <0.00%> (ø)`
internal/clientconn/listener.go	`54.35% <33.33%> (-18.70%)`	⬇️
internal/handlers/pg/pgdb/transactions.go	`70.12% <100.00%> (-2.60%)`	⬇️
internal/util/ctxutil/ctxutil.go	`41.66% <100.00%> (-39.29%)`	⬇️

... and 122 files with indirect coverage changes

Flag	Coverage Δ
integration	`4.72% <0.00%> (-52.65%)`	⬇️
mongodb	`4.72% <0.00%> (-0.01%)`	⬇️
pg	`?`
unit	`25.57% <85.00%> (+0.21%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

j0holo · 2023-04-12T20:46:16Z

@j0holo could you please squash your commits so there is only one with a email address that matches your GitHub email address?

Yeah, I already saw that problem. Fixed it now.

chilagrow

Thanks for contributing! I have a few suggestions 🤗

cmd/envtool/envtool.go

internal/util/ctxutil/ctxutil.go

internal/handlers/pg/pgdb/transactions.go

internal/handlers/tigris/tigrisdb/databases.go

AlekSi · 2023-04-13T16:39:16Z

@j0holo Assuming that your git configuration includes email address now, please don't squash and force-push your commits anymore – we are squashing them on PR merge anyway, but that makes reviews harder.

@w84thesun PTAL

AlekSi · 2023-04-13T16:40:07Z

package.txt: open package.txt: no such file or directory

That's not an error, just a bit confusing message. I will fix it in another PR

AlekSi · 2023-04-13T16:43:05Z

@j0holo Please reformat your code with task fmt. We use https://github.com/mvdan/gofumpt that is a bit more opinionated than go fmt

w84thesun

Changes look reasonable to me! Let's fix the linter complaints and merge this!

…erretDB into exponential-backoff-with-jitter

j0holo · 2023-04-13T17:25:49Z

@j0holo Assuming that your git configuration includes email address now, please don't squash and force-push your commits anymore – we are squashing them on PR merge anyway, but that makes reviews harder.

I figured because I had to squash my changes in the beginning and because nothing is noted about in the contribution guide.

AlekSi · 2023-04-13T17:27:57Z

It is documented here – that's our extra guidelines for advanced contributors. :) But yeah, I think we could move a part of it to CONTRIBUTING.md

j0holo · 2023-04-13T17:35:16Z

I see, it would help for new contributors to do the right thing at once.

AlekSi · 2023-04-13T17:40:21Z

Created #2427

chilagrow

🚀

Closes FerretDB#1720.

j0holo requested a review from a team as a code owner April 12, 2023 20:25

j0holo requested review from AlekSi, w84thesun, rumyantseva and chilagrow April 12, 2023 20:25

AlekSi enabled auto-merge (squash) April 12, 2023 20:29

AlekSi added the code/chore Code maintenance improvements label Apr 12, 2023

AlekSi added this to the v1.1.0 milestone Apr 12, 2023

AlekSi assigned j0holo Apr 12, 2023

auto-merge was automatically disabled April 12, 2023 20:31
Head branch was pushed to by a user without write access

j0holo force-pushed the exponential-backoff-with-jitter branch from 531bd4c to 5065b5a Compare April 12, 2023 20:31

AlekSi requested review from a team and noisersup April 12, 2023 20:31

j0holo force-pushed the exponential-backoff-with-jitter branch from 5065b5a to 5653b38 Compare April 12, 2023 20:36

j0holo force-pushed the exponential-backoff-with-jitter branch 3 times, most recently from ad1fde5 to c2d9d1d Compare April 12, 2023 20:45

chilagrow reviewed Apr 13, 2023

View reviewed changes

cmd/envtool/envtool.go Outdated Show resolved Hide resolved

internal/util/ctxutil/ctxutil.go Outdated Show resolved Hide resolved

internal/handlers/pg/pgdb/transactions.go Outdated Show resolved Hide resolved

internal/handlers/tigris/tigrisdb/databases.go Outdated Show resolved Hide resolved

AlekSi removed request for rumyantseva and noisersup April 13, 2023 12:34

AlekSi enabled auto-merge (squash) April 13, 2023 12:35

AlekSi changed the title ~~Exponential backoff with jitter~~ Use exponential backoff with jitter Apr 13, 2023

AlekSi removed their request for review April 13, 2023 12:35

Add SleepWithJitter to reduce contention for resources in loops

4681e54

auto-merge was automatically disabled April 13, 2023 15:43
Head branch was pushed to by a user without write access

j0holo force-pushed the exponential-backoff-with-jitter branch from c2d9d1d to 4681e54 Compare April 13, 2023 15:43

Add SleepWithJitter to reduce contention for resources in loops

c8a27b6

j0holo force-pushed the exponential-backoff-with-jitter branch from 4681e54 to c8a27b6 Compare April 13, 2023 15:59

AlekSi requested a review from chilagrow April 13, 2023 16:39

AlekSi enabled auto-merge (squash) April 13, 2023 16:41

w84thesun previously approved these changes Apr 13, 2023

View reviewed changes

j0holo added 2 commits April 13, 2023 19:22

Run task fmt

33e5c5c

Merge branch 'exponential-backoff-with-jitter' of github.com:j0holo/F…

7b582a9

…erretDB into exponential-backoff-with-jitter

auto-merge was automatically disabled April 13, 2023 17:25
Head branch was pushed to by a user without write access

j0holo dismissed w84thesun’s stale review via 7b582a9 April 13, 2023 17:25

AlekSi requested a review from w84thesun April 13, 2023 17:31

AlekSi enabled auto-merge (squash) April 13, 2023 17:31

AlekSi approved these changes Apr 13, 2023

View reviewed changes

chilagrow approved these changes Apr 14, 2023

View reviewed changes

Merge branch 'main' into exponential-backoff-with-jitter

05a39a1

AlekSi merged commit 12342c7 into FerretDB:main Apr 14, 2023

AuruTus mentioned this pull request Apr 14, 2023

Document squash&push in contributing guidelines #2427

Closed

fenogentov pushed a commit to fenogentov/FerretDB that referenced this pull request Apr 17, 2023

Use exponential backoff with jitter (FerretDB#2419)

be00196

Closes FerretDB#1720.

fenogentov pushed a commit to fenogentov/FerretDB that referenced this pull request Apr 20, 2023

Use exponential backoff with jitter (FerretDB#2419)

c593e9f

Closes FerretDB#1720.

fenogentov pushed a commit to fenogentov/FerretDB that referenced this pull request Apr 20, 2023

Use exponential backoff with jitter (FerretDB#2419)

692a964

Closes FerretDB#1720.

fenogentov pushed a commit to fenogentov/FerretDB that referenced this pull request Apr 20, 2023

Use exponential backoff with jitter (FerretDB#2419)

96efb33

Closes FerretDB#1720.

fenogentov pushed a commit to fenogentov/FerretDB that referenced this pull request Apr 20, 2023

Use exponential backoff with jitter (FerretDB#2419)

235b9ea

Closes FerretDB#1720.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use exponential backoff with jitter #2419

Use exponential backoff with jitter #2419

j0holo commented Apr 12, 2023 •

edited

Loading

CLAassistant commented Apr 12, 2023 •

edited

Loading

AlekSi commented Apr 12, 2023

codecov bot commented Apr 12, 2023 •

edited

Loading

j0holo commented Apr 12, 2023

chilagrow left a comment

AlekSi commented Apr 13, 2023 •

edited

Loading

AlekSi commented Apr 13, 2023

AlekSi commented Apr 13, 2023

w84thesun left a comment

j0holo commented Apr 13, 2023

AlekSi commented Apr 13, 2023

j0holo commented Apr 13, 2023

AlekSi commented Apr 13, 2023

chilagrow left a comment

Use exponential backoff with jitter #2419

Use exponential backoff with jitter #2419

Conversation

j0holo commented Apr 12, 2023 • edited Loading

Description

Readiness checklist

CLAassistant commented Apr 12, 2023 • edited Loading

AlekSi commented Apr 12, 2023

codecov bot commented Apr 12, 2023 • edited Loading

Codecov Report

j0holo commented Apr 12, 2023

chilagrow left a comment

Choose a reason for hiding this comment

AlekSi commented Apr 13, 2023 • edited Loading

AlekSi commented Apr 13, 2023

AlekSi commented Apr 13, 2023

w84thesun left a comment

Choose a reason for hiding this comment

j0holo commented Apr 13, 2023

AlekSi commented Apr 13, 2023

j0holo commented Apr 13, 2023

AlekSi commented Apr 13, 2023

chilagrow left a comment

Choose a reason for hiding this comment

j0holo commented Apr 12, 2023 •

edited

Loading

CLAassistant commented Apr 12, 2023 •

edited

Loading

codecov bot commented Apr 12, 2023 •

edited

Loading

AlekSi commented Apr 13, 2023 •

edited

Loading