Reset TestPlanVersion nextval during import if IDs aren't used
#1153
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
howard-e
changed the title
nextval during import if IDs aren't used
howard-e
commented
Jul 16, 2024
| id | ||
| status | ||
| } | ||
| v2TestPlanVersion: testPlanVersion(id: 80) { |
There was a problem hiding this comment.
For reviewers: Formatting has messed with this diff. This id was changed to 80 from 133
| directory | ||
| } | ||
| } | ||
| recommendedTestPlanVersion: testPlanVersion(id: 67) { |
There was a problem hiding this comment.
For reviewers: Formatting has messed with this diff. This id was changed to 67 from 69
stalgiag
approved these changes
Jul 17, 2024
| { transaction } | ||
| ); | ||
| const currentTestPlanVersionId = | ||
| currentTestPlanVersionIdResult[0].currval - 1; |
There was a problem hiding this comment.
Took me a second to understand why currval -1 works here even with the comment helping explain but I eventually understood and the approach makes sense.
howard-e
added a commit
that referenced
this pull request
Jul 22, 2024
Includes the following changes: **Features and Fixes** * #1125 * #1135 * #1144 * #1146 * #1150 * #1152 * #1153 * #1160 **Infrastructure changes** * #969 * #1151 * #1148 --------- Co-authored-by: alflennik <[email protected]> Co-authored-by: Mx Corey Frang <[email protected]> Co-authored-by: cypress evelyn masso <[email protected]> Co-authored-by: Stalgia Grigg <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was inspecting the production's data from July 10 2024 and took note of significantly high id numbers in the
TestPlanVersiontable. The latest row's id is163615but there is just 103 rows in the table, which uses auto-increment IDs. Runningselect currval(pg_get_serial_sequence('"TestPlanVersion"', 'id'))also gives175465.This happens because the
import-testsscript is dependent on knowing the "next" id to create relevant data that is retrieved using postgres' nextval (which also also updates the next possible ID value). The issue with this is that those ids could go unused if a potential TestPlanVersion being created is "discarded" if non-unique data is found, but the incremented value is left as is.The amount of test plan versions that could be imported is 35 and the cronjob responsible for doing that should run every 15 minutes . So potentially, the incremented value is unnecessarily bumped 140 times per hour (105 at a minimum) if there are no new test plan versions.
So this PR does the following:
nextvalif it hasn't been used due to that non-unique data condition (continued usingnextvalfor the evaluation of the reset value becausecurrvalorlastvalmay not successfully run ifnextvalhasn't been ran during the db's transaction session)nextvalto current latest ID + 1 in a migration