[
https://issues.apache.org/jira/browse/HDDS-17094?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrey Yarovoy reassigned HDDS-17094:
-------------------------------------
Assignee: Andrey Yarovoy
> ECBlockInputStream serial read can return wrong data on a retry after a
> failed read
> -----------------------------------------------------------------------------------
>
> Key: HDDS-17094
> URL: https://issues.apache.org/jira/browse/HDDS-17094
> Project: Apache Ozone
> Issue Type: Bug
> Components: EC Client
> Reporter: Andrey Yarovoy
> Assignee: Andrey Yarovoy
> Priority: Minor
>
> h3. Problem
> `ECBlockInputStream` keeps its own `position` for the block group and one
> inner block stream per data index. In the common case without seeks it trusts
> that each inner stream is already at the right offset:
> `seekStreamIfNecessary` only checks and re-seeks inner streams after `seeked`
> has been set by `seek()` or `closeStreams()`.
> `readWithStrategy` advances `position` only after a cell has been read fully.
> If the inner stream fails partway through a cell, it has already moved
> forward by the bytes it returned before failing, but `position` has not. If
> `seeked` is still false, the next read on the same `ECBlockInputStream` reads
> that index from the stream's current offset, not the offset `position`
> implies, and silently returns the wrong data.
> Reproduced in a unit test with rs-3-2, 100-byte cells, serial read:
> # read 101 bytes: the index 2 stream is at offset 1, `position` is 101
> # index 2 fails after 49 more bytes: `BadDataLocationException`, `position`
> stays 101, the index 2 stream is at 50
> # clear the error and read 199 bytes without a seek: the first byte read does
> not match the block data at offset 101
> h3. Impact
> The main read path is not affected:
> * every `ECBlockInputStream` is created by `ECBlockInputStreamFactoryImpl`
> behind `ECBlockInputStreamProxy`
> * `readWithStrategy` wraps each `IOException` in a
> `BadDataLocationException`, and on that exception the proxy closes the reader
> and fails over to reconstruction, so the stream with the stale offset is
> never read again
> The exposure is limited to an unchecked exception thrown by an inner block
> stream. `readWithStrategy` does not catch it, and the proxy rethrows it but
> keeps the same reader. A caller that catches it and reads again from the same
> stream would get wrong data, with no error. It is not known whether any inner
> stream throws unchecked exceptions on a read in practice. This is a latent
> correctness hazard rather than an observed failure.
> h3. Proposed fix
> When `readWithStrategy` exits with any exception, set `seeked = true`, so
> that the next read checks and re-seeks the inner streams, as after a `seek()`:
> {code:java}
> boolean completed = false;
> try {
> ... // existing loop
> completed = true;
> } finally {
> if (!completed) {
> seeked = true;
> }
> }
> {code}
> h3. Testing
> Add a `TestECBlockInputStream` case: a cell fails partway
> (`TestBlockInputStream#setShouldError(true, position, error)`), the error is
> cleared, and a read on the same stream without a seek must return the correct
> bytes. Cover both an `IOException` and an unchecked exception from the inner
> stream.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
