Optimize the DB updates to use bulk UPDATE instead of row-level locks.#13349
Open
sureshanaparti wants to merge 5 commits into
Open
Optimize the DB updates to use bulk UPDATE instead of row-level locks.#13349sureshanaparti wants to merge 5 commits into
sureshanaparti wants to merge 5 commits into
Conversation
…el locks Replace lockRows + per-row update loop with a single bulk UPDATE using the existing createForUpdate/UpdateBuilder/update(ub, sc, null) pattern (same as markHostsAsDisconnected in the same file). Eliminates N SELECT FOR UPDATE + N individual UPDATE round-trips during MS startup/reconnect. The lock was safe to remove because resetHosts only modifies rows where management_server_id = ourMS (no other MS targets these rows), and setting to NULL is idempotent with no read-dependent computation. A non-locking SELECT is issued before the bulk UPDATE when TRACE logging is enabled to preserve per-host-ID logging from the original code.
…evel locks Replace per-row lockRow + update + commit loop with a single bulk UPDATE using createForUpdate/UpdateBuilder/update(ub, sc, null) pattern. AlertDaoImpl.archiveAlert: reuses the existing SearchCriteria to issue one UPDATE alert SET archived=1 WHERE ... instead of N individual lock-update-commit cycles. EventDaoImpl.archiveEvents: builds an ID-based SearchCriteria from the pre-fetched event list and issues a single bulk UPDATE. Also fixes a latent NPE in both methods where lockRow returning null (row deleted concurrently) would cause the next line to throw.
…tead of row-level locks Replace lockRow/lockRows + update pattern in both updateStep overloads with createForUpdate + update(entity, sc) — the same pattern already used in findAndCleanupUnfinishedWork in the same file. updateStep(vmId, seqNum, step): replaces lockRows(LIMIT 1) + update with a single UPDATE ... SET step=? WHERE instance_id=? AND seq_no=?. updateStep(workId, step): replaces lockRow + null check + update with a single UPDATE ... SET step=? WHERE id=?. Non-existent row results in 0 rows affected — same no-op as the original null check. The locks were redundant because the work queue model guarantees single-owner access: take() assigns a serverId to each work item, and only the owning server updates its step.
Contributor
Author
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13349 +/- ##
============================================
- Coverage 18.10% 18.10% -0.01%
Complexity 16750 16750
============================================
Files 6037 6037
Lines 542798 542790 -8
Branches 66457 66455 -2
============================================
- Hits 98298 98290 -8
- Misses 433453 433456 +3
+ Partials 11047 11044 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18155 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
This PR optimizes the DB updates to use bulk UPDATE instead of row-level locks, here:
Eliminates N SELECT FOR UPDATE + N individual UPDATE round-trips during MS startup/reconnect.
The lock was safe to remove because resetHosts only modifies rows where management_server_id = MS (no other MS targets these rows), and setting to NULL is idempotent with no read-dependent computation.
Reuses the existing SearchCriteria to issue one UPDATE alert SET archived=1 WHERE ... instead of N individual lock-update-commit cycles.
Builds an ID-based SearchCriteria from the pre-fetched event list and issues a single bulk UPDATE.
updateStep(workId, step): replaces lockRow + null check + update with a single UPDATE ... SET step=? WHERE id=?. Non-existent row results in 0 rows affected — same no-op as the original null check.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?