Siyao Meng created HDDS-17001:
---------------------------------
Summary: EC reconstruction read returns unfilled stripe buffers
when the stream is read again after a failed read or seek
Key: HDDS-17001
URL: https://issues.apache.org/jira/browse/HDDS-17001
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-3-invalidate-ec-reconstruction-buffers-on-failed-stripe-read.patch,
TestBugMC3StaleReconstructionBuffers.java
h3. Mechanism
{{ECBlockReconstructedInputStream.readStripe}} calls {{clearBuffers()}}, which
sets every stripe buffer to position 0 and limit of one cell, and then
{{stripeReader.readStripe(bufs)}}. If the stripe read throws (for example
{{InsufficientLocationsException}} once too many replicas have failed), the
buffers stay readable. The next {{read()}} on the same stream finds bytes
remaining in {{selectNextBuffer}}, copies the buffers to the caller without any
I/O and advances the position, without an error.
{{seek()}} has the same problem: it moves the stripe reader to the target
stripe and loads that stripe before it updates {{position}}. If loading fails,
the buffers hold part of the target stripe while {{position}} is still the old
one, and the next {{read()}} returns that data as if it were at the old
position.
h3. Trigger
# A block is read by the reconstruction reader, and a stripe read or a
{{seek()}} fails because not enough replicas can be read.
# The caller catches the {{IOException}} and reads the same stream again.
No in-tree caller does step 2: {{KeyInputStream}}, {{OzoneFSInputStream}}, the
S3 gateway GET and {{ozone sh key get}} pass the exception up, and positioned
reads use a fresh reader per call. An application that retries a read on the
same {{FSDataInputStream}} or {{OzoneInputStream}} after an exception is
exposed.
h3. Impact
The repeated read returns up to one stripe from the buffers, in which the bytes
that were not loaded before the failure (about one cell in the reproduction)
are not the block data, and the position moves past them. After a failed
{{seek()}} the returned data belongs to another stripe. A stream whose replicas
are exhausted should throw again instead.
h3. Reproduction
PASS on unmodified source, deterministic.
[^TestBugMC3StaleReconstructionBuffers.java] ({{hadoop-hdds/client}}) uses
rs-3-2 with 1 MiB cells, the stock {{ECBlockInputStreamFactoryImpl}} and the
client test replica streams for datanode read errors. Three cases: failover
from the plain reader at position 0 followed by exhausted replicas, a failed
{{seek()}} into stripe 2 after stripe 0 was read, and a failed read through
{{MultipartInputStream}} (the base class of {{KeyInputStream}}). In each the
caller catches {{InsufficientLocationsException}} and reads again, gets 3 MiB
without an exception, and about 1 MiB (3 MiB after the failed seek) differs
from the block data. A passing test means the defect is present.
h3. Patch
[^MC-3-invalidate-ec-reconstruction-buffers-on-failed-stripe-read.patch],
against 1fb64c8e499161d1452ae6948ffd461fc9d2c302. It also applies to master at
8664dc4fc3c.
When the stripe read fails, with an {{IOException}} or a {{RuntimeException}},
{{readStripe}} sets the limit of every stripe buffer to 0 and sets the existing
{{unBuffered}} flag before rethrowing, so nothing from the failed stripe is
returned and the next {{read()}} first seeks the stripe reader back to the
current position, as it does after {{unbuffer()}}. With exhausted replicas the
next read throws again. After a failed {{seek()}} the stream stays at its old
position.
Covered by two tests in the existing {{TestECBlockReconstructedInputStream}}:
{{testReadFailsAgainAfterFailedStripeRead}} (exhausted replicas; read, read,
seek and read all throw, position stays 0) and
{{testFailedSeekLeavesStreamAtOldPosition}} (the stripe read of a {{seek()}}
fails once with an injected exception after the stripe reader has moved to the
target stripe; the next read must return the data at the old position). Both
fail without the change. The second also fails if only the limits are reset
without the re-seek, and if only {{IOException}} is handled. With the change
the EC client stream suites (75 tests) and {{TestOzoneECClient}} plus
{{TestKeyInputStreamEC}} (36 tests) pass and checkstyle is clean. With the
patch all three reproduction cases fail because the second read throws
{{InsufficientLocationsException}}.
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. 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]
