Skip to content

fix: claim update slot atomically#427

Open
timkambic-ngen wants to merge 2 commits into
UpstreamDataInc:masterfrom
timkambic-ngen:fix-atomic-update-slot-claim
Open

fix: claim update slot atomically#427
timkambic-ngen wants to merge 2 commits into
UpstreamDataInc:masterfrom
timkambic-ngen:fix-atomic-update-slot-claim

Conversation

@timkambic-ngen

Copy link
Copy Markdown
Contributor

While working on #425 I noticed the non atomic update slot handling. Which I introduced in #290

We have seen the max_concurrent_updates limit being breached quite a lot on our deployment. Especially when a new rollout is triggered and all devices compete for the slots.

This PR introduces a new state RESERVED which is atomically claimed and assigned to a device. After the device reports progress it's state is changed to RUNNING - same behavior as before.

Note: I have also considered to change device's state directly to RUNNING as soon as the slot is claimed but it introduced quite some behavioral changes.

@timkambic-ngen

Copy link
Copy Markdown
Contributor Author

Tested this also in our production setting with lots of devices. Before it would overshoot the configured max_concurrent_updates limit of 50 by 10-15devices on new rollout trigger.
Now it correctly limits to the configured amount of slots

@timkambic-ngen
timkambic-ngen marked this pull request as ready for review July 21, 2026 13:10
@b-rowan

b-rowan commented Jul 21, 2026

Copy link
Copy Markdown
Member

Any chance we can avoid the raw SQL? If not then this is fine as is, I will approve and merge, just want a sanity check, and my SQL skills are not great.

@timkambic-ngen

Copy link
Copy Markdown
Contributor Author

Any chance we can avoid the raw SQL? If not then this is fine as is, I will approve and merge, just want a sanity check, and my SQL skills are not great.

Dropped the raw sql and used tortoise. Looks cleaner

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