Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jobs: remove WithTxn #57534

Closed
ajwerner opened this issue Dec 4, 2020 · 1 comment · Fixed by #60554
Closed

jobs: remove WithTxn #57534

ajwerner opened this issue Dec 4, 2020 · 1 comment · Fixed by #60554
Assignees
Labels

Comments

@ajwerner
Copy link
Contributor

ajwerner commented Dec 4, 2020

There is a WithTxn() method which will associate (and store) a *kv.Txn with the *jobs.Job object. That txn will be used in all of the methods which update the job state in the KV store. This is unfortunate on several fronts. Firstly, it's not always clear which methods operate on in-memory state vs on the key-value store because nothing takes a txn. Secondly, if you don't clear the transaction, then later you can run into trouble using a potentially already committed transaction.

Methods which write to the KV-store should take a transaction.

@ajwerner ajwerner added the A-jobs label Dec 4, 2020
@blathers-crl
Copy link

blathers-crl bot commented Dec 4, 2020

Hi @ajwerner, please add a C-ategory label to your issue. Check out the label system docs.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@thoszhang thoszhang self-assigned this Feb 11, 2021
@craig craig bot closed this as completed in be115b9 Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants