[jira] [Created] (DRILL-7658) Vector allocateNew() has poor error reporting

[Paul Rogers (Jira)]

Paul Rogers created DRILL-7658:
----------------------------------

             Summary: Vector allocateNew() has poor error reporting
                 Key: DRILL-7658
                 URL: https://issues.apache.org/jira/browse/DRILL-7658
             Project: Apache Drill
          Issue Type: Bug
    Affects Versions: 1.17.0
            Reporter: Paul Rogers


See posting by Charles on 2020-03-24 on the user and dev lists of a message 
forwarded from another user where a query ran out of memory. Stack trace:

{noformat}
Caused by: org.apache.drill.exec.exception.OutOfMemoryException: null
    at 
org.apache.drill.exec.vector.complex.AbstractContainerVector.allocateNew(AbstractContainerVector.java:59)
    at 
org.apache.drill.exec.test.generated.PartitionerGen5$OutgoingRecordBatch.allocateOutgoingRecordBatch(PartitionerTemplate.
{noformat}

Notice the complete lack of context. The method in question:

{code:java}
  public void allocateNew() throws OutOfMemoryException {
   if (!allocateNewSafe()) {
     throw new OutOfMemoryException();
     }
   }
{code}

A generated implementation of the {{allocateNewSafe()}} method:

{code:java}
  @Override
  public boolean allocateNewSafe() {
    long curAllocationSize = allocationSizeInBytes;
    if (allocationMonitor > 10) {
      curAllocationSize = Math.max(8, curAllocationSize / 2);
      allocationMonitor = 0;
    } else if (allocationMonitor < -2) {
      curAllocationSize = allocationSizeInBytes * 2L;
      allocationMonitor = 0;
    }

    try{
      allocateBytes(curAllocationSize);
    } catch (DrillRuntimeException ex) {
      return false;
    }
    return true;
  }
{code}

Note that the {{allocateNew()}} method is not "safe" (it throws an exception), 
but it does so by discarding the underlying exception. What should happen is 
that the "non-safe" {{allocateNew()}} should call the {{allocateBytes()}} 
method and simply forward the {{DrillRuntimeException}}. It probably does not 
do so because the author wanted to reuse the extra size calcs in 
{{allocateNewSafe()}}.

The solution is to put the calcs and the call to {{allocateBytes()}} in a 
"non-safe" method, and call that entire method from {{allocateNew()}} and 
{{allocateNewSafe()}}.  Or, better, generate {{allocateNew()}} using the above 
code, but have the base class define {{allocateNewSafe()}} as a wrapper.

Note an extra complexity: although the base class provides the method shown 
above, each generated vector also provides:

{code:java}
  @Override
  public void allocateNew() {
    if (!allocateNewSafe()) {
      throw new OutOfMemoryException("Failure while allocating buffer.");
    }
  }
{code}

Which is both redundant and inconsistent (one has a message, the other does 
not.)



--
This message was sent by Atlassian Jira
(v8.3.4#803005)