You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Steven Phillips <sp...@maprtech.com> on 2015/01/08 21:35:54 UTC

Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/
-----------------------------------------------------------

Review request for drill and Jacques Nadeau.


Repository: drill-git


Description
-------

When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.


Diffs
-----

  contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
  contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
  exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
  exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
  exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
  exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
  exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
  exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
  exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
  exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 113e883 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java d7cf904 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java 0502f7e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java e8ccd62 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java 5b56f8e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 5deb67f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java fa983aa 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java e0cce8b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java cc3d24c 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 362d806 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java e140c8b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java c60730c 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 15f8a2b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 3ec66ff 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
  exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 

Diff: https://reviews.apache.org/r/29723/diff/


Testing
-------


Thanks,

Steven Phillips

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Chris Westin <cw...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/#review70376
-----------------------------------------------------------



exec/java-exec/src/main/codegen/templates/FixedValueVectors.java
<https://reviews.apache.org/r/29723/#comment115501>

    Why don't you multiply by two once on the next line instead of dividing by two twice three lines down?



exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java
<https://reviews.apache.org/r/29723/#comment115503>

    add
    final Accessor accessor = v.getAccessor();
    and then use that instead of v.getAccessor() throughout this function, including the loop body.



exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java
<https://reviews.apache.org/r/29723/#comment115504>

    getAccessor() once at the top of the function body, and use the result throughout.



exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java
<https://reviews.apache.org/r/29723/#comment115506>

    getMutator() once and reuse the result. h.vector.getAccessor() once outside the loop body.



exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java
<https://reviews.apache.org/r/29723/#comment115509>

    getMutator() once, and reuse. Figure currentOffset + bytes.length once, and reuse.



exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
<https://reviews.apache.org/r/29723/#comment115510>

    Use the other form of for(WorkspaceJVar workspaceJVar :workspaceJVars) and then reference workspaceJVar throughout the loop body to save on bounds checking and dereferencing on every use.



exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
<https://reviews.apache.org/r/29723/#comment115511>

    Can we do some of this before the outermost if? g.getWorkspaceVector() at least, if not also .get(workspaceVar)?



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
<https://reviews.apache.org/r/29723/#comment115516>

    fix spacing



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java
<https://reviews.apache.org/r/29723/#comment115517>

    fix spacing



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java
<https://reviews.apache.org/r/29723/#comment115518>

    partitionValues.getMutator() once outside the loop and reuse.



exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
<https://reviews.apache.org/r/29723/#comment115522>

    It looks like vector doesn't change, so how about getting vector.getMutator() once outside the loop and reusing.


- Chris Westin


On Jan. 22, 2015, 9:40 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29723/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2015, 9:40 p.m.)
> 
> 
> Review request for drill, Hanifi Gunes and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
> The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.
> 
> 
> Diffs
> -----
> 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
>   exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 35faf22 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 2fd5ce1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggregator.java 0f7f394 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java b5cfdca 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java 1ec74bf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java ba980d7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java f370dc7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 539d028 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueCopierTemplate.java c57ec28 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 495e3e2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java 62a5140 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 58eb546 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java 43a3881 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java ae2f779 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 3171d8a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 76f9e2f 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
>   exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 
> 
> Diff: https://reviews.apache.org/r/29723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Hanifi Gunes <hg...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/#review69296
-----------------------------------------------------------

Ship it!


Ship It!

- Hanifi Gunes


On Jan. 22, 2015, 9:40 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29723/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2015, 9:40 p.m.)
> 
> 
> Review request for drill, Hanifi Gunes and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
> The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.
> 
> 
> Diffs
> -----
> 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
>   exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 35faf22 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 2fd5ce1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggregator.java 0f7f394 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java b5cfdca 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java 1ec74bf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java ba980d7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java f370dc7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 539d028 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueCopierTemplate.java c57ec28 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 495e3e2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java 62a5140 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 58eb546 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java 43a3881 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java ae2f779 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 3171d8a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 76f9e2f 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
>   exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 
> 
> Diff: https://reviews.apache.org/r/29723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/
-----------------------------------------------------------

(Updated Jan. 22, 2015, 9:40 p.m.)


Review request for drill, Hanifi Gunes and Jacques Nadeau.


Repository: drill-git


Description
-------

When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.


Diffs (updated)
-----

  contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
  contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
  exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
  exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
  exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
  exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
  exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
  exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
  exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
  exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 35faf22 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 2fd5ce1 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggregator.java 0f7f394 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java b5cfdca 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java 1ec74bf 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java ba980d7 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java f370dc7 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java 539d028 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/PriorityQueueCopierTemplate.java c57ec28 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 495e3e2 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java 62a5140 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 58eb546 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java 43a3881 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java ae2f779 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 3171d8a 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 76f9e2f 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
  exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 

Diff: https://reviews.apache.org/r/29723/diff/


Testing
-------


Thanks,

Steven Phillips

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Hanifi Gunes <hg...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/#review68854
-----------------------------------------------------------



exec/java-exec/src/main/codegen/templates/MapWriters.java
<https://reviews.apache.org/r/29723/#comment113351>

    Removing this will raise IndexOutOfBoundEx at the boundaries unless we make use of #setSafe in Repeated(Map|List)Vector#startNewGroup rather than #set.



exec/java-exec/src/main/codegen/templates/MapWriters.java
<https://reviews.apache.org/r/29723/#comment113353>

    the same once we make sure to #setSafe this block is irrelevant.



exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java
<https://reviews.apache.org/r/29723/#comment113354>

    I would consider that requesting a value sitting at an index beyond capacity(result of #getValueCapacity) as illegal. We should not re-alloc in #get unless there is a specific reason for doing so. If I get this right, my previous comment regarding use of #setSafe in #startNewGroup should make this block redundant any way. So we should remove this.


Initial set of concerns addressing ValueVector changes. Will look at the rest very soon.

- Hanifi Gunes


On Jan. 20, 2015, 2:44 a.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29723/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 2:44 a.m.)
> 
> 
> Review request for drill, Hanifi Gunes and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
> The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.
> 
> 
> Diffs
> -----
> 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
>   exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 113e883 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java d7cf904 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java 0502f7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java e8ccd62 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java 5b56f8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 5deb67f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java fa983aa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java e0cce8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java cc3d24c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 362d806 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java e140c8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java c60730c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 15f8a2b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 3ec66ff 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
>   exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 
> 
> Diff: https://reviews.apache.org/r/29723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/
-----------------------------------------------------------

(Updated Jan. 20, 2015, 2:44 a.m.)


Review request for drill, Hanifi Gunes and Jacques Nadeau.


Repository: drill-git


Description
-------

When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.


Diffs
-----

  contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
  contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
  exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
  exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
  exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
  exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
  exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
  exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
  exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
  exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 113e883 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java d7cf904 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java 0502f7e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java e8ccd62 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java 5b56f8e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 5deb67f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java fa983aa 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java e0cce8b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java cc3d24c 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 362d806 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java e140c8b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java c60730c 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 15f8a2b 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 3ec66ff 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
  exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 

Diff: https://reviews.apache.org/r/29723/diff/


Testing
-------


Thanks,

Steven Phillips

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Steven Phillips <sp...@maprtech.com>.

> On Jan. 19, 2015, 4:56 p.m., Jason Altekruse wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java, line 95
> > <https://reviews.apache.org/r/29723/diff/1/?file=813534#file813534line95>
> >
> >     Do we really want this to default this high? I'd be concerned about wide varlen values, will the automatic reallocation just keep increasing the buffer size to try to allow this many record go through?

I think this value is fine, and automatic reallocation should handle it.


- Steven


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/#review68625
-----------------------------------------------------------


On Jan. 8, 2015, 8:35 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29723/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2015, 8:35 p.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
> The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.
> 
> 
> Diffs
> -----
> 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
>   exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 113e883 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java d7cf904 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java 0502f7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java e8ccd62 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java 5b56f8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 5deb67f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java fa983aa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java e0cce8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java cc3d24c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 362d806 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java e140c8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java c60730c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 15f8a2b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 3ec66ff 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
>   exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 
> 
> Diff: https://reviews.apache.org/r/29723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Steven Phillips <sp...@maprtech.com>.

> On Jan. 19, 2015, 4:56 p.m., Jason Altekruse wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java, line 39
> > <https://reviews.apache.org/r/29723/diff/1/?file=813554#file813554line39>
> >
> >     As in the other cases, this should be encapsualted to allow later refactoring.

I don't think this particular combination is used else where. Not much benefit in encapsulating.


- Steven


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/#review68625
-----------------------------------------------------------


On Jan. 8, 2015, 8:35 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29723/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2015, 8:35 p.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
> The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.
> 
> 
> Diffs
> -----
> 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
>   exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 113e883 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java d7cf904 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java 0502f7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java e8ccd62 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java 5b56f8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 5deb67f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java fa983aa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java e0cce8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java cc3d24c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 362d806 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java e140c8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java c60730c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 15f8a2b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 3ec66ff 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
>   exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 
> 
> Diff: https://reviews.apache.org/r/29723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/#review68625
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
<https://reviews.apache.org/r/29723/#comment112951>

    Same here, do we need this to ensure the hash is positive?



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
<https://reviews.apache.org/r/29723/#comment112954>

    Do we really want this to default this high? I'd be concerned about wide varlen values, will the automatic reallocation just keep increasing the buffer size to try to allow this many record go through?



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java
<https://reviews.apache.org/r/29723/#comment112957>

    While this refactoring is happening it might be worth taking this out of the project operator and trying to look for other places with similar requirements to consolidate the code. I think it would be especially useful here because the code currently uses instanceof and we will likely want to add an interface to the vector eventually for "isFixedWidth".



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java
<https://reviews.apache.org/r/29723/#comment112959>

    remove



exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java
<https://reviews.apache.org/r/29723/#comment112963>

    I think I marked them where they appeared, but there are places inthe patch that have this code duplicated.



exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java
<https://reviews.apache.org/r/29723/#comment112964>

    As in the other cases, this should be encapsualted to allow later refactoring.


- Jason Altekruse


On Jan. 8, 2015, 8:35 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29723/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2015, 8:35 p.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
> The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.
> 
> 
> Diffs
> -----
> 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
>   exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 113e883 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java d7cf904 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java 0502f7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java e8ccd62 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java 5b56f8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 5deb67f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java fa983aa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java e0cce8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java cc3d24c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 362d806 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java e140c8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java c60730c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 15f8a2b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 3ec66ff 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
>   exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 
> 
> Diff: https://reviews.apache.org/r/29723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Steven Phillips <sp...@maprtech.com>.

> On Jan. 17, 2015, 2:38 a.m., Jason Altekruse wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java, line 621
> > <https://reviews.apache.org/r/29723/diff/1/?file=813525#file813525line621>
> >
> >     Doesn't this line ensure we have a positive index into the hash table 'array'?

If you look at the code in getBucketIndex(), it is already guaranteed to be positive.


> On Jan. 17, 2015, 2:38 a.m., Jason Altekruse wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java, line 164
> > <https://reviews.apache.org/r/29723/diff/1/?file=813523#file813523line164>
> >
> >     This is duplicated frequently, I know that it might be a hard to find a centeralized place for this to live, but it would be better to encapsulate it so we can make future changes in a single location.

I will look into this.


- Steven


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/#review68495
-----------------------------------------------------------


On Jan. 8, 2015, 8:35 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29723/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2015, 8:35 p.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
> The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.
> 
> 
> Diffs
> -----
> 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
>   exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 113e883 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java d7cf904 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java 0502f7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java e8ccd62 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java 5b56f8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 5deb67f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java fa983aa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java e0cce8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java cc3d24c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 362d806 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java e140c8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java c60730c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 15f8a2b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 3ec66ff 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
>   exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 
> 
> Diff: https://reviews.apache.org/r/29723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>

Re: Review Request 29723: DRILL-1960: Automatically realloc buffers when vector runs out of space

Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29723/#review68495
-----------------------------------------------------------



contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java
<https://reviews.apache.org/r/29723/#comment112791>

    leftOver is not set anywhere else in this class, should be removed entirely.



exec/java-exec/src/main/codegen/templates/ComplexWriters.java
<https://reviews.apache.org/r/29723/#comment112793>

    We should remove this method rather than leave it here blank.



exec/java-exec/src/main/codegen/templates/MapWriters.java
<https://reviews.apache.org/r/29723/#comment112794>

    delete



exec/java-exec/src/main/codegen/templates/MapWriters.java
<https://reviews.apache.org/r/29723/#comment112761>

    should remove this entire unused block



exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
<https://reviews.apache.org/r/29723/#comment112800>

    I think there should be a comment here explaining why it is commented out. With the number of lines its probably worth deleting all together to avoid clutter. Might be worth leaving a comment here in this case still to explain what was done previously and how it was changed.



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
<https://reviews.apache.org/r/29723/#comment112842>

    This is duplicated frequently, I know that it might be a hard to find a centeralized place for this to live, but it would be better to encapsulate it so we can make future changes in a single location.



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
<https://reviews.apache.org/r/29723/#comment112843>

    Doesn't this line ensure we have a positive index into the hash table 'array'?



exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java
<https://reviews.apache.org/r/29723/#comment112844>

    remove


- Jason Altekruse


On Jan. 8, 2015, 8:35 p.m., Steven Phillips wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29723/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2015, 8:35 p.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> When reaching the end of a buffer, the current way drill handles this is for setSafe() to return false, and then whatever operator is doing the write will send the batch, and the redo the last record. This creates a lot of difficulty, because it sometimes requires being able to "rewind" the input stream to replay the last record.
> The proposal is to move the handling of buffer sizing and allocation into the value vectors themselves, making it transparent to the user of the value vector. The operators will now no longer have to worry about the possibility that writing into a vector may fail due to lack of space.
> 
> 
> Diffs
> -----
> 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java 33bf376 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveFieldConverter.java 82e038c 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveRecordReader.java 625a7b2 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveTextRecordReader.java 5406048 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java 79abe60 
>   exec/java-exec/src/main/codegen/templates/ComplexReaders.java 027f61d 
>   exec/java-exec/src/main/codegen/templates/ComplexWriters.java 5ba1c64 
>   exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 58e6ccc 
>   exec/java-exec/src/main/codegen/templates/MapWriters.java b8bd73e 
>   exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b222024 
>   exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java d261050 
>   exec/java-exec/src/main/codegen/templates/TypeHelper.java c83c301 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b8ffe5d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java 5cf4a35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 6e0b282 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillComplexWriterFuncHolder.java df56174 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java 113e883 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java d7cf904 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ef85a36 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggTemplate.java 556b260 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java 0502f7e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java e8ccd62 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java 5b56f8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java e82dd29 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 5deb67f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbe.java 7599f9e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java c58f9a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java c1dffc1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java d0f9d7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverGeneratorBase.java 2885c52 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingReceiverTemplate.java c29ef75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java acbb755 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionProjectorTemplate.java f5068b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 20f6195 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java fa983aa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectorTemplate.java 49ad390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate2.java 5cc308a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/CopierTemplate4.java c42332d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java a3e7940 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameTemplate.java b4e3fed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/TransferPair.java 9645be9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/FixedWidthRepeatedReader.java 0c4437a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableBitReader.java b9b808b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthColumnReaders.java 83f9bde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/VarLengthValuesColumn.java 7c4b33b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoRecordReader.java 241fa95 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/PojoWriter.java 0ffa55c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/pojo/Writers.java fee011a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java e0cce8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/AllocationHelper.java 51726a3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java d2523c5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/CopyUtil.java 1ed7f37 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedFixedWidthVector.java aadc563 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/RepeatedMutator.java ad2ba1b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/VariableWidthVector.java ff3ee63 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java cc3d24c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedListVector.java 362d806 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/RepeatedMapVector.java e140c8b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedListReaderImpl.java c60730c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/RepeatedMapReaderImpl.java 15f8a2b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/SingleMapReaderImpl.java 3ec66ff 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java 7c04477 
>   exec/java-exec/src/test/java/org/apache/drill/exec/vector/TestAdaptiveAllocation.java ebc4df7 
> 
> Diff: https://reviews.apache.org/r/29723/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Steven Phillips
> 
>