Return-Path: X-Original-To: apmail-drill-commits-archive@www.apache.org Delivered-To: apmail-drill-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id D1ECA18599 for ; Thu, 4 Jun 2015 18:48:36 +0000 (UTC) Received: (qmail 29848 invoked by uid 500); 4 Jun 2015 18:48:36 -0000 Delivered-To: apmail-drill-commits-archive@drill.apache.org Received: (qmail 29820 invoked by uid 500); 4 Jun 2015 18:48:36 -0000 Mailing-List: contact commits-help@drill.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: commits@drill.apache.org Delivered-To: mailing list commits@drill.apache.org Received: (qmail 29810 invoked by uid 99); 4 Jun 2015 18:48:36 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 04 Jun 2015 18:48:36 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 80FFCE1083; Thu, 4 Jun 2015 18:48:36 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: mehant@apache.org To: commits@drill.apache.org Date: Thu, 04 Jun 2015 18:48:37 -0000 Message-Id: <21d32aaaca204983b611d068463f7d8a@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [2/3] drill git commit: DRILL-3155: Part 2: Clear allocated memory for composite vectors if one of the allocations fails DRILL-3155: Part 2: Clear allocated memory for composite vectors if one of the allocations fails Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/09e46df0 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/09e46df0 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/09e46df0 Branch: refs/heads/master Commit: 09e46df0dada514aeca16cf80a71fa7179f1d8ce Parents: 21de138 Author: Mehant Baid Authored: Sat May 30 00:31:29 2015 -0700 Committer: Mehant Baid Committed: Wed Jun 3 15:08:08 2015 -0700 ---------------------------------------------------------------------- .../codegen/templates/NullableValueVectors.java | 61 ++++++++++++++++---- .../codegen/templates/RepeatedValueVectors.java | 52 ++++++++++++++--- .../templates/VariableLengthVectors.java | 46 ++++++++++----- .../exec/vector/complex/AbstractMapVector.java | 20 ++++++- .../vector/complex/BaseRepeatedValueVector.java | 20 ++++++- .../exec/vector/complex/RepeatedMapVector.java | 33 ++++++++--- 6 files changed, 185 insertions(+), 47 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/09e46df0/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java index 90ec6be..7f83542 100644 --- a/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java +++ b/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java @@ -124,8 +124,21 @@ public final class ${className} extends BaseDataValueVector implements <#if type @Override public boolean allocateNewSafe() { - if(!values.allocateNewSafe()) return false; - if(!bits.allocateNewSafe()) return false; + /* Boolean to keep track if all the memory allocations were successful + * Used in the case of composite vectors when we need to allocate multiple + * buffers for multiple vectors. If one of the allocations failed we need to + * clear all the memory that we allocated + */ + boolean success = false; + try { + if(!values.allocateNewSafe()) return false; + if(!bits.allocateNewSafe()) return false; + success = true; + } finally { + if (!success) { + clear(); + } + } bits.zeroVector(); mutator.reset(); accessor.reset(); @@ -134,8 +147,13 @@ public final class ${className} extends BaseDataValueVector implements <#if type @Override public void allocateNew(int totalBytes, int valueCount) { - values.allocateNew(totalBytes, valueCount); - bits.allocateNew(valueCount); + try { + values.allocateNew(totalBytes, valueCount); + bits.allocateNew(valueCount); + } catch(OutOfMemoryRuntimeException e){ + clear(); + throw e; + } bits.zeroVector(); mutator.reset(); accessor.reset(); @@ -175,8 +193,13 @@ public final class ${className} extends BaseDataValueVector implements <#if type @Override public void allocateNew() { - values.allocateNew(); - bits.allocateNew(); + try { + values.allocateNew(); + bits.allocateNew(); + } catch(OutOfMemoryRuntimeException e) { + clear(); + throw e; + } bits.zeroVector(); mutator.reset(); accessor.reset(); @@ -185,8 +208,21 @@ public final class ${className} extends BaseDataValueVector implements <#if type @Override public boolean allocateNewSafe() { - if(!values.allocateNewSafe()) return false; - if(!bits.allocateNewSafe()) return false; + /* Boolean to keep track if all the memory allocations were successful + * Used in the case of composite vectors when we need to allocate multiple + * buffers for multiple vectors. If one of the allocations failed we need to + * clear all the memory that we allocated + */ + boolean success = false; + try { + if(!values.allocateNewSafe()) return false; + if(!bits.allocateNewSafe()) return false; + success = true; + } finally { + if (!success) { + clear(); + } + } bits.zeroVector(); mutator.reset(); accessor.reset(); @@ -195,8 +231,13 @@ public final class ${className} extends BaseDataValueVector implements <#if type @Override public void allocateNew(int valueCount) { - values.allocateNew(valueCount); - bits.allocateNew(valueCount); + try { + values.allocateNew(valueCount); + bits.allocateNew(valueCount); + } catch(OutOfMemoryRuntimeException e) { + clear(); + throw e; + } bits.zeroVector(); mutator.reset(); accessor.reset(); http://git-wip-us.apache.org/repos/asf/drill/blob/09e46df0/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java b/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java index 813c3f8..12dce25 100644 --- a/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java +++ b/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java @@ -177,18 +177,36 @@ public final class Repeated${minor.class}Vector extends BaseRepeatedValueVector } - public boolean allocateNewSafe(){ - if(!offsets.allocateNewSafe()) return false; + public boolean allocateNewSafe() { + /* boolean to keep track if all the memory allocation were successful + * Used in the case of composite vectors when we need to allocate multiple + * buffers for multiple vectors. If one of the allocations failed we need to + * clear all the memory that we allocated + */ + boolean success = false; + try { + if(!offsets.allocateNewSafe()) return false; + if(!values.allocateNewSafe()) return false; + success = true; + } finally { + if (!success) { + clear(); + } + } offsets.zeroVector(); - if(!values.allocateNewSafe()) return false; mutator.reset(); return true; } public void allocateNew() { - offsets.allocateNew(); + try { + offsets.allocateNew(); + values.allocateNew(); + } catch (OutOfMemoryRuntimeException e) { + clear(); + throw e; + } offsets.zeroVector(); - values.allocateNew(); mutator.reset(); } @@ -200,9 +218,14 @@ public final class Repeated${minor.class}Vector extends BaseRepeatedValueVector } public void allocateNew(int totalBytes, int valueCount, int innerValueCount) { - offsets.allocateNew(valueCount+1); + try { + offsets.allocateNew(valueCount+1); + values.allocateNew(totalBytes, innerValueCount); + } catch (OutOfMemoryRuntimeException e) { + clear(); + throw e; + } offsets.zeroVector(); - values.allocateNew(totalBytes, innerValueCount); mutator.reset(); } @@ -230,9 +253,20 @@ public final class Repeated${minor.class}Vector extends BaseRepeatedValueVector public void allocateNew(int valueCount, int innerValueCount) { clear(); - offsets.allocateNew(valueCount+1); + /* boolean to keep track if all the memory allocation were successful + * Used in the case of composite vectors when we need to allocate multiple + * buffers for multiple vectors. If one of the allocations failed we need to + * clear all the memory that we allocated + */ + boolean success = false; + try { + offsets.allocateNew(valueCount+1); + values.allocateNew(innerValueCount); + } catch(OutOfMemoryRuntimeException e){ + clear(); + throw e; + } offsets.zeroVector(); - values.allocateNew(innerValueCount); mutator.reset(); } http://git-wip-us.apache.org/repos/asf/drill/blob/09e46df0/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java index b3389e2..bd41e10 100644 --- a/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java +++ b/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java @@ -18,6 +18,7 @@ import java.lang.Override; +import org.apache.drill.exec.memory.OutOfMemoryRuntimeException; import org.apache.drill.exec.vector.BaseDataValueVector; import org.apache.drill.exec.vector.BaseValueVector; import org.apache.drill.exec.vector.VariableWidthVector; @@ -282,17 +283,28 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V allocationMonitor = 0; } - DrillBuf newBuf = allocator.buffer(allocationTotalByteCount); - if(newBuf == null){ - return false; + /* Boolean to keep track if all the memory allocations were successful + * Used in the case of composite vectors when we need to allocate multiple + * buffers for multiple vectors. If one of the allocations failed we need to + * clear all the memory that we allocated + */ + boolean success = false; + try { + DrillBuf newBuf = allocator.buffer(allocationTotalByteCount); + if (newBuf == null) { + return false; + } + this.data = newBuf; + if (!offsetVector.allocateNewSafe()) { + return false; + } + success = true; + } finally { + if (!success) { + clear(); + } } - - this.data = newBuf; data.readerIndex(0); - - if(!offsetVector.allocateNewSafe()){ - return false; - } offsetVector.zeroVector(); return true; } @@ -300,15 +312,19 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V public void allocateNew(int totalBytes, int valueCount) { clear(); assert totalBytes >= 0; - DrillBuf newBuf = allocator.buffer(totalBytes); - if(newBuf == null){ - throw new OutOfMemoryRuntimeException(String.format("Failure while allocating buffer of %d bytes", totalBytes)); + try { + DrillBuf newBuf = allocator.buffer(totalBytes); + if (newBuf == null) { + throw new OutOfMemoryRuntimeException(String.format("Failure while allocating buffer of %d bytes", totalBytes)); + } + this.data = newBuf; + offsetVector.allocateNew(valueCount + 1); + } catch (OutOfMemoryRuntimeException e) { + clear(); + throw e; } - - this.data = newBuf; data.readerIndex(0); allocationTotalByteCount = totalBytes; - offsetVector.allocateNew(valueCount+1); offsetVector.zeroVector(); } http://git-wip-us.apache.org/repos/asf/drill/blob/09e46df0/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java index 3c01939..af364bd 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java @@ -52,9 +52,23 @@ public abstract class AbstractMapVector extends AbstractContainerVector { @Override public boolean allocateNewSafe() { - for (ValueVector v : vectors.values()) { - if (!v.allocateNewSafe()) { - return false; + /* boolean to keep track if all the memory allocation were successful + * Used in the case of composite vectors when we need to allocate multiple + * buffers for multiple vectors. If one of the allocations failed we need to + * clear all the memory that we allocated + */ + boolean success = false; + try { + + for (ValueVector v : vectors.values()) { + if (!v.allocateNewSafe()) { + return false; + } + } + success = true; + } finally { + if (!success) { + clear(); } } return true; http://git-wip-us.apache.org/repos/asf/drill/blob/09e46df0/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java index 88b44db..f292e4c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java @@ -61,13 +61,27 @@ public abstract class BaseRepeatedValueVector extends BaseValueVector implements @Override public boolean allocateNewSafe() { - if (!offsets.allocateNewSafe()) { - return false; + /* boolean to keep track if all the memory allocation were successful + * Used in the case of composite vectors when we need to allocate multiple + * buffers for multiple vectors. If one of the allocations failed we need to + * clear all the memory that we allocated + */ + boolean success = false; + try { + if (!offsets.allocateNewSafe()) { + return false; + } + success = vector.allocateNewSafe(); + } finally { + if (!success) { + clear(); + } } offsets.zeroVector(); - return vector.allocateNewSafe(); + return success; } + @Override public UInt4Vector getOffsetVector() { return offsets; http://git-wip-us.apache.org/repos/asf/drill/blob/09e46df0/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java index 2e12e55..4617ede 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java @@ -35,6 +35,7 @@ import org.apache.drill.exec.expr.TypeHelper; import org.apache.drill.exec.expr.holders.ComplexHolder; import org.apache.drill.exec.expr.holders.RepeatedMapHolder; import org.apache.drill.exec.memory.BufferAllocator; +import org.apache.drill.exec.memory.OutOfMemoryRuntimeException; import org.apache.drill.exec.proto.UserBitShared.SerializedField; import org.apache.drill.exec.record.MaterializedField; import org.apache.drill.exec.record.TransferPair; @@ -103,11 +104,16 @@ public class RepeatedMapVector extends AbstractMapVector implements RepeatedValu @Override public void allocateNew(int groupCount, int innerValueCount) { clear(); - offsets.allocateNew(groupCount+1); - offsets.zeroVector(); - for (ValueVector v : getChildren()) { - AllocationHelper.allocatePrecomputedChildCount(v, groupCount, 50, innerValueCount); + try { + offsets.allocateNew(groupCount + 1); + for (ValueVector v : getChildren()) { + AllocationHelper.allocatePrecomputedChildCount(v, groupCount, 50, innerValueCount); + } + } catch (OutOfMemoryRuntimeException e){ + clear(); + throw e; } + offsets.zeroVector(); mutator.reset(); } @@ -216,11 +222,24 @@ public class RepeatedMapVector extends AbstractMapVector implements RepeatedValu @Override public boolean allocateNewSafe() { - if (!offsets.allocateNewSafe()) { - return false; + /* boolean to keep track if all the memory allocation were successful + * Used in the case of composite vectors when we need to allocate multiple + * buffers for multiple vectors. If one of the allocations failed we need to + * clear all the memory that we allocated + */ + boolean success = false; + try { + if (!offsets.allocateNewSafe()) { + return false; + } + success = super.allocateNewSafe(); + } finally { + if (!success) { + clear(); + } } offsets.zeroVector(); - return super.allocateNewSafe(); + return success; } protected static class SingleMapTransferPair implements TransferPair {