Siyao Meng created HDDS-16999:
---------------------------------
Summary: 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
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]
