Add exponential backoff

[sylane]

I don't really know how to add a test for this, any idea ?

[matlaj]

Maybe by connecting to some non-existent server?

[sylane]

Maybe by connecting to some non-existent server?

I already kind of run this code when testing the bad protocol version, what I don't know is how to validate that it is actually doing exponential backoff...

[matlaj]

Can you observe the state of the process, for example by calling sys:get_state/1?

[sylane]

Can you observe the state of the process, for example by calling sys:get_state/1?

I could, but I am wondering if this wouldn't be a bad idea. And to really test it properly I would have to try first without the server, kind of check it retries with a timing that kind of match expbackoff (it is random), then start the server, wait it connects, then shut down the server and check the delay reset... Seems like a lot of trouble...

[ziopio]

The bump in grisp version should appear in the commit history

[sylane]

@ziopio I added it to the changeling, but do you want me to remove it from the PR altogether ?

[ziopio]

It is fine just needs to appear in the commit history, once on main

[ziopio]

Basically what I am asking is just that the commit that increases the version tells that is doing it. Ideally, this should be a commit by itself.

[ziopio]

@GwendalLaurent bsl removes the need of rounding the result of math:pow

[sylane]

@ziopio You are right, the RetryCount -1 is wrong, I didn't really validated it yet.

[GwendalLaurent]

I would just add the link to the Amazon blog post and mention that we decided to go with the FullJitter algorithm. Like that anyone coming back to that code in a couple of weeks/months/years will know where the calculation comes from

[maehjam]

Why first add 1000 and then subtract it again? With the subtraction one always has to double check that the calculation never can lead to a negative delay.

[maehjam]

How about

Delay = 1000 * rand:uniform(min(64, (1 bsl RetryCount))) 

It's less expensive for rand:uniform/1, since it's a smaller interval. About code readability: it's clearly a positive value. If you want to start at 1s instead of 2s (assuming RetryCount >= 1; I haven't checked the code), you can also do 1 bsl (RetryCount - 1).

[sylane]

Ir RetryCount is always > 0, (1 bsl RetryCount) * 1000 is always >= 2000` so - 1000 cannot be negative. The 1000 + random() is to have always a minimum delay.

[sylane]

I want the value to be between 1000 and (2^RetryCount * 1000)

[maehjam]

Never mind the timer stuff. First one was of cause much slower.

[sylane]

So looking at this times: You need nearly half a second just to have a random number in steps of microseconds your way. That does not make sense at all.

YES it is bad. The whole point of exponential backoff, is to distribute the load of the client reconnecting to the server. The jitter done because without it the reconnection append in fixed time and overload the server every N seconds (see your own link for more details). Are we actually trying to optimise a call to random:uniform/1 happening at most every seconds on the grisp board while not connected at the cost of the goal of the exponential backoff itself ????

[maehjam]

The new version of the formula looks good.

[sylane]

The whole point is to spread the delay over the whole time zone, if you do the random like this they will all be clustered at multiples of 1000, this wouldn't make sense.

Why? What is so bad of doing retries in steps of seconds?

BTW:

1> timer:tc(random, uniform, [64000]).
{407,28390}
2> timer:tc(random, uniform, [64]).
{3,47}

Not sure where you get that from. On the board:

Eshell V15.2.2 (press Ctrl+G to abort, type help(). for help)
(grisp_demo@wasp)1> timer:tc(random, uniform, [64000]).
{33,28390}
(grisp_demo@wasp)2> timer:tc(random, uniform, [64000]).
{31,46275}
(grisp_demo@wasp)3> timer:tc(random, uniform, [64000]).
{31,60533}
(grisp_demo@wasp)4> timer:tc(random, uniform, [64000]).
{32,32096}

[maehjam]

As I said:

Never mind the timer stuff.

[peerst]

In the comments and anywhere else there are no units for time delays mentioned. Is it ms, µs or something else? I recommend always being clear about units everywhere (including possibly variable name suffix and function suffix). but certainly in all values that have a unit in comments. Otherwise I have to assume its count of potatoes ;-)