[IMPALA-4249] Clarify lifetime of DiskIoMgr::ScanRange objects

• Type: Improvement
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Affects Version/s: Impala 2.8.0
• Component/s: Backend
• Labels: resource-management
• Target Version: Product Backlog
• Assignee: Unassigned
• Reporter: Tim Armstrong
• Created: 05/Oct/16 17:16
• Updated: 22/Jan/19 19:42

Description

When working on the DiskIoMgr code I noticed that the lifetime of the ScanRange objects isn't very clear. In particular, it's not clear when it is safe for a client to free the ScanRange object. We should at least document the expectation in the code.

It seems like in principle that the client should be able to free the scan range after it hits eosr() or cancels the scan range. However, that's not true because a) BufferDescriptors hold internal pointers to the ScanRange and b) the DiskIoMgr touches the ScanRange in the below code after enqueueing the last buffer:

  bool queue_full = scan_range->EnqueueBuffer(buffer);
  if (eosr) {
    // For cached buffers, we can't close the range until the cached buffer is returned.
    // Close() is called from DiskIoMgr::ReturnBuffer().
    if (!is_cached) scan_range->Close();
  } else {
...

It seems like we (mostly?) avoid issues by sticking the ScanRanges in object pools where they will outlive the reader contexts. This seems suboptimal - we shouldn't really leave lots of small unused objects lying around.

Issue Links

• is related to: IMPALA-6587 Crash in DiskMgr::AllocateBuffersForRange
• is related to: IMPALA-8090 DiskIoMgrTest.SyncReadTest hits file_ != nullptr DCHECK in LocalFileReader::ReadFromPos()