[
https://issues.apache.org/jira/browse/HDDS-17271?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17271:
------------------------------
Affects Version/s: 2.2.1
2.0.0
2.1.2
2.1.0
2.1.1
2.2.0
> 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
> Affects Versions: 2.0.0, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1
> Reporter: Siyao Meng
> Priority: Minor
> 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]
