sb-abhish3k opened a new pull request, #14016:
URL: https://github.com/apache/cloudstack/pull/14016
### Description
The `op_host_capacity` table experiences severe lock contention during
concurrent VM lifecycle operations. Slow query analysis shows:
- **25s average execution time** (99% lock wait, not query execution)
- **5,724 executions in 20 hours** across 3 management server nodes
- **3% failure rate** from `errno 1205` (InnoDB lock wait timeout exceeded)
- Query: `SELECT ... FROM op_host_capacity WHERE id = ? FOR UPDATE`
**Root cause:** `CapacityManagerImpl.releaseVmCapacity()` and
`allocateVmCapacity()` use a lock-read-compute-write pattern via
`GenericDaoBase.lockRow()`. Each VM operation acquires exclusive row locks on 3
capacity rows (CPU, Memory, CPU Core), then holds those locks while performing
Java computation, querying `cluster_details` for overcommit ratios, logging,
and validating — before finally committing. Any concurrent VM operation
targeting the same host queues behind this entire sequence.
**Fix:** Replace the `lockRow()` + `Transaction.execute()` pattern with
single-statement atomic SQL UPDATEs that push arithmetic to the database:
```sql
-- Example: decrement used capacity (was lockRow + Java subtract + update)
UPDATE op_host_capacity SET
used_capacity = CASE WHEN used_capacity >= ? THEN used_capacity - ? ELSE
used_capacity END,
update_time = NOW()
WHERE id = ?
```
Six new atomic DAO methods cover all capacity mutation patterns:
- `decrementUsedCapacity` — VM stop/migrate away
- `decrementReservedCapacity` — release reserved (destroy/expunge)
- `incrementUsedCapacity` — VM start/migrate to
- `decrementUsedIncrementReservedCapacity(id, used, reserved,
overcommitRatio)` — VM stop with reservation (capped at overcommitted total)
- `decrementUsedIncrementReservedCapacity(id, used, reserved)` — same,
uncapped (for CPU core which has no overcommit)
- `incrementUsedDecrementReservedCapacity` — allocate from last host
**Lock duration reduction:** From seconds (full transaction span including
cluster_details reads, Java math, logging) to microseconds (single UPDATE
statement). InnoDB still acquires an implicit row lock for each UPDATE, but
releases it immediately on statement completion.
**Behavioral changes from original code:**
| Aspect | Before | After |
|---|---|---|
| Lock scope | 3 rows locked simultaneously in one transaction | 1 row at a
time, independent autocommit |
| Negative capacity guard | Java `if (used >= amount)` — leaves value
unchanged | SQL `CASE WHEN used >= ? THEN used - ? ELSE used END` — same
semantics |
| `fromLastHost` reserved decrement | Cross-row check: only decrements all 3
if CPU AND Memory both have enough reserved | Per-row: each independently
decrements with `GREATEST(reserved - ?, 0)` floor. Fixes a reserved capacity
leak in the original where one insufficient resource blocked all three from
being freed |
| Capacity validation in `allocateVmCapacity` | Validated inside lock after
incrementing (rollback on failure) | Validated before atomic increment (same
pre-update DB state check). Tiny race window — acceptable since capacity
accounting is approximate and `updateCapacityForHost()` periodically
recalibrates |
| Overcommit ratio reads | Inside lock (adds lock hold time) | Before update
(no lock contention contribution) |
**Atomicity guarantees:**
The old code relied on explicit `SELECT ... FOR UPDATE` row locks held
across a multi-statement transaction to ensure correctness. The new code relies
on InnoDB's implicit row-level locking within single UPDATE statements. Both
are correct, but the lock hold time differs by orders of magnitude.
**Before — explicit transaction locking:**
```
BEGIN
SELECT ... FOR UPDATE ← X lock acquired on row (blocks here if contended)
-- lock held --
Java: read used/reserved/total from locked row
Java: query cluster_details table for overcommit ratios (extra DB
round-trip while holding lock)
Java: compute new capacity values
Java: log debug statements
Java: validate capacity
UPDATE row 1 (CPU)
UPDATE row 2 (Memory)
UPDATE row 3 (CPU Core)
COMMIT ← X locks on all 3 rows released
```
Lock hold time: **seconds** (measured avg 25s in production). Three rows
locked simultaneously for the entire transaction span. Concurrent VM ops on the
same host queue behind this.
**After — single-statement atomic updates:**
```
-- All reads and computation done BEFORE any locks --
Java: query capacity rows (regular SELECT, no lock)
Java: query cluster_details for overcommit ratios
Java: validate capacity
UPDATE row 1 (CPU) ← X lock acquired, UPDATE executes, autocommit
releases lock
UPDATE row 2 (Memory) ← X lock acquired, UPDATE executes, autocommit
releases lock
UPDATE row 3 (CPU Core) ← X lock acquired, UPDATE executes, autocommit
releases lock
```
Lock hold time per row: **microseconds** (single UPDATE statement). Each row
locked independently for the minimum possible duration.
**How InnoDB guarantees correctness for concurrent updates:**
When two sessions concurrently issue `UPDATE op_host_capacity SET
used_capacity = used_capacity + ? WHERE id = ?` on the same row:
1. Session A reaches the row first, acquires an exclusive (X) lock on the
clustered index entry
2. Session B attempts to acquire the same X lock, enters InnoDB's lock wait
queue
3. Session A's UPDATE completes and autocommits — X lock is released
4. Session B is granted the lock and reads the **latest committed value** of
`used_capacity` (post-Session-A), then applies its own increment
5. Session B's UPDATE completes and autocommits
This is guaranteed by InnoDB's locking protocol: an UPDATE always reads the
latest committed version of the row, not an MVCC snapshot. Both increments are
correctly applied with no lost updates. This behavior is identical in MySQL
(5.6+) and MariaDB (10.x+), which both use InnoDB as the default storage
engine. All SQL constructs used (`GREATEST`, `CASE WHEN`, `CAST ... AS SIGNED`,
`NOW()`) are supported since MySQL 4.0+.
**What changes and what is acceptable:**
1. **Single-row correctness (no change):** Each UPDATE is atomic and
serialized by InnoDB's row lock. `used_capacity = used_capacity + ?` cannot
lose updates, and `CASE WHEN used_capacity >= ? THEN used_capacity - ? ELSE
used_capacity END` prevents negative values. Equivalent to the old Java guards
(`if (usedCpu >= vmCPU)`).
2. **Cross-row atomicity (relaxed, acceptable):** The old code updated CPU,
Memory, and CPU Core in a single transaction — all-or-nothing. The new code
uses three independent autocommit statements. If a DB connection dies between
UPDATE 1 and UPDATE 2, capacity state is temporarily inconsistent for that
host. This is an extremely unlikely failure mode (requires connection loss
between two statements milliseconds apart), and `updateCapacityForHost()`
periodic recalibration self-heals any inconsistency by recomputing capacity
from actual VM state.
3. **Capacity validation window (slightly wider, acceptable):** In
`allocateVmCapacity`, the capacity check (`checkIfHostHasCapacity`) now runs
before the atomic UPDATE rather than inside the locked transaction. Two VMs
could both pass validation before either writes. However, this is the same
semantic as the original code — the original also validated against pre-update
DB state (the `checkIfHostHasCapacity` call performs its own `findByHostIdType`
SELECT, which reads uncommitted-to-disk values since the
`_capacityDao.update()` hasn't been called yet within the transaction). The
validation window is slightly wider without locks, but capacity accounting is
inherently approximate and `updateCapacityForHost()` recalibration is the
safety net.
4. **`fromLastHost` reserved decrement (improved):** The old code had a
cross-row invariant (`reservedCpu >= cpu && reservedMem >= ram`) gating all
three decrements — if memory didn't have enough reserved, CPU reserved wasn't
freed either, leaking reserved capacity until the next recalibration. The new
per-row `GREATEST(reserved - ?, 0)` frees each resource independently. This is
strictly more correct.
### 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)
- [ ] Build/CI
- [ ] Test (unit or integration test code)
### 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?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to
-->
#### How did you try to break this feature and the system with this change?
9 new unit tests added to `CapacityManagerImplTest` (15 total, all pass):
- **`testReleaseVmCapacityNullHostReturnsTrue`** — null host guard clause
- **`testReleaseVmCapacityNullCapacityReturnsFalse`** — null capacity entry
guard clause
- **`testReleaseVmCapacityDecrementUsed`** — verifies
`decrementUsedCapacity` called for CPU/Memory/CPU Core with correct amounts
when `moveFromReserved=false, moveToReserved=false`
- **`testReleaseVmCapacityDecrementUsedIncrementReserved`** — verifies
capped variant called with overcommit ratio for CPU/Memory, uncapped variant
for CPU Core when `moveToReserved=true`
- **`testReleaseVmCapacityDecrementReserved`** — verifies
`decrementReservedCapacity` called when `moveFromReserved=true`
- **`testAllocateVmCapacityNewHost`** — verifies `incrementUsedCapacity`
called for all three capacity types
- **`testAllocateVmCapacityFromLastHost`** — verifies
`incrementUsedDecrementReservedCapacity` called for all three capacity types
- **`testAllocateVmCapacityInsufficientThrows`** — verifies
`CloudRuntimeException` thrown when host lacks capacity
- **`testAllocateVmCapacityNullCapacityReturnsEarly`** — verifies no DAO
mutation when capacity entries are null
All tests verify `lockRow` is **never** called, confirming the lock-free
atomic path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
