László Bodor created HIVE-30065:
-----------------------------------
Summary: EncodedReaderImpl.putFileData batch calls can leak cache
locks on partial failure
Key: HIVE-30065
URL: https://issues.apache.org/jira/browse/HIVE-30065
Project: Hive
Issue Type: Bug
Reporter: László Bodor
{{EncodedReaderImpl}} calls {{cacheWrapper.putFileData(fileKey, cacheKeys,
targetBuffers, baseOffset, tag)}} with the full batch in one shot at two sites:
* [EncodedReaderImpl.java:1071 -- prepareRangesForCompressedRead (uncompressed
CB
put)|https://github.com/apache/hive/blob/4cf07f490af49d0e0e019d07c45b629ed8802136/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java#L1071-L1074]
* [EncodedReaderImpl.java:1381 -- preReadUncompressedStreams (pre-read
uncompressed
put)|https://github.com/apache/hive/blob/4cf07f490af49d0e0e019d07c45b629ed8802136/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java#L1380-L1384]
{{LowLevelCacheImpl.putFileData}} processes the array one entry at a time and
can throw {{RuntimeException}} mid-array -- the [length-mismatch guard at
LowLevelCacheImpl.java:360|https://github.com/apache/hive/blob/4cf07f490af49d0e0e019d07c45b629ed8802136/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheImpl.java#L360]
({{"Found a block with different length at the same offset..."}}) is the
concrete case. When it throws at index {{k}}:
* Entries {{0..k-1}} were already inserted and locked in the sub-cache.
* The caller unwinds without running
[processCacheCollisions|https://github.com/apache/hive/blob/4cf07f490af49d0e0e019d07c45b629ed8802136/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java#L1545],
so the swap-out ({{allocator.deallocate}} our fresh buffer, adopt the cache's)
never happens.
* Ownership of the already-inserted buffers is now split-brain: the cache is
holding a lock on them, and the caller still treats them as raw allocations,
deallocating them on cleanup. Classic use-after-free on the cache side.
The Parquet native cache reader has the same shape and was fixed on HIVE-30059
by inserting one range at a time so ownership flips atomically per entry -- see
{{ParquetEncodedDataReader.putColumn}} in [PR
#6793|https://github.com/apache/hive/pull/6793] for the pattern (one-entry
{{putFileData}} loop, {{part.owned = true}} set per successful insert, cleanup
routes each part correctly on a mid-run throw).
The length-mismatch throw is normally an invariant violation (matching ranges
must have matching sizes), so this is latent rather than routinely-hit -- but
it's a real hazard that survives any future path that widens what
{{putFileData}} may reject mid-batch (compression stats mismatch, checksum,
quota, etc.).
h3. Proposed fix
Convert both call sites to one-entry-at-a-time {{putFileData}} calls, mirroring
{{ParquetEncodedDataReader.putColumn}}, and fold the corresponding
{{processCacheCollisions}} handling into the per-entry loop. Alternative: make
[LowLevelCacheImpl.putFileData|https://github.com/apache/hive/blob/4cf07f490af49d0e0e019d07c45b629ed8802136/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheImpl.java#L319]
transactional (roll back the entries it already inserted before throwing) --
probably simpler and fixes ORC and Parquet uniformly, but changes a
widely-shared contract.
h3. Related
* HIVE-30059
* [PR #6793|https://github.com/apache/hive/pull/6793]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
