[jira] [Updated] (HDDS-16999) EC reconstruction read returns data of the wrong stripe after it switches an index to a spare replica

["Siyao Meng (Jira)" <ji...@apache.org>]

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

Siyao Meng updated HDDS-16999:
------------------------------
    Attachment: TestBugMC2SpareReplicaWrongStripe.java

> EC reconstruction read returns data of the wrong stripe after it switches an index to a spare replica
> -----------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-16999
>                 URL: https://issues.apache.org/jira/browse/HDDS-16999
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Major
>         Attachments: MC-2-reposition-ec-stream-reopened-on-spare-replica.patch, TestBugMC2SpareReplicaWrongStripe.java
>
>
> h3. Mechanism
> When reading one EC index fails, {{ECBlockReconstructedStripeInputStream.readIntoBuffer}} closes that index's block stream with {{ECBlockInputStream.closeStream(int)}}, switches the index to a spare replica ({{shouldRetryFailedRead}}) and retries. {{getOrOpenStream}} opens the new block stream at offset 0, and {{ECBlockInputStream.seekStreamIfNecessary}} only moves a block stream to the current stripe when {{seeked}} is set. {{seek()}} and {{closeStreams()}} set it (the comment in {{closeStreams()}} says reopened streams must be seeked), {{closeStream(int)}} does not. So if the reconstruction reader was never seeked and the failure happens in stripe 1 or later, the spare serves the cell of stripe 0 for that index, and every following stripe is shifted by the same amount. Indexes decoded from it are wrong as well. No exception is raised.
> The non reconstruction reader is not affected, {{ECBlockInputStream.read}} calls {{seek()}} after switching to a spare.
> h3. Trigger
> # The read pipeline holds two replicas of one EC index. For a closed container SCM builds the read pipeline from every replica on a node that is not dead ({{ECPipelineProvider.createForRead}}), and the client keeps the second replica of an index as a spare. This is the case for example while a node holding a replica is decommissioning or entering maintenance and its replica has already been copied, or while a container is over replicated.
> # The block is read by the reconstruction reader, which was never seeked: a data index has no location when the block is opened, or the plain reader fails during the first read call at position 0 so {{ECBlockInputStreamProxy}} fails over without a seek. A plain sequential read never seeks, and an earlier key level {{seek()}} does not help when the reconstruction reader is only created afterwards by such a failover.
> # After the first stripe, reading the first replica of the duplicated index fails, and the reader continues on the spare.
> Each step was run with the client test replica streams, not on a cluster. Steps 1 and 2 on a real cluster are argued from code.
> h3. Impact
> The read returns wrong bytes for the duplicated index and for the indexes decoded from it, for the rest of the block, without an error. The spare returns a genuine chunk of the same internal block, so chunk checksum verification would not detect it (argued, the test streams have no checksums). The datanode side reconstruction uses the same class but has one source per index, so it never uses a spare and is not affected.
> h3. Reproduction
> PASS on unmodified source, deterministic. [^TestBugMC2SpareReplicaWrongStripe.java] ({{hadoop-hdds/client}}) reads a 3 stripe rs-3-2 block (1 KiB cells) through {{ECBlockInputStreamProxy}} with the stock {{ECBlockInputStreamFactoryImpl}}; the pipeline holds two replicas of index 1 and the first one fails after stripe 0. Two cases (index 2 missing at open, and failover from the plain reader at position 0) show stripe 1 of index 1 equal to stripe 0 of index 1, with every byte of index 1 and index 2 wrong in stripes 1 and 2 and index 3 correct. A control case that seeks to 0 first returns correct data. A passing test means the defect is present.
> h3. Patch
> [^MC-2-reposition-ec-stream-reopened-on-spare-replica.patch], against 1fb64c8e499161d1452ae6948ffd461fc9d2c302. It also applies to master at 8664dc4fc3c.
> {{closeStream(int)}} sets {{seeked}} when it drops a block stream, so a reopened stream is moved to the current stripe before it is read. The other callers are {{closeStreams()}}, which already sets the flag, and {{ECBlockInputStream.read}}, which either seeks right after or rethrows, after which the proxy discards that reader. Neither changes in behaviour. The only cost is a position check per cell read after a stream was closed, the same as after any seek.
> Covered by {{testSpareLocationReadsCurrentStripe}} in the existing {{TestECBlockReconstructedStripeInputStream}}: three stripes, index 1 with a spare, index 3 missing, the first replica of index 1 failing in stripe 1. It fails without the change (first byte of stripe 1 of index 1 differs) and passes with it. With the change the EC client stream suites (74 tests) and {{TestOzoneECClient}} plus {{TestKeyInputStreamEC}} (36 tests) pass and checkstyle is clean. With the patch the two defect cases of the reproduction fail because the data is correct.
> Found by TLA+ model checking and code review of the Ozone client read retry and failover paths under HDDS-15926, on commit 1fb64c8e499161d1452ae6948ffd461fc9d2c302. Checked against HDDS issues and apache/ozone pull requests for duplicates before filing (HDDS-7918 added the spare retry, its test uses a block shorter than one stripe). 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org