Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Remove non-transactional mode #65

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Remove non-transactional mode #65

merged 1 commit into from
Feb 6, 2024

Conversation

bretthoerner
Copy link
Contributor

This drops the non-transactional mode, i.e. any usage of the running status.

Note that I didn't add a migration to drop the running status yet, changing an enum that's in use is moderately complicated in PG. It'd be better to do the next time we make a new shard (if we even care).

What I think we'd have to do (not worth it):

CREATE TYPE job_status_new AS ENUM('available', 'completed', 'failed');
ALTER TABLE job_queue ADD COLUMN status_new job_status_new;
-- Here we'd need to set all rows to the proper status, and make sure we get it right with transaction in flight and locked rows, etc?
ALTER TABLE job_queue DROP COLUMN status;
ALTER TABLE job_queue RENAME COLUMN status_new TO status;
DROP TYPE job_status;
ALTER TYPE job_status_new RENAME TO job_status;

@bretthoerner bretthoerner requested a review from a team February 5, 2024 18:22
@bretthoerner bretthoerner changed the title Don't hold connection for non-txn jobs Remove non-transactional mode Feb 5, 2024
@xvello
Copy link
Contributor

xvello commented Feb 5, 2024

Re: schema: I'd support keeping the column for now (and marking it as vestigial). If we find out that non-tx is needed, we would not have to change the schema back. As it's an enum, the overhead should be pretty small.

Copy link
Contributor

@tomasfarias tomasfarias left a comment

Choose a reason for hiding this comment

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

Happy to keep the enum for the time being, don't think it's much of an overhead 👍

Base automatically changed from brett/multi-get to main February 6, 2024 15:27
@bretthoerner bretthoerner merged commit 26672ae into main Feb 6, 2024
4 checks passed
@bretthoerner bretthoerner deleted the brett/drop-non-txn branch February 6, 2024 15:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants