You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by abdelhakim deneche <ad...@gmail.com> on 2015/07/01 21:34:49 UTC
Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/
-----------------------------------------------------------
Review request for drill, Chris Westin and Hanifi Gunes.
Bugs: DRILL-3445
https://issues.apache.org/jira/browse/DRILL-3445
Repository: drill-git
Description
-------
BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
Diffs
-----
exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
Diff: https://reviews.apache.org/r/36103/diff/
Testing
-------
unit tests are passing, cluster test ongoing...
Thanks,
abdelhakim deneche
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
> On July 1, 2015, 9:44 p.m., Hanifi Gunes wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java, line 94
> > <https://reviews.apache.org/r/36103/diff/1/?file=997588#file997588line94>
> >
> > Not a show stopper but I would suggest renaming this to allocateNewSafe, following conventions in the rest of codebase.
>
> Jacques Nadeau wrote:
> I mean mean limit. Best if also suggested what limit we hit (operator, query, system).
>
> For example, we're constrained to 20mb for this operator allocator and we fail to allocate because we're at 10mb and just asked for a 50mb allocation.
>
> abdelhakim deneche wrote:
> I'm still not sure how to find which limit we did hit, at least with the "current" allocator
>
> abdelhakim deneche wrote:
> @hanifi, when I renamed allocateNew -> allocateNewSafe I noticed 2 places where the code doesn't check if the allocation was sucessful or not. Should I fix both places as part of this patch ?
will fix as part of my next patch
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90137
-----------------------------------------------------------
On July 3, 2015, 2:37 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 2:37 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java cd2fbe9
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java d9330ea
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 6da5582
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> testing...
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
> On July 1, 2015, 9:44 p.m., Hanifi Gunes wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java, line 94
> > <https://reviews.apache.org/r/36103/diff/1/?file=997588#file997588line94>
> >
> > Not a show stopper but I would suggest renaming this to allocateNewSafe, following conventions in the rest of codebase.
>
> Jacques Nadeau wrote:
> I mean mean limit. Best if also suggested what limit we hit (operator, query, system).
>
> For example, we're constrained to 20mb for this operator allocator and we fail to allocate because we're at 10mb and just asked for a 50mb allocation.
>
> abdelhakim deneche wrote:
> I'm still not sure how to find which limit we did hit, at least with the "current" allocator
@hanifi, when I renamed allocateNew -> allocateNewSafe I noticed 2 places where the code doesn't check if the allocation was sucessful or not. Should I fix both places as part of this patch ?
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90137
-----------------------------------------------------------
On July 3, 2015, 2:37 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 2:37 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java cd2fbe9
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java d9330ea
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 6da5582
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> testing...
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by Jacques Nadeau <ja...@gmail.com>.
> On July 1, 2015, 9:44 p.m., Hanifi Gunes wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java, line 94
> > <https://reviews.apache.org/r/36103/diff/1/?file=997588#file997588line94>
> >
> > Not a show stopper but I would suggest renaming this to allocateNewSafe, following conventions in the rest of codebase.
I mean mean limit. Best if also suggested what limit we hit (operator, query, system).
For example, we're constrained to 20mb for this operator allocator and we fail to allocate because we're at 10mb and just asked for a 50mb allocation.
- Jacques
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90137
-----------------------------------------------------------
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
> On July 1, 2015, 9:44 p.m., Hanifi Gunes wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java, line 94
> > <https://reviews.apache.org/r/36103/diff/1/?file=997588#file997588line94>
> >
> > Not a show stopper but I would suggest renaming this to allocateNewSafe, following conventions in the rest of codebase.
>
> Jacques Nadeau wrote:
> I mean mean limit. Best if also suggested what limit we hit (operator, query, system).
>
> For example, we're constrained to 20mb for this operator allocator and we fail to allocate because we're at 10mb and just asked for a 50mb allocation.
I'm still not sure how to find which limit we did hit, at least with the "current" allocator
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90137
-----------------------------------------------------------
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by Hanifi Gunes <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90137
-----------------------------------------------------------
Ship it!
Looks good.
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java (line 94)
<https://reviews.apache.org/r/36103/#comment143113>
Not a show stopper but I would suggest renaming this to allocateNewSafe, following conventions in the rest of codebase.
- Hanifi Gunes
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90278
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java (line 96)
<https://reviews.apache.org/r/36103/#comment143281>
We should probably move this exception inside of reserve. Then we can know which accountor. This means that we need to label the allocators when we get them. That being said, let's hold on this until we get Chris's new allocator merged. You want to open a separate bug to add better out of memory exceptions? Maybe for now just add information about what was the allocation size when we failed.
- Jacques Nadeau
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90279
-----------------------------------------------------------
Ship it!
Ship It!
- Jacques Nadeau
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/
-----------------------------------------------------------
(Updated July 3, 2015, 4:13 p.m.)
Review request for drill, Chris Westin and Hanifi Gunes.
Bugs: DRILL-3445
https://issues.apache.org/jira/browse/DRILL-3445
Repository: drill-git
Description
-------
BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
Diffs
-----
exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java cd2fbe9
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 6da5582
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
Diff: https://reviews.apache.org/r/36103/diff/
Testing (updated)
-------
all unit tests are passing along with customer and tpch100
Thanks,
abdelhakim deneche
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/
-----------------------------------------------------------
(Updated July 3, 2015, 3:14 p.m.)
Review request for drill, Chris Westin and Hanifi Gunes.
Bugs: DRILL-3445
https://issues.apache.org/jira/browse/DRILL-3445
Repository: drill-git
Description
-------
BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
Diffs (updated)
-----
exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java cd2fbe9
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 6da5582
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
Diff: https://reviews.apache.org/r/36103/diff/
Testing
-------
testing...
Thanks,
abdelhakim deneche
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
> On July 3, 2015, 2:41 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java, line 97
> > <https://reviews.apache.org/r/36103/diff/2/?file=998942#file998942line97>
> >
> > This creates the same problem I've seen elsewhere where you get two separate log events that have the same meaning. Why wouldn't you simply include this information in the Exception? I'm not seeing the upside here. Given the change in pattern to always throwing an exception, this may not even be an error since we may be just checking to see if we can allocate.
will fix
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90344
-----------------------------------------------------------
On July 3, 2015, 2:37 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 2:37 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java cd2fbe9
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java d9330ea
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 6da5582
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> testing...
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by Charitha Madusankha <ch...@gmail.com>.
P
On Jul 3, 2015 8:11 PM, "Jacques Nadeau" <ja...@gmail.com> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/#review90344
> -----------------------------------------------------------
>
>
>
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java
> (line 97)
> <https://reviews.apache.org/r/36103/#comment143360>
>
> This creates the same problem I've seen elsewhere where you get two
> separate log events that have the same meaning. Why wouldn't you simply
> include this information in the Exception? I'm not seeing the upside
> here. Given the change in pattern to always throwing an exception, this
> may not even be an error since we may be just checking to see if we can
> allocate.
>
>
> - Jacques Nadeau
>
>
> On July 3, 2015, 2:37 p.m., abdelhakim deneche wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/36103/
> > -----------------------------------------------------------
> >
> > (Updated July 3, 2015, 2:37 p.m.)
> >
> >
> > Review request for drill, Chris Westin and Hanifi Gunes.
> >
> >
> > Bugs: DRILL-3445
> > https://issues.apache.org/jira/browse/DRILL-3445
> >
> >
> > Repository: drill-git
> >
> >
> > Description
> > -------
> >
> > BufferAllocator.buffer(int) implementations throw an
> OutOfMemoryRuntimeException instead of returning null.
> >
> >
> > Diffs
> > -----
> >
> > exec/java-exec/src/main/codegen/templates/FixedValueVectors.java
> 7103a17
> > exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java
> 50ae770
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java
> 016cd92
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java
> 811cceb
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java
> b4386a4
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java
> cd2fbe9
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java
> d9330ea
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
> 6da5582
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java
> 7a7c012
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java
> 4e03f11
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java
> cf30db6
> >
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java
> 10bdf07
> >
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java
> 6337d4c
> >
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java
> 051ad4e
> >
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java
> 74ce225
> >
> > Diff: https://reviews.apache.org/r/36103/diff/
> >
> >
> > Testing
> > -------
> >
> > testing...
> >
> >
> > Thanks,
> >
> > abdelhakim deneche
> >
> >
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90344
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java (line 97)
<https://reviews.apache.org/r/36103/#comment143360>
This creates the same problem I've seen elsewhere where you get two separate log events that have the same meaning. Why wouldn't you simply include this information in the Exception? I'm not seeing the upside here. Given the change in pattern to always throwing an exception, this may not even be an error since we may be just checking to see if we can allocate.
- Jacques Nadeau
On July 3, 2015, 2:37 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 3, 2015, 2:37 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java cd2fbe9
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java d9330ea
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 6da5582
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> testing...
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/
-----------------------------------------------------------
(Updated July 3, 2015, 2:37 p.m.)
Review request for drill, Chris Westin and Hanifi Gunes.
Changes
-------
- Top and Child allocators will log current and attempted allocations before throwing an OOMRE
- renamed SelectionVector2.allocateNew -> allocateNewSafe
Bugs: DRILL-3445
https://issues.apache.org/jira/browse/DRILL-3445
Repository: drill-git
Description
-------
BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
Diffs (updated)
-----
exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterTemplate2.java cd2fbe9
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java d9330ea
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java 6da5582
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
Diff: https://reviews.apache.org/r/36103/diff/
Testing (updated)
-------
testing...
Thanks,
abdelhakim deneche
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
> On July 1, 2015, 9:26 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java, line 254
> > <https://reviews.apache.org/r/36103/diff/1/?file=997587#file997587line254>
> >
> > Can we make it standard for all of these locations to report: current allocation, max and attempted allocation
by max do you mean peak memory allocation ?
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90134
-----------------------------------------------------------
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
> On July 1, 2015, 9:26 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java, line 153
> > <https://reviews.apache.org/r/36103/diff/1/?file=997594#file997594line153>
> >
> > Shouldn't we make sure we actually got the exception since we expect it?
if no exception is thrown the unit test will fail, because of the fail("...") inside the try block. Do I need to do something more ?
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90134
-----------------------------------------------------------
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by Jacques Nadeau <ja...@gmail.com>.
> On July 1, 2015, 9:26 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java, line 153
> > <https://reviews.apache.org/r/36103/diff/1/?file=997594#file997594line153>
> >
> > Shouldn't we make sure we actually got the exception since we expect it?
>
> abdelhakim deneche wrote:
> if no exception is thrown the unit test will fail, because of the fail("...") inside the try block. Do I need to do something more ?
good point :)
- Jacques
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90134
-----------------------------------------------------------
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/#review90134
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java (line 247)
<https://reviews.apache.org/r/36103/#comment143110>
Can we make it standard for all of these locations to report: current allocation, max and attempted allocation
exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java
<https://reviews.apache.org/r/36103/#comment143111>
Shouldn't we make sure we actually got the exception since we expect it?
- Jacques Nadeau
On July 1, 2015, 9:21 p.m., abdelhakim deneche wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36103/
> -----------------------------------------------------------
>
> (Updated July 1, 2015, 9:21 p.m.)
>
>
> Review request for drill, Chris Westin and Hanifi Gunes.
>
>
> Bugs: DRILL-3445
> https://issues.apache.org/jira/browse/DRILL-3445
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
> exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
> exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
> exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
> exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
> exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
> exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
> exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
> exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
>
> Diff: https://reviews.apache.org/r/36103/diff/
>
>
> Testing
> -------
>
> unit tests are passing, along with functional and tpch100
>
>
> Thanks,
>
> abdelhakim deneche
>
>
Re: Review Request 36103: DRILL-3445: BufferAllocator.buffer()
implementations should throw an OutOfMemoryRuntimeException
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36103/
-----------------------------------------------------------
(Updated July 1, 2015, 9:21 p.m.)
Review request for drill, Chris Westin and Hanifi Gunes.
Bugs: DRILL-3445
https://issues.apache.org/jira/browse/DRILL-3445
Repository: drill-git
Description
-------
BufferAllocator.buffer(int) implementations throw an OutOfMemoryRuntimeException instead of returning null.
Diffs
-----
exec/java-exec/src/main/codegen/templates/FixedValueVectors.java 7103a17
exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java 50ae770
exec/java-exec/src/main/java/org/apache/drill/exec/cache/VectorAccessibleSerializable.java 016cd92
exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 811cceb
exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java b4386a4
exec/java-exec/src/main/java/org/apache/drill/exec/record/selection/SelectionVector2.java 7a7c012
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java 4e03f11
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetDirectByteBufferAllocator.java cf30db6
exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java 10bdf07
exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 6337d4c
exec/java-exec/src/test/java/org/apache/drill/TestAllocationException.java 051ad4e
exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 74ce225
Diff: https://reviews.apache.org/r/36103/diff/
Testing (updated)
-------
unit tests are passing, along with functional and tpch100
Thanks,
abdelhakim deneche