Optimize UsageJobDaoImpl.updateJobSuccess to use direct UPDATE instead of row-level lock

[vishesh92]

Description

Replace lockRow + explicit transaction with a single update(jobId, jobForUpdate) call. All field values are parameters passed by the caller — nothing is read from the locked row.

The TransactionLegacy.open(USAGE_DB) is kept since this method explicitly targets the usage database. The txn.start/commit/rollback are removed since the single update() call is autocommit.

The lock was redundant because the usage server model is single-owner: one server processes one job at a time.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Compilation and static analysis:

• mvn -pl engine/schema -am compile -DskipTests — compiles clean
• mvn -pl engine/schema checkstyle:check — passes
• mvn -pl engine/schema test — all tests pass

Code-level verification:

• The update(jobId, jobForUpdate) call generates a single UPDATE usage_job SET start_millis=?, end_millis=?, exec_time=?, start_date=?, end_date=?, success=? WHERE id=? — all columns written atomically in one statement
• TransactionLegacy.open(USAGE_DB) is kept since this method explicitly targets the usage database connection. The txn.start()/txn.commit()/txn.rollback() are removed since the single update() call is autocommit
• Confirmed lockRow no longer appears in the method
• The original code read job.getId() from the locked row to pass to update() — but job.getId() always equals jobId (the parameter), since lockRow(jobId) fetches the row by that same ID. The new code uses jobId directly, eliminating the unnecessary indirection

Behavioral equivalence:

• All six field values (startMillis, endMillis, execTime, startDate, endDate, success) are parameters passed by the caller — nothing is read from the locked row that feeds into the write
• The usage server model is single-owner: one server processes one job at a time. Concurrent updates to the same job row do not occur by design, so the lock was defense-in-depth rather than a correctness requirement

[vishesh92]

@blueorangutan package

[blueorangutan]

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 17.67%. Comparing base (a289bb0) to head (6c6aa4e).

Files with missing lines	Patch %	Lines
...main/java/com/cloud/usage/dao/UsageJobDaoImpl.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.22   #13160      +/-   ##
============================================
- Coverage     17.67%   17.67%   -0.01%     
+ Complexity    15792    15787       -5     
============================================
  Files          5922     5922              
  Lines        533123   533119       -4     
  Branches      65201    65201              
============================================
- Hits          94246    94239       -7     
+ Misses       428236   428234       -2     
- Partials      10641    10646       +5     

Flag	Coverage Δ
uitests	3.69% <ø> (ø)
unittests	18.75% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Removes the explicit row-level lock and surrounding transaction (start/commit/rollback) in UsageJobDaoImpl.updateJobSuccess, relying on a single autocommit update(jobId, jobForUpdate) against the usage DB. The TransactionLegacy.open(USAGE_DB) is preserved to bind the update to the usage database connection. Justified by the single-owner usage server model where concurrent updates to the same job row do not occur.

Changes:

• Drop lockRow(jobId, ...) and the explicit txn.start() / txn.commit() / txn.rollback() calls.
• Replace update(job.getId(), ...) with update(jobId, ...) since job.getId() was always equal to jobId.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.