You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by bo...@apache.org on 2019/08/12 17:28:56 UTC
[drill] 01/04: DRILL-7341: Vector reAlloc may fail after exchange
This is an automated email from the ASF dual-hosted git repository.
bohdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
commit e5ce59ee6fe328ff338556ed6c5ff6adfc6dd64c
Author: ozinoviev <oz...@solit-clouds.ru>
AuthorDate: Wed Aug 7 16:38:01 2019 +0300
DRILL-7341: Vector reAlloc may fail after exchange
closes #1838
---
.../main/codegen/templates/FixedValueVectors.java | 12 ++++++++---
.../codegen/templates/VariableLengthVectors.java | 9 +++++++-
.../org/apache/drill/exec/vector/BitVector.java | 9 +++++++-
.../exec/vector/VariableLengthVectorTest.java | 24 ++++++++++++++++++++++
4 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/exec/vector/src/main/codegen/templates/FixedValueVectors.java b/exec/vector/src/main/codegen/templates/FixedValueVectors.java
index 508e484..5741555 100644
--- a/exec/vector/src/main/codegen/templates/FixedValueVectors.java
+++ b/exec/vector/src/main/codegen/templates/FixedValueVectors.java
@@ -210,10 +210,17 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements F
// a zero-length buffer. Instead, just allocate a 256 byte
// buffer if we start at 0.
- final long newAllocationSize = allocationSizeInBytes == 0
+ long newAllocationSize = allocationSizeInBytes == 0
? 256
: allocationSizeInBytes * 2L;
+ final int currentCapacity = data.capacity();
+ // Some operations, such as Value Vector#exchange, can be change DrillBuf data field without corresponding allocation size changes.
+ // Check that the size of the allocation is sufficient to copy the old buffer.
+ while (newAllocationSize < currentCapacity) {
+ newAllocationSize *= 2L;
+ }
+
// TODO: Replace this with MAX_BUFFER_SIZE once all
// code is aware of the maximum vector size.
@@ -222,8 +229,7 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements F
}
reallocRaw((int) newAllocationSize);
- final int halfNewCapacity = data.capacity() / 2;
- data.setZero(halfNewCapacity, halfNewCapacity);
+ data.setZero(currentCapacity, data.capacity() - currentCapacity);
}
@Override
diff --git a/exec/vector/src/main/codegen/templates/VariableLengthVectors.java b/exec/vector/src/main/codegen/templates/VariableLengthVectors.java
index 8dd8eb1..1bb758f 100644
--- a/exec/vector/src/main/codegen/templates/VariableLengthVectors.java
+++ b/exec/vector/src/main/codegen/templates/VariableLengthVectors.java
@@ -390,7 +390,14 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V
}
public void reAlloc() {
- final long newAllocationSize = allocationSizeInBytes*2L;
+ long newAllocationSize = allocationSizeInBytes*2L;
+
+ // Some operations, such as Value Vector#exchange, can be change DrillBuf data field without corresponding allocation size changes.
+ // Check that the size of the allocation is sufficient to copy the old buffer.
+ while (newAllocationSize < data.capacity()) {
+ newAllocationSize *= 2L;
+ }
+
if (newAllocationSize > MAX_ALLOCATION_SIZE) {
throw new OversizedAllocationException("Unable to expand the buffer. Max allowed buffer size is reached.");
}
diff --git a/exec/vector/src/main/java/org/apache/drill/exec/vector/BitVector.java b/exec/vector/src/main/java/org/apache/drill/exec/vector/BitVector.java
index 634e4d6..7b2cd28 100644
--- a/exec/vector/src/main/java/org/apache/drill/exec/vector/BitVector.java
+++ b/exec/vector/src/main/java/org/apache/drill/exec/vector/BitVector.java
@@ -179,7 +179,14 @@ public final class BitVector extends BaseDataValueVector implements FixedWidthVe
* Allocate new buffer with double capacity, and copy data into the new buffer. Replace vector's buffer with new buffer, and release old one
*/
public void reAlloc() {
- final long newAllocationSize = allocationSizeInBytes * 2L;
+ long newAllocationSize = allocationSizeInBytes * 2L;
+
+ // Some operations, such as Value Vector#exchange, can be change DrillBuf data field without corresponding allocation size changes.
+ // Check that the size of the allocation is sufficient to copy the old buffer.
+ while (newAllocationSize < data.capacity()) {
+ newAllocationSize *= 2L;
+ }
+
if (newAllocationSize > MAX_ALLOCATION_SIZE) {
throw new OversizedAllocationException("Requested amount of memory is more than max allowed allocation size");
}
diff --git a/exec/vector/src/test/java/org/apache/drill/exec/vector/VariableLengthVectorTest.java b/exec/vector/src/test/java/org/apache/drill/exec/vector/VariableLengthVectorTest.java
index 4c6aeed..c77854f 100644
--- a/exec/vector/src/test/java/org/apache/drill/exec/vector/VariableLengthVectorTest.java
+++ b/exec/vector/src/test/java/org/apache/drill/exec/vector/VariableLengthVectorTest.java
@@ -89,6 +89,30 @@ public class VariableLengthVectorTest
}
}
+ @Test
+ public void testDRILL7341() {
+ try (RootAllocator allocator = new RootAllocator(10_000_000)) {
+ final MaterializedField field = MaterializedField.create("stringCol", Types.optional(TypeProtos.MinorType.VARCHAR));
+ final NullableVarCharVector sourceVector = new NullableVarCharVector(field, allocator);
+ final NullableVarCharVector targetVector = new NullableVarCharVector(field, allocator);
+
+ sourceVector.allocateNew();
+ targetVector.allocateNew();
+
+ try {
+ final NullableVarCharVector.Mutator sourceMutator = sourceVector.getMutator();
+ sourceMutator.setValueCount(sourceVector.getValueCapacity() * 4);
+
+ targetVector.exchange(sourceVector);
+ final NullableVarCharVector.Mutator targetMutator = targetVector.getMutator();
+ targetMutator.setValueCount(targetVector.getValueCapacity() * 2);
+ } finally {
+ sourceVector.clear();
+ targetVector.clear();
+ }
+ }
+ }
+
/**
* Set 10000 values. Then go back and set new values starting at the 1001 the record.
*/