Partial segment loading: bootstrap-restored cache entries can fail to download after restart (reused BootstrapRangeReader throws)

[qgambit2]

Affected version

master (feature introduced in #19535, "on-demand partial segment loading"). The feature is gated off by default behind druid.segmentCache.virtualStorage + druid.segmentCache.virtualStoragePartialDownloadsEnabled.

Note: this was found by static analysis while reading the #19535 change; I have not reproduced it at runtime yet (a repro sketch is included below). Filing it because the code path looks deterministically broken across restarts and is currently untested.

Description

With partial downloads enabled, a segment can be cached with only some of its containers on local disk. After a process restart, PartialSegmentCacheBootstrap.reserveFromDisk restores such entries from disk — but builds each restored PartialSegmentMetadataCacheEntry with BootstrapRangeReader.INSTANCE, whose readRange() unconditionally throws:

// PartialSegmentCacheBootstrap.BootstrapRangeReader
public InputStream readRange(String filename, long offset, long length) {
  throw DruidException.defensive(
      "BootstrapRangeReader was asked to fetch [%s] @[%d:%d]; bootstrap should only read from local disk", ...);
}

A partially-downloaded segment restores with isFullyDownloaded() == false (that requires downloadedFiles.containsAll(metadata.getFiles().keySet()), while restoreBundlesFromDisk admits any bundle whose container files merely exist). When a query later acquires that segment, SegmentLocalCacheManager.acquireSegment opens a real working reader via tryOpenRangeReader, but findOrReservePartial reuses the pre-existing restored entry and discards that reader:

final ReservedPartial existing = findExistingPartialWithHold(dataSegment.getId());
if (existing != null) { return existing; }          // the real rangeReader arg is dropped
return reservePartial(dataSegment, rangeReader);     // only reached when no entry exists yet

The reader can't be replaced afterward — the entry's and the file mapper's rangeReader fields are both final, and doMount builds the mapper exactly once. So the download then runs through the restored entry's throw-only BootstrapRangeReader:

• AcquireMode.FULL: ensureAllDownloaded() → downloadContainer → streamRangeIntoContainer → rangeReader.readRange(...) → throws.
• AcquireMode.PARTIAL: at cursor build, mapFile → ensureFileDownloaded → streamRangeIntoContainer → rangeReader.readRange(...) → throws.

Impact

After any restart, a query that needs not-yet-cached bytes of a partially-downloaded segment fails with a DruidException (defensive) instead of fetching the missing bytes from deep storage — i.e., the feature's core lazy-load path is broken across restarts. Only deployments running partial downloads (the off-by-default experimental flags) are affected.

Existing mitigation doesn't cover this path

doMount's cleanup (and testMountFailureRemovesLingeringWeakEntry) handle the mount-time header-refetch case: on mount failure, removeUnheldWeakEntry drops the stale entry. But that only fires on mount failure. When the header file is present the mount succeeds, the entry persists, and the later container/file download throws with no fallback. SegmentLocalCacheManagerPartialAcquireTest exercises acquireSegment + download only via fresh reservations (real reader), so the restore-then-download path is untested.

Suggested fix

When an on-demand acquire reuses a pre-existing (bootstrap-restored) metadata entry, replace its BootstrapRangeReader with the real reader from tryOpenRangeReader — or evict + re-reserve the entry via reservePartial — so restored partial segments can complete downloads. A regression test that restores a partially-downloaded entry and then drives both a FULL and a PARTIAL acquire would lock it in.

Suggested reproduction (not yet run)

1. Enable druid.segmentCache.virtualStorage=true and druid.segmentCache.virtualStoragePartialDownloadsEnabled=true on a historical/MSQ worker (non-ephemeral, the default).
2. Run a query that lazily downloads only part of a segment, so its on-disk state is partial.
3. Restart the process (triggering bootstrap() → reserveFromDisk).
4. Run a query touching a column/range that wasn't previously downloaded.
5. Expected: the missing bytes are fetched from deep storage. Observed (per this trace): DruidException from BootstrapRangeReader.readRange.

Traced across PartialSegmentCacheBootstrap, SegmentLocalCacheManager, PartialSegmentMetadataCacheEntry, and PartialSegmentFileMapperV10 by inspection.