[
https://issues.apache.org/jira/browse/HDDS-16999?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16999:
------------------------------
Attachment: MC-2-reposition-ec-stream-reopened-on-spare-replica.patch
> 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: [email protected]
For additional commands, e-mail: [email protected]
