[jira] [Created] (HDDS-17001) EC reconstruction read returns unfilled stripe buffers when the stream is read again after a failed read or seek

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

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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org