[IMPALA-2620] FunctionContext::Allocate() doesn't check against mem_limit

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: Impala 2.3.0
• Fix Version/s: Impala 2.5.0
• Labels: resource-management
• Target Version: Product Backlog
• Assignee: Michael Ho
• Reporter: Daniel Hecht
• Created: 02/Nov/15 20:48
• Updated: 19/Dec/15 07:12
• Resolved: 19/Dec/15 07:12

Description

udf.h has this comment:

  /// Allocates memory. All UDF/UDAs should use this if possible instead of
  /// malloc/new. The UDF/UDA is responsible for calling Free() on all buffers returned by
  /// Allocate(). If this Allocate causes the memory limit to be exceeded, the error will
  /// be set in this object causing the query to fail.
  uint8_t* Allocate(int byte_size);

However, the implementation of Allocate() doesn't actually check against the memory limit:

uint8_t* FunctionContext::Allocate(int byte_size) {
  assert(!impl_->closed_);
  if (byte_size == 0) return NULL;
  uint8_t* buffer = impl_->pool_->Allocate(byte_size);
  if (impl_->debug_) {
    impl_->allocations_[buffer] = byte_size;
    memset(buffer, 0xff, byte_size);
  }
  VLOG_ROW << "Allocate: FunctionContext=" << this
           << " size=" << byte_size
           << " result=" << reinterpret_cast<void*>(buffer);
  return buffer;
}

This can manifest as a query going over the memory limit before failing the query. All mem limits are still checked periodically so the query shouldn't continue indefinitely. This could be particularly bad for expr evaluations that take a long time to run though, since the mem limit checks will happen less frequently then. In most cases I imagine the query will be cancelled quickly, but the query will still spike over the limit.

Workaround
Potential workaround: use the batch_size to use a lower batch size than the default (1024 rows). This determines how often the mem limits are checked, but lowering the batch size can also slow down the query.