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

Siyao Meng updated HDDS-16972:
------------------------------
    Attachment: TestBugCR2EcStreamSetupFailure.java

> EC key write fails with NullPointerException instead of retrying the stripe 
> when a datanode client cannot be acquired for a new block group
> -------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16972
>                 URL: https://issues.apache.org/jira/browse/HDDS-16972
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: 
> CR-2-retry-ec-stripe-when-block-stream-creation-fails.patch, 
> TestBugCR2EcStreamSetupFailure.java
>
>
> h3. Mechanism
> {{ECBlockOutputStreamEntry.checkStream}} creates one {{ECBlockOutputStream}} 
> per node when the first cell is written to a block group, and each 
> constructor calls {{XceiverClientFactory.acquireClient}}. If that throws for 
> one node, the failure is recorded nowhere. The half built array is dropped 
> without releasing the clients already acquired, and 
> {{ECKeyOutputStream.handleOutputStreamWrite}} calls {{markFailed}}, which 
> does nothing while the entry is uninitialized. The next cell calls 
> {{checkStream}} again and the loop starts at {{currentStreamIdx}}, so the 
> entry is initialized with the streams of the later nodes only and the earlier 
> slots stay null. {{streamsWithWriteFailure}} skips null slots and reports no 
> failure. {{commitStripeWrite}} then calls {{calculateChecksum}}, which 
> dereferences slot 0 and throws a {{NullPointerException}} before the stripe 
> can be rolled back and retried. If the failing node is the last one of the 
> block group, no array is ever assigned and the same call throws 
> {{IOException: Block Output Stream is null}}.
> The flush thread ends with that exception, {{close}} (or the next {{write}}) 
> rethrows it and the key is not committed, although the other nodes are 
> healthy and no stripe retry was used. When the same node fails one step 
> later, at WriteChunk, the stripe is rolled back, the pipeline and the node 
> are excluded and the stripe is written to a new block group.
> h3. Trigger
> # An EC write starts a new block group (the first stripe of a key or of a 
> later block group).
> # {{acquireClient}} throws for one node of the group.
> With {{XceiverClientManager}} step 2 means that creating the gRPC channel 
> failed. A datanode that is down or whose name does not resolve does not 
> trigger it: the channel connects lazily and the error shows at WriteChunk, 
> which is handled.
> The trigger I ran with the real client needs 
> {{hdds.datanode.use.datanode.hostname=true}} (default false) and a datanode 
> whose hostname is not a valid URI host, for example one containing an 
> underscore. {{NettyChannelBuilder.forAddress}} rejects it 
> ({{IllegalArgumentException: Invalid host or port}}), while SCM registers the 
> datanode as healthy and places it in EC pipelines.
> In the default configuration the client connects to the IP address SCM 
> recorded for the datanode, and I found no node specific trigger there. Run 
> against the real {{XceiverClientManager}}: a closed manager, a client evicted 
> from the cache while referenced, an unresolvable host and an out of range 
> port do not make {{acquireClient}} throw. A local failure while the channel 
> is built (for example file descriptor exhaustion) would be a trigger in the 
> default configuration; this is argued from the code and not run.
> h3. Impact
> * The write fails with a {{NullPointerException}} or {{Block Output Stream is 
> null}} and the key is not committed. Committed data is not affected. On a 
> mini cluster with 7 datanodes, one of them with the hostname {{dn_1}}, 5 of 
> 12 writes of a one stripe RS-3-2 key failed this way (the number varies with 
> pipeline placement).
> * Datanode clients stay acquired. Every cell of the stripe repeats 
> {{checkStream}} and leaves the clients it acquired before the failing node 
> unreleased. Counted with a client factory that records acquire and release 
> calls, after {{close}}: 3 clients for a failing third node and 10 for a 
> failing fifth node, per stripe. {{XceiverClientManager}} closes a client only 
> when its reference count is zero, so with the real manager those channels 
> would stay open (argued from {{XceiverClientSpi.cleanup}}, not run).
> h3. Reproduction
> PASS, deterministic, unmodified source. 
> [^TestBugCR2EcStreamSetupFailure.java] ({{hadoop-ozone/client}}, about one 
> second) drives the unmodified write path through {{OzoneBucket.createKey}} 
> and {{OzoneOutputStream}} with the test doubles of {{TestOzoneECClient}}. A 
> subclass of {{MockXceiverClientFactory}} makes {{acquireClient}} throw for 
> chosen calls and counts acquired and released clients. It writes one RS-3-2 
> stripe with the client of node 0, 2 or 4 of the block group failing, and with 
> a single failed call. In each case the write fails as described, only the 
> block allocation of {{createKey}} is made, the key is not committed, and the 
> counted clients remain. A control with node 0 failing at WriteChunk commits 
> the key in a second block group. A passing test means the defect is present.
> Output of the three cases with one failing node (node 0, 2 and 4, in this 
> order):
> {noformat}
> caller saw: java.lang.NullPointerException: Cannot invoke 
> "org.apache.hadoop.hdds.scm.storage.ECBlockOutputStream.getContainerBlockData()"
>  because "this.blockOutputStreams[0]" is null, acquireClient calls: 5, 
> clients never released: 0, block allocations: 1, key commits: 0
> caller saw: java.lang.NullPointerException: Cannot invoke 
> "org.apache.hadoop.hdds.scm.storage.ECBlockOutputStream.getContainerBlockData()"
>  because "this.blockOutputStreams[0]" is null, acquireClient calls: 8, 
> clients never released: 3, block allocations: 1, key commits: 0
> caller saw: java.io.IOException: Block Output Stream is null, acquireClient 
> calls: 15, clients never released: 10, block allocations: 1, key commits: 0
> {noformat}
> The mini cluster run mentioned under Impact used the real 
> {{XceiverClientManager}} and real datanodes. It is not attached.
> h3. Patch
> [^CR-2-retry-ec-stripe-when-block-stream-creation-fails.patch], against 
> 031e76aa2c9a60b67185390b5b33499cbbb82e43. It also applies to master at 
> c5d4f28266d.
> {{checkStream}} builds the streams from index 0 and, when one cannot be 
> created, records the node, logs the cause once, closes the streams built so 
> far so that their clients are released, and rethrows. The entry then counts 
> as closed and is not initialized later. {{commitStripeWrite}} checks for the 
> recorded node first, excludes the pipeline and the node and returns 
> {{FAILED}}, so the stripe takes the existing rollback and retry path, as for 
> a failed WriteChunk. The nearest earlier change is HDDS-9659 (fixed in 
> 1.4.0): a comment there shows the same {{NullPointerException}} in 
> {{calculateChecksum}} and notes that it may also happen if the stream with 
> index 0 is not created during a retry. Its fix, assigning the array only when 
> complete, is kept. A failure caused by the stream closing (HDDS-7931) does 
> not start a retry.
> One visible change: if no datanode client can be created at all, the write 
> now fails after {{ozone.client.max.ec.stripe.write.retries}} further block 
> allocations with "Completed max allowed retries N on stripe failures." 
> instead of at once with "Block Output Stream is null", the same as when every 
> WriteChunk fails. Each attempt excludes one more datanode, so on a small 
> cluster the write fails earlier, with the block allocation error, as soon as 
> fewer than d+p datanodes are left outside the exclude list (with the mock 
> block allocator, RS-3-2 and the default 10 retries need 15 datanodes to reach 
> the retry limit; not run against SCM).
> Covered by {{testStripeWriteRetriesOnClientAcquireFailure}} (failing node 0, 
> 2 and 4) and {{testStripeWriteRetriesAreLimitedOnClientAcquireFailure}} in 
> the existing {{TestOzoneECClient}}. Without the change they fail with the two 
> exceptions above. With it the 18 test classes of {{hadoop-ozone/client}} (163 
> tests) and {{TestECKeyOutputStream}} on a mini cluster (14 tests) pass and 
> checkstyle is clean. On the mini cluster with the {{dn_1}} datanode all 12 
> writes commit and read back. With the patch the failure cases of the 
> reproduction fail, because 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