[ 
https://issues.apache.org/jira/browse/HDDS-16975?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-16975:
------------------------------
    Attachment: TestBugCR4RatisStreamCreationFailure.java

> Ratis key write fails with NullPointerException and hides the real error when 
> a block output stream cannot be created
> ---------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16975
>                 URL: https://issues.apache.org/jira/browse/HDDS-16975
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: 
> CR-4-fix-npe-when-ratis-block-stream-creation-fails.patch, 
> TestBugCR4RatisStreamCreationFailure.java
>
>
> h3. Mechanism
> {{BlockOutputStreamEntry.checkStream}} creates the {{RatisBlockOutputStream}} 
> of a block on the first write to it, and the constructor calls 
> {{XceiverClientFactory.acquireClient}}. When that throws an {{IOException}}, 
> {{KeyOutputStream.writeToOutputStream}} passes it to 
> {{handleExceptionInternal}}. Since HDDS-11239 (2.0.0) the first statement 
> there is {{streamEntry.waitForAllPendingFlushes()}}, which calls 
> {{outputStream.waitForAllPendingFlushes()}} on the stream that was never 
> created. The {{NullPointerException}} is not caught by the surrounding 
> {{catch (IOException ignored)}}, leaves the handler before the original 
> exception is logged, and is wrapped by {{handleWrite}}. The caller gets 
> {{IOException: java.lang.NullPointerException}}, and the reason why the 
> stream could not be created is neither in the exception nor in the client log.
> Before HDDS-11239 the handler went on: it logged the exception, excluded the 
> pipeline and called {{BlockOutputStreamEntry.cleanup}}, which calls 
> {{checkStream}} once more. If that second attempt failed as well, the write 
> failed with the creation failure as the cause. If it succeeded, the stream 
> was released again and the write continued on the next block. Both outcomes 
> are now replaced by the {{NullPointerException}}.
> The same happens for the block that takes over the buffered data of a failed 
> block ({{writeOnRetry}}). {{flush}}, {{hsync}} and {{close}} check 
> {{isInitialized}} first and are not affected. The streaming write 
> ({{KeyDataStreamOutput}}) has no such call and still reports the cause. EC 
> keys take a different path.
> h3. Trigger
> # A Ratis key write (factor ONE or THREE) makes the first write to a block.
> # Creating the block output stream throws an {{IOException}}, in practice 
> from {{acquireClient}}.
> Creating a Ratis client opens no connection, so a datanode that is down or a 
> hostname that does not resolve does not trigger it. What does, run against 
> the real {{XceiverClientManager}}:
> * An unsupported value of {{hdds.ratis.client.request.watch.type}} or 
> {{hdds.container.ratis.rpc.type}} in the client configuration. This fails for 
> every pipeline.
> * Datanode details without a RATIS port, with a port out of range or without 
> an IP address. This fails for one pipeline, but I do not see how a registered 
> datanode would have such details (code reading, not run against SCM).
> I found no trigger that is specific to one pipeline of a healthy cluster. 
> Checked and not a trigger: hostname mode 
> ({{hdds.datanode.use.datanode.hostname=true}}) with a hostname that contains 
> an underscore, is empty or does not resolve, a closed manager, a client 
> evicted from the cache or invalidated on release.
> h3. Impact
> * The write fails with a {{NullPointerException}} that says nothing about the 
> cause, and the key is not committed. Committed data is not affected and only 
> the writing client is involved.
> * Run on a mini cluster with 3 datanodes: a client configured with 
> {{hdds.ratis.client.request.watch.type=MAJORITY}} fails every RATIS/ONE and 
> RATIS/THREE key write with the exception below, and nothing in its log names 
> the setting. With the patch it gets "MAJORITY is not supported. Currently 
> only ALL_COMMITTED or MAJORITY_COMMITTED are supported".
> * A single failed attempt to create the stream now fails the write, where 
> before HDDS-11239 it continued on the next block.
> {noformat}
> java.io.IOException: java.lang.NullPointerException: Cannot invoke 
> "org.apache.hadoop.hdds.scm.storage.BlockOutputStream.waitForAllPendingFlushes()"
>  because "this.outputStream" is null
> {noformat}
> h3. Reproduction
> PASS, deterministic, unmodified source. 
> [^TestBugCR4RatisStreamCreationFailure.java] ({{hadoop-ozone/client}}, about 
> one second) drives the unmodified write path through 
> {{OzoneBucket.createKey}} and {{OzoneOutputStream}} with the test doubles of 
> {{TestOzoneClient}}. A subclass of {{MockXceiverClientFactory}} makes 
> {{acquireClient}} throw and counts acquired and released clients. Cases: the 
> client of the first block can never be acquired (factor ONE and THREE), only 
> the first call fails, and the client of a retry block cannot be acquired. In 
> each case the write fails with the exception above, the acquire failure 
> appears nowhere in the stack trace, and the key is not committed. A control 
> in which the datanode fails the write instead commits the key in a second 
> block. No client stays acquired in any case. A passing test means the defect 
> is present.
> The mini cluster run mentioned under Impact used the real 
> {{XceiverClientManager}} and real datanodes. It is not attached.
> h3. Patch
> [^CR-4-fix-npe-when-ratis-block-stream-creation-fails.patch], against 
> 031e76aa2c9a60b67185390b5b33499cbbb82e43. It also applies to master at 
> c5d4f28266d.
> {{BlockOutputStreamEntry.waitForAllPendingFlushes}} returns when there is no 
> stream. That restores what the handler did before HDDS-11239 for a block 
> without a stream and changes nothing for a stream that exists, so the wait 
> for pending flushes that HDDS-11239 added is kept. A lasting failure fails 
> the write with the creation failure as the root cause, after one WARN from 
> {{handleExceptionInternal}} that names the pipeline. A single failed attempt 
> continues on the next block.
> Not changed: a lasting failure to create the stream for one pipeline still 
> fails the write instead of moving to another pipeline, because {{cleanup}} 
> tries to create the stream of the same block again. That has been so since 
> HDDS-959. Handling it like a failed write would need the block entry to be 
> skipped and a limit on such attempts, and I found no pipeline specific 
> trigger that would benefit, so it is left for a follow up if wanted.
> Covered by {{testPutKeyWhenClientAcquireFailsOnce}} and 
> {{testPutKeyFailsWithCauseWhenClientCannotBeAcquired}} in the existing 
> {{TestOzoneClient}}. Without the change both fail with the 
> {{NullPointerException}}. With it the 18 test classes of 
> {{hadoop-ozone/client}} (161 tests) and {{TestHSync}} on a mini cluster (42 
> tests) pass and checkstyle is clean. With the patch the failure cases of the 
> reproduction fail, because the cause is reported or the write succeeds; the 
> control still passes.
> Found by code review of the client EC key write and file checksum paths, as 
> part of the TLA+ verification effort under HDDS-15926, on commit 
> 031e76aa2c9a60b67185390b5b33499cbbb82e43. 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]

Reply via email to