yandrey321 opened a new pull request, #11937:
URL: https://github.com/apache/ozone/pull/11937
## What changes were proposed in this pull request?
**Problem.** `ContainerReportQueue` is the work queue of SCM's FCR and ICR
executors
(`FixedThreadPoolWithAffinityExecutor`).
- **Producers and consumer:** heartbeat handler threads add reports, and one
worker thread per queue polls with a
1 ms timeout.
- **Locking:** every operation holds the queue's `synchronized` monitor,
which guards `dataMap` and the capacity count.
The order of datanodes is kept in an inner `LinkedBlockingQueue`, which
has two more locks of its own.
- **Effect:** when several handlers add reports at once, they all contend on
the monitor together with the worker.
Throughput then falls steeply as producers are added.
**Change.**
- **One lock:** the monitor and the inner `LinkedBlockingQueue` are replaced
by one `ReentrantLock`, which guards
everything. The ordering queue becomes a plain `ArrayDeque` under that
lock.
- **Waiting:** `take()` and `poll(timeout)` wait on a `Condition` of the
lock, and each new entry in the ordering queue
signals it.
- **`isEmpty()`** now also takes the lock.
**Key decisions.**
- **A `ReentrantLock`, not just dropping the inner queue.**
- The Jira proposed replacing `LinkedBlockingQueue` with
`ConcurrentLinkedQueue`. But that has no blocking
`poll(timeout)`, so the worker would need a sleep loop.
- Measured first, I dropped the inner queue and kept the monitor, waiting
with `wait`/`notify`. That was slower than
master, 0.4x at 16 producers. The inner queue had kept the waiting
worker off the monitor.
- So what contends is the `synchronized` monitor itself. Only replacing it
fixes that.
- **The queue stays bounded as before.** The bound is still `maxCapacity`,
from
`ozone.scm.event.<ContainerReport_OR_IncrementalContainerReport>.queue.size`,
default 100,000.
- The removed `LinkedBlockingQueue` was created without a capacity, so it
never bounded anything.
- `add()` still throws, and `offer()` still returns false, when the queue
is full.
- A newer FCR from a datanode still replaces its queued FCR (HDDS-7244).
- **A non-fair lock**, the `ReentrantLock` default, which favours
throughput. Its cost shows only with a single
producer at rates far above what SCM sees; see the benchmark.
**Side effects.**
- **Behaviour is unchanged:** the same order, dedup, capacity and exceptions.
- **`ReconContainerReportQueue`** overrides only `mergeIcr`, which still
runs under the lock, so it needs no change.
- **External synchronization:** no code synchronizes on a
`ContainerReportQueue` instance from outside.
Co-authored-by: Claude Opus 5.5 [email protected]
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13020
## How was this patch tested?
- **Benchmark:** a JUnit micro-benchmark, not part of this PR.
- Setup: producer threads add ICRs from 100 distinct datanodes each. One
consumer calls `poll(1, MILLISECONDS)` in a
loop, as the executor's worker does, and processing does nothing.
- Runs: master and this PR, alternating, 2 rounds. JDK 21, macOS laptop,
JaCoCo off.
- **Queue full** (producers add as fast as they can, retrying when full).
Median reports/ms, n = 10:
| producers | master | this PR |
|---|---|---|
| 1 | 3,888 (2,414–7,690) | 3,436 (2,832–4,733) |
| 4 | 861 (507–2,056) | 2,062 (1,723–2,454) (2.4x) |
| 16 | 253 (105–401) | 599 (486–767) (2.4x) |
- **Paced** (a fixed total rate, a burst every millisecond, so the queue
never fills). CPU per report is thread CPU
time divided by reports. Latency is from `offer` to `poll`. Medians, n =
6:
| producers | rate/s | CPU ns/report, master → this PR | p50 µs | p99 µs
|
|---|---|---|---|---|
| 1 | 20,000 | 2,044 → 1,909 | 10 → 12 | 45 → 41 |
| 1 | 200,000 | 1,490 → 900 | 39 → 43 | 124 → 181 |
| 1 | 1,000,000 | 976 → 461 | 54 → 1,768 | 288 → 9,468 |
| 4 | 20,000 | 5,952 → 4,994 | 11 → 12 | 70 → 64 |
| 4 | 200,000 | 1,732 → 1,076 | 40 → 29 | 193 → 147 |
| 4 | 1,000,000 | 1,046 → 651 | 186 → 126 | 931 → 973 |
- **What the numbers show:**
- **Several producers:** this PR has 2.4x the throughput, uses 16–38%
less CPU per report, and latency is the
same or better.
- **One producer at 20,000/s:** the same. That is already a large report
storm for SCM.
- **One producer at 200,000/s and above:** less CPU but higher latency.
With a non-fair lock, the producer
re-takes the lock for each report of a 1,000-report burst before the
woken worker gets it. So the worker drains
mostly between bursts. On master the worker waits on the inner queue's
own lock, which the producer touches
only when the queue goes from empty to non-empty.
- **In SCM:** processing a report takes far longer than either version
of the queue operations. The difference
matters when many heartbeat handlers add reports at the same time,
such as after an SCM restart.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
