Siyao Meng created HDDS-17290:
---------------------------------
Summary: Close on an interrupted thread fails but OM still commits
the key, so a small encrypted file or an interrupted write becomes empty or
truncated
Key: HDDS-17290
URL: https://issues.apache.org/jira/browse/HDDS-17290
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: CR-10-do-not-commit-key-on-interrupted-close.patch,
TestBugCR10InterruptedCloseStillCommits.java
h3. Mechanism
{{KeyOutputStream.write}} rejects the call with {{InterruptedIOException}}
("Write aborted. Interrupted waiting for KeyOutputStream semaphore") when the
thread is interrupted, because {{KeyOutputStreamSemaphore.acquire}} fails.
Unlike the other write failures of {{KeyOutputStream}}, this one does not mark
the stream closed, and {{acquire}} sets the interrupt flag again. A later
{{close()}} therefore runs {{KeyOutputStream.closeInternal}} on the interrupted
thread. If the stream holds no data in an open block, nothing in the close has
to go to the datanodes, so nothing fails on the interrupt, and {{commitKey}}
sends the commit with the bytes in the blocks already filled and closed (none,
or only the full blocks). Hadoop IPC usually sends the request, then fails the
wait for the response with "Call interrupted". The caller gets that exception,
and OM applies the commit.
For a key in an encrypted bucket the client writes through
{{CryptoOutputStream}}, which keeps up to its buffer size (8 KiB by default)
and hands it to the {{KeyOutputStream}} only on flush or close. Hadoop's
{{CryptoOutputStream.close}} closes the inner stream even when its final flush
failed (since HADOOP-18621). So a writer whose {{write}} calls all returned
normally hits the same path as soon as its thread is interrupted before
{{close()}}.
h3. Trigger
Encrypted bucket (Java client, ofs/o3fs, or any other {{OzoneOutputStream}}
user):
# Write a key or file smaller than the crypto buffer (8 KiB), or one whose data
past its last full block is smaller than that, with no flush since the last
full block. All {{write}} calls return normally.
# The writer thread is interrupted before or during {{close()}}.
Any bucket with Ratis replication:
# Create a key, and the thread is interrupted before the first {{write}} has
passed any byte to the stream, or when the data written so far ends exactly at
a block boundary.
# {{write}} fails with "Write aborted", and the stream is then closed, for
example by try with resources.
Thread interrupts of writer threads are routine in frameworks (task
cancellation, {{Future.cancel(true)}}, {{ExecutorService.shutdownNow}}); the
reproduction sets the interrupt flag directly. If the stream holds data in an
open block, including data pushed there by a flush, the close fails writing it
and nothing is committed (run). Erasure coded keys are not affected:
{{ECKeyOutputStream.close}} fails with "Interrupted while adding stripe to
queue" before the commit (run, empty and 16 byte keys). Datastream keys are not
affected: a failed write there fails the stream (from the code). The S3 gateway
form of this is HDDS-17272.
h3. Impact
* {{close()}} throws {{InterruptedIOException}}, yet the key is committed with
size 0, or with only its full blocks, and replaces the previous content. A new
key or file appears empty or truncated. Run on a mini cluster: empty commits
for the Java client and ofs on an encrypted bucket and for the rejected write
case on a plain bucket; truncated commits for both buckets as below.
* The commit can keep only part of the data. Data in blocks the stream has
already filled and closed does not have to be written again at close, so a key
of one block plus 100 bytes on an encrypted bucket was committed with the block
only (run with a 1 MiB block size). With the default 256 MiB block size this
applies to any encrypted file whose data past the last full block is less than
8 KiB: a 256 MiB plus 100 byte key was committed with 268435456 bytes (run with
the default block, chunk and stream buffer sizes). The same happened on a plain
bucket when the write after a full block was rejected.
* The commit becomes visible shortly after {{close()}} has thrown, so a check
right after the failure can still see the old content.
* Affects 2.0.0 and later: before HDDS-11184 added the semaphore, an
interrupted write was accepted into the buffer and the close then failed on
datanode I/O (from the code, not run).
h3. Reproduction
PASS on unmodified source. [^TestBugCR10InterruptedCloseStillCommits.java]
({{hadoop-ozone/integration-test}}) starts one MiniOzoneCluster with three
datanodes and a {{JavaKeyStoreProvider}} as KMS, with a 1 MiB block size. Each
test writes V1, then writes new content to the same key with the writer thread
interrupted.
* {{encryptedKeyCommittedEmptyAfterFailedClose}} (Java client) and
{{encryptedOfsFileCommittedEmptyAfterFailedClose}} (ofs), 16 bytes: {{close()}}
fails with "Call interrupted", then the key has size 0.
* {{encryptedKeyTruncatedToFullBlockAfterFailedClose}}, one write of 1 MiB plus
100 bytes: {{close()}} fails with "Call interrupted", then the key holds the
first 1 MiB only.
* {{plainKeyCommittedEmptyAfterInterruptedWrite}} and
{{plainKeyTruncatedAfterInterruptedWrite}} (the rejected write follows a full 1
MiB block): the write fails with "Write aborted", the close with "Call
interrupted", then the key has size 0 or 1 MiB.
* {{plainKeyKeepsPreviousContentAfterFailedClose}} (control): the accepted
write leaves data in an open block, the close fails with "Failed to write chunk
... into block", and V1 stays.
A passing test means the defect is present. With the patch all tests except the
control fail because V1 stays, and the control passes.
h3. Patch
[^CR-10-do-not-commit-key-on-interrupted-close.patch], against
b10a7d427bdf83807a7e48a1be507d3b02d5a503. It also applies to master at
1ff29ee6b86, alone and together with the patches of HDDS-17271 and HDDS-17272.
{{KeyOutputStream.closeInternal}} throws {{InterruptedIOException}} instead of
committing when the closing thread is interrupted. The check runs after the
pre-commits and before {{commitKey}}. A close on a thread that is already
interrupted no longer sends the commit, and the open key is left to the open
key cleanup. The caller already got an exception in this case. Uninterrupted
closes are unchanged. Failing the stream on a rejected write would also cover a
caller that clears the interrupt flag or closes the stream from another thread,
which still commits the bytes written before the rejected write, but it changes
{{write}} for callers that keep using the stream and touches the lines changed
by HDDS-17271.
Covered by {{testInterruptedCloseOfEncryptedKeyDoesNotCommit}} (key stream
wrapped in a real {{CryptoOutputStream}}) and
{{testCloseAfterInterruptedWriteDoesNotCommit}} in the existing
{{TestOzoneClient}}, which check that the key keeps its previous content.
Without the change they fail with {{expected: 14L but was: 0L}} and "Expecting
code to raise a throwable". With it the {{hadoop-ozone/client}} unit tests
(340), {{TestOzoneAtRestEncryption}} (36) and {{TestHSync}} (43) on a mini
cluster pass, and checkstyle is clean. With this patch alone, the reproduction
of HDDS-17272 also no longer commits.
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]
