Siyao Meng created HDDS-17271:
---------------------------------
Summary: KeyOutputStream releases a request permit it never
acquired when write, flush or hsync is interrupted
Key: HDDS-17271
URL: https://issues.apache.org/jira/browse/HDDS-17271
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: CR-4-acquire-key-output-stream-permit-before-try.patch,
TestBugCR4InterruptedCallReleasesPermit.java
h3. Mechanism
{{KeyOutputStream.write}}, {{flush}} and {{hsync}} call
{{KeyOutputStreamSemaphore.acquire()}} as the first statement inside {{try}}
and {{release()}} in {{finally}}. When the thread is interrupted, either
already interrupted on entry or while it waits for a permit, {{acquire()}}
throws {{InterruptedIOException}} without taking a permit, and the {{finally}}
block still releases one. Every such call adds one permit to the stream's
semaphore for the rest of its life. The pattern came in with HDDS-11184.
{{ozone.client.key.write.concurrency}} defaults to 1 and is forced to 1 unless
{{ozone.client.hbase.enhancements.allowed=true}}. HDDS-11184 added it as a
safety fallback to the behaviour before HDDS-9844, where writes and hsyncs on
one key did not overlap. After one interrupted call that fallback no longer
holds.
h3. Trigger
# Two or more threads use the same key output stream (for example a writer
thread and a thread calling {{hflush}} or {{hsync}}, as HBase does).
# One of them is interrupted while it waits for the permit, or calls {{write}},
{{flush}} or {{hsync}} with its interrupt flag already set
({{Future.cancel(true)}}, {{ExecutorService.shutdownNow}}).
No configuration change and no fault are needed.
h3. Impact
* After the interrupted call, the stream admits two calls at once at the
default concurrency of 1, and more after further interrupts (run, see
Reproduction).
* {{write}} (around {{handleWrite}}) and {{close}} still take the stream's
write lock, so two writes, or a write and close, still do not overlap. The
calls that do overlap are {{flush}} and {{hsync}} with each other and with a
{{write}}, which today only happens with the HBase enhancements enabled and the
limit raised. {{hflush}} and {{hsync}} with {{ozone.fs.hsync.enabled=false}}
reach {{KeyOutputStream.flush}} without the monitor that
{{OzoneOutputStream.flush}} holds, so two {{hflush}} callers on one ofs/o3fs
stream can overlap too.
* HDDS-16979 (open) reports an hsync or flush that never returns when two such
calls overlap and two blocks fail in a row, and notes that with the default
concurrency of 1 the semaphore keeps the second call out. This defect removes
that protection on a stream that saw one interrupted call. Argued from the
code, not run.
* No data loss or wrong data was observed. Erasure coded keys
({{ECKeyOutputStream}} does not use the semaphore) and the S3 Gateway (one
request thread per stream, no flush or hsync) are not affected.
h3. Reproduction
PASS on unmodified source, deterministic, no cluster.
[^TestBugCR4InterruptedCallReleasesPermit.java] ({{hadoop-ozone/client}}) uses
the setup of {{TestKeyOutputStream.testConcurrentWriteLimitOne}}: a spied
{{KeyOutputStream}} with a real {{KeyOutputStreamSemaphore(1)}} and a
{{handleWrite}} that blocks until released.
* {{interruptedWaiterAdmitsSecondWrite}}: a first write holds the only permit,
a second write is interrupted while it waits, and a third write then gets past
the semaphore (0 calls queued on it instead of 1) and waits on the write lock.
* {{interruptedCallsAddPermitsForGood}}: one interrupted {{write}}, {{flush}}
and {{hsync}} each on an idle stream, then four writes all get past the
semaphore (0 queued instead of 3).
A passing test means the defect is present. With the patch both tests fail on
the queue length assertion (1 and 3 queued).
h3. Patch
[^CR-4-acquire-key-output-stream-permit-before-try.patch], against
b10a7d427bdf83807a7e48a1be507d3b02d5a503. It also applies to master at
e0de543cbe9.
{{write}}, {{flush}} and {{hsync}} call {{acquire()}} before the {{try}} block,
so {{release()}} only runs after a permit was taken. Nothing else changes;
uninterrupted calls behave as before.
Covered by {{testInterruptedCallDoesNotReleasePermit}} in the existing
{{TestKeyOutputStream}}: a write waiting for the permit is interrupted,
{{flush}} and {{hsync}} are called on an interrupted thread, and a further
write must still queue on the semaphore while the first write holds the permit,
then complete once it is released. Without the change it fails with {{expected:
1 but was: 0}}. With it the {{hadoop-ozone/client}} unit tests (339) and
{{TestHSync}} on a mini cluster (43) pass, and checkstyle is clean.
Found by code review of the S3 gateway object put and key commit paths, as part
of the TLA+ verification effort under HDDS-15926, on commit
b10a7d427bdf83807a7e48a1be507d3b02d5a503. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. The attached patch is
a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
