[I] [kv] RocksDBKv.multiGet reports read errors as missing keys [fluss]

["fresh-borzoni (via GitHub)" <gi...@apache.org>]

fresh-borzoni opened a new issue, #4484:
URL: https://github.com/apache/fluss/issues/4484

   ### Search before asking
   
   - [x] I searched in the [issues](https://github.com/apache/fluss/issues) and found nothing similar.
   
   
   ### Fluss version
   
   main (development)
   
   ### Please describe the bug 🐞
   
   RocksDBKv#multiGet calls db.multiGetAsList, which returns null for a key whose read fails. So a corrupted SST makes the key look absent, and lookups return "not found" with no error.
   
   Repro: write keys, flush, flip bytes in an SST data block while keeping its size. db.get(key) throws block checksum mismatch, while multiGet returns null.
   
   Expected: a read error fails the lookup, like get does.
   
   ### Solution
   
   RocksJava 6.20.3 has no status-returning multiGet. 
   Re-check null results with db.get(key), which throws on read errors. Genuine misses pay one extra lookup which is still cheap as it should be bloom filter path.
   
   Also check whether prefixLookup checks iterator.status().
   
   ### Are you willing to submit a PR?
   
   - [x] I'm willing to submit a PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [kv] RocksDBKv.multiGet reports read errors as missing keys [fluss]

["zhang-arvin (via GitHub)" <gi...@apache.org>]

zhang-arvin commented on issue #4484:
URL: https://github.com/apache/fluss/issues/4484#issuecomment-5836107941

   PR opened: https://github.com/apache/fluss/pull/4488
   
   Implemented the direction described here:
   
   - `multiGet` now re-checks every `null` returned by `multiGetAsList` with `get()`, so a read error surfaces instead of being reported as a missing key. Genuine misses only pay one extra (bloom-filtered) lookup.
   - Audited the prefix-iteration path as suggested: `prefixLookup` had the same class of issue, and so did `limitScan` - an iterator stopped by an error is indistinguishable from one that reached the end of its range by `isValid()` alone. Both now check `status()` before closing the iterator rather than silently truncating the result set.
   
   One note on the regression test, since it differs from the corruption recipe in the report: flipping bytes in an SST does make `get()` throw, but RocksDB opens a database with `paranoid_checks` enabled by default, so a corrupted SST makes the instance fail to open at all - the read path is never reached. The test therefore injects the failure through `get()`, the method `multiGet` uses for the re-check, and asserts both directions (a read error fails the lookup, a genuine miss still returns `null`).
   
   Local runs: `mvn -pl fluss-server test -Dtest=RocksDBKvTest` -> 13 tests, 0 failures; `spotless:check` clean.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [kv] RocksDBKv.multiGet reports read errors as missing keys [fluss]

["zhang-arvin (via GitHub)" <gi...@apache.org>]

zhang-arvin commented on issue #4484:
URL: https://github.com/apache/fluss/issues/4484#issuecomment-5835494632

   @zhang-arvin here — I'd like to take this one.
   
   Confirmed the root cause in `RocksDBKv#multiGet`: it delegates to `db.multiGetAsList`, which yields `null` for a key whose read fails, so a corrupted SST surfaces as "not found" instead of an error — while the single-key `db.get(key)` path correctly throws (e.g. block checksum mismatch).
   
   Plan:
   1. Re-check every `null` element of the `multiGetAsList` result with `db.get(key)`, letting a real read error throw instead of being swallowed. Genuine misses only cost one extra bloom-filtered lookup.
   2. Audit the prefix-iteration path for the same class of issue (`iterator.status()` / `RocksDBException` handling).
   3. Add a regression test that flips bytes in an SST data block (size preserved) and asserts `multiGet` fails rather than returning `null`.
   
   Please assign this to me if convenient.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@fluss.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org