mazhengxuan created HBASE-30317:
-----------------------------------
Summary: TestExecutorService.testExecutorService has a
synchronization race when checking the executor status dump
Key: HBASE-30317
URL: https://issues.apache.org/jira/browse/HBASE-30317
Project: HBase
Issue Type: Bug
Components: test
Affects Versions: 2.6.5, 2.5.10, 3.0.0-alpha-1, 4.0.0-alpha-1
Reporter: mazhengxuan
Assignee: mazhengxuan
Fix For: 4.0.0-alpha-1
h3. Problem
TestExecutorService.testExecutorService has a synchronization race when it
checks the executor status dump.
The test submits five TestEventHandler instances and waits only until counter
reaches five. Each handler increments the counter immediately after entering
process():
{code:java}
int num = counter.incrementAndGet();
LOG.info("Running process #" + num + ", threadName=" +
Thread.currentThread().getName());
synchronized (lock) {
while (lock.get()) {
lock.wait();
}
}
{code}
The main test thread treats counter == 5 as proof that all handlers are waiting
on lock, captures ExecutorStatus, and expects the dump to contain:
{code}
Waiting on java.util.concurrent.atomic.AtomicBoolean
{code}
However, counter == 5 proves only that all handlers have started. It does not
prove that they have completed the logging expression and entered lock.wait().
Under resource pressure, the main test thread can capture the status while all
five handlers are still RUNNABLE. In the reproduced failure, all handler stacks
were executing:
{code}
StringConcatFactory.makeConcatWithConstants(...)
{code}
from the first LOG.info expression. None of them had reached lock.wait(), so
checkStatusDump failed to find the expected text.
h3. Reproduction
The failure was reproduced independently with:
{code:bash}
mvn \
-pl hbase-server -am \
-Dtest=org.apache.hadoop.hbase.executor.TestExecutorService \
-Dsurefire.failIfNoSpecifiedTests=false \
test
{code}
Result:
{code}
Tests run: 3, Failures: 1, Errors: 0
{code}
Observed system load:
{code}
SystemLoadAverage=471
AvailableMemoryMB=21
ProcessCount=574
{code}
The high load makes the race much easier to reproduce, but the test itself
contains the synchronization defect.
h3. Expected behavior
The status dump should be checked only after the handlers have actually entered
the WAITING state on the AtomicBoolean monitor.
h3. Proposed fix
Before the final assertion, poll ExecutorStatus until its dump contains the
expected waiting state, subject to a bounded timeout. Then run the existing
final assertions.
A fixed Thread.sleep should not be added because it would only reduce the
probability of the race and would remain unreliable on slow or overloaded
machines.
h3. Related issues
* HBASE-30303: this race was discovered while validating its changes.
* HBASE-23241: fixed a different synchronization race in
TestExecutorService.testSnapshotHandlers; this issue is not a duplicate.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
