Skip to content

Fixed bug in exponential retry intervals#83

Merged
britzl merged 3 commits into
masterfrom
fix-exponential-retries
Aug 26, 2025
Merged

Fixed bug in exponential retry intervals#83
britzl merged 3 commits into
masterfrom
fix-exponential-retries

Conversation

@britzl
Copy link
Copy Markdown
Collaborator

@britzl britzl commented Apr 10, 2025

Fixed crash when creating an exponential retry interval. Also added tests for the retries.lua module.

./nakama/util/retries.lua:12: attempt to perform arithmetic on a nil value (field '?')
stack traceback:
	./nakama/util/retries.lua:12: in function 'nakama.util.retries.exponential'
	test/test_retries.lua:11: in function <test/test_retries.lua:10>
	[C]: in function 'xpcall'
	./telescope.lua:418: in local 'invoke_test'
	./telescope.lua:458: in function 'telescope.run'
	./tsc:274: in main chunk
	[C]: in ?

@britzl britzl requested a review from DannyIsYog April 10, 2025 06:36
@britzl britzl requested a review from novabyte August 14, 2025 07:52
@britzl
Copy link
Copy Markdown
Collaborator Author

britzl commented Aug 14, 2025

@DannyIsYog could you please review this PR and the other's in this repo assigned to you? Thanks!

Copy link
Copy Markdown
Contributor

@DannyIsYog DannyIsYog left a comment

Choose a reason for hiding this comment

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

Hey @britzl this looks good!

I just have a suggestions for a future PR.
I'm not very familiar with retries so I was looking at how it's done on our other sdks and noticed most (if not all) use retries with jitter. This is, if a large number of clients all start retrying a failed service at the same time, this predictable sequence can lead to a problem, where all clients hit the service simultaneously after the same delay, causing it to fail again, since the pattern is "predictable". To fix this, we should add a bit of randomness to this so to "spread" when the clients will re-hit the server.

You can see something like that for the Nakama Unity SDK

Let me know if this makes sense.

@britzl
Copy link
Copy Markdown
Collaborator Author

britzl commented Aug 26, 2025

Let me know if this makes sense.

It totally does. I added an issue to track this: #86

- uses: actions/checkout@v3
name: Checkout project

- uses: leafo/gh-actions-lua@v9
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Seems like leafo/gh-actions-lua has some issues since a few months back. There's a PR to fix it but the author has not merged it. We might as well just install Lua using apt-get.

@britzl britzl merged commit 3a0447a into master Aug 26, 2025
3 checks passed
@britzl britzl deleted the fix-exponential-retries branch August 26, 2025 05:36
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