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

Open transactions after validating data #318

Closed
invidian opened this issue Oct 2, 2020 · 1 comment · Fixed by #369
Closed

Open transactions after validating data #318

invidian opened this issue Oct 2, 2020 · 1 comment · Fixed by #369
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. hacktoberfest priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/S estimate of the amount of work to address the issue

Comments

@invidian
Copy link
Contributor

invidian commented Oct 2, 2020

Expected Behaviour

As validation code do not access the database, it can be executed outside transaction to reduce transaction time.

Current Behaviour

Currently transaction is opened before data is validated, which extends lock time on database, which increases the latency on multiple requests: https://github.com/tinkerbell/tink/blob/master/db/template.go#L17-L30.

Possible Solution

Validate data first, then open transaction.

@gianarb gianarb added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. hacktoberfest priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/S estimate of the amount of work to address the issue labels Oct 2, 2020
@vbhv007
Copy link

vbhv007 commented Oct 2, 2020

Hey, Can I pick this up under hacktoberfest?

@mergify mergify bot closed this as completed in #369 Nov 18, 2020
mergify bot added a commit that referenced this issue Nov 18, 2020
## Description
Moves the code for data validation before creating the db transaction.

## Why is this needed
Fixes: #318 

## How Has This Been Tested?
Ran `make test` with Linux host.
Cbkhare pushed a commit to Cbkhare/tink that referenced this issue Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. hacktoberfest priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/S estimate of the amount of work to address the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants