> On Oct. 27, 2014, 4:17 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/memory/Accountor.java, > > line 359 > > <https://reviews.apache.org/r/27005/diff/1/?file=728351#file728351line359> > > > > isn't this already contained in the same condition on line 325?
There appears to be some concurrency issue. I was running into some cases where buffers was not empty at 325, but then no stacktraces were being printed, because buffers was empty by the time it got to that point. Adding this extra check seemed to fix the problem. > On Oct. 27, 2014, 4:17 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java, > > line 53 > > <https://reviews.apache.org/r/27005/diff/1/?file=728352#file728352line53> > > > > It seems like even if RecordBatches have two methods, RootExec doesn't > > need it since we're still sending a batch. Why separate there, too? > > > > If it needs to exists... > > Since this is an abstract class and the interfaces defines this, why > > not skip it so implementations clearly have to implement? A couple of reasons I designed it this way: 1. PartitionSender does not send a batch every time next() is called. A batch is sent only when an outgoing batch is filled. 2. Looking forward to the fix for DRILL-1370 (staging), the way this first batch is sent would be different than the rest of the batches I can go ahead and remove the Abstract implementation, so implementers will have to implement. > On Oct. 27, 2014, 4:17 a.m., Jacques Nadeau wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java, > > line 54 > > <https://reviews.apache.org/r/27005/diff/1/?file=728378#file728378line54> > > > > ? This was a mistake. I will correct this. - Steven ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27005/#review58582 ----------------------------------------------------------- On Oct. 22, 2014, 12:23 a.m., Steven Phillips wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27005/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2014, 12:23 a.m.) > > > Review request for drill and Aman Sinha. > > > Repository: drill-git > > > Description > ------- > > The basic design is as such: > buildSchema() method is added to RecordBatch interface. This method will call > the same on incoming batches, and use upstream schema info to populate > outgoing VectorContainer, making schema info available to downstream operator. > the buildSchema() method of RootExec operators will also be responsible for > sending empty schema batches downstream. > Fragment executor will call buildSchema() on the RootExec before calling > next(); > > > Diffs > ----- > > exec/java-exec/src/main/java/org/apache/drill/exec/memory/Accountor.java > 67beb95 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java > 412da85 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java > 4250e27 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java > fc23441 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java > bd15ac9 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java > 34196b7 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java > 473e3a3 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java > 8c1a4c0 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java > c522870 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java > 4d3925e > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java > c594e70 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java > 6028a04 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java > 6024523 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java > 85f664c > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java > 2a08c05 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinProbeTemplate.java > 133289e > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java > 1d4e353 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java > 8ffd7be > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java > ed49cf1 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java > 2c3e85a > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java > 338a704 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java > 7f3a966 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java > 224753e > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java > 7178d4c > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java > 4e644df > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java > 6b83d04 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java > 364fc4f > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java > 171d12c > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/StreamingWindowFrameRecordBatch.java > 2a92089 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java > 9c48838 > > exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java > a835bee > > exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java > f05243d > > exec/java-exec/src/main/java/org/apache/drill/exec/record/ExpandableHyperContainer.java > b8c5a8f > > exec/java-exec/src/main/java/org/apache/drill/exec/record/FragmentWritableBatch.java > f0453d9 > exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java > 4189576 > > exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java > b1b7c76 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java > 037f1c7 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java > ecc8df2 > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestDateFunctions.java > 6338e23 > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestMultiInputAdd.java > 9b8070b > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewAggregateFunctions.java > 189af39 > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewMathFunctions.java > 54c1700 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java > f878bcb > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java > 0d9f014 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestDecimal.java > 5357a13 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestImplicitCastFunctions.java > 141c9cd > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestSimpleFragmentRun.java > 68e2112 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestStringFunctions.java > aa3548d > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TopN/TestSimpleTopN.java > ccc052d > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java > f466171 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/orderedpartitioner/TestOrderedPartitionExchange.java > 8419860 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/sort/TestSimpleSort.java > d27ad79 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/window/TestWindowFrame.java > ac7b035 > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestWriter.java > 530883b > > exec/java-exec/src/test/java/org/apache/drill/exec/record/vector/TestDateTypes.java > 46e721d > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java > f450e5d > exec/java-exec/src/test/resources/agg/test1.json 12dab5f > exec/java-exec/src/test/resources/agg/twokey.json 5e33c58 > exec/java-exec/src/test/resources/decimal/test_decimal_sort_complex.json > 1fbe106 > exec/java-exec/src/test/resources/functions/cast/testICastConstant.json > 69e4058 > exec/java-exec/src/test/resources/functions/date/interval_arithmetic.json > 50ae92b > exec/java-exec/src/test/resources/functions/string/testRegexpReplace.json > 014c09b > exec/java-exec/src/test/resources/join/join_batchsize.json 4817e7c > exec/java-exec/src/test/resources/join/mj_multi_condition.json 25b1391 > exec/java-exec/src/test/resources/mergerecv/merging_receiver.json 042052d > exec/java-exec/src/test/resources/mergerecv/multiple_providers.json 0eb8007 > exec/java-exec/src/test/resources/record/vector/test_sort_date.json 1d7d854 > exec/java-exec/src/test/resources/window/oneKeyCount.json d8965fb > exec/java-exec/src/test/resources/window/oneKeyCountMultiBatch.json 069bc1f > exec/java-exec/src/test/resources/window/twoKeys.json 6282ad2 > > Diff: https://reviews.apache.org/r/27005/diff/ > > > Testing > ------- > > > Thanks, > > Steven Phillips > >
