Skip to content

Dequeue atomic - #290

Open
AlbertoCasasOrtiz wants to merge 8 commits into
devfrom
dequeue-atomic
Open

Dequeue atomic#290
AlbertoCasasOrtiz wants to merge 8 commits into
devfrom
dequeue-atomic

Conversation

@AlbertoCasasOrtiz

Copy link
Copy Markdown
Member

Fixes #267.

Most changes are due to indentation. Real changes are the addition of atomic to make everything a single transaction:

with transaction.atomic():

Select for update, which locks the trial so other workers cannot modify or lock them. If already locked by another worker, it skips it.

.select_for_update(skip_locked=True)

And a status check that, if for any reason a worker gets a trial already locked and in a non-available status, will skip it:

if trial.status not in ["stopped", "reprocess"]:
    raise Http404

@AlbertoCasasOrtiz AlbertoCasasOrtiz changed the title WIP - Dequeue atomic Dequeue atomic Jun 30, 2026
@AlbertoCasasOrtiz

Copy link
Copy Markdown
Member Author

@carmichaelong I just fixed conflicts on this branch

@carmichaelong
carmichaelong requested a review from csherry04 July 30, 2026 21:41
Comment thread mcserver/views.py Outdated
Comment on lines 1667 to 1703

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@AlbertoCasasOrtiz It looks like maybe this second section got copied in with merge but it reassigning the things just assigned in the atomic transaction. The second version (not atomic) seems to maybe be the one with updated logic for the delayed uploads? Perhaps then can get rid of the first one and wrap the second one in an atomic transaction?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you @csherry04. I just updated it accordingly (to avoid conflicts with dev and duplicated code, I modified from original code again to be atomic).

@AlbertoCasasOrtiz

Copy link
Copy Markdown
Member Author

I just created a new test for this. Also, changed some database calls to be more efficient.

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