Andrey Yarovoy created HDDS-17094:
-------------------------------------
Summary: 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
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]
