[GitHub] drill issue #905: DRILL-1162: Fix OOM for hash join operator when the right ...

2017-09-09 Thread jinfengni
Github user jinfengni commented on the issue: https://github.com/apache/drill/pull/905 This OOM problem exposes two problems. The first one is in planning time, where we choose a sub-optimal plan, due to the inaccurate estimation of row count because of missing of appropriate statisti

[GitHub] drill issue #934: DRILL-3449 When Foreman node dies, the FragmentExecutor st...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/934 Thanks for the explanation. +1 ---

[GitHub] drill pull request #934: DRILL-3449 When Foreman node dies, the FragmentExec...

2017-09-09 Thread vrozov
Github user vrozov commented on a diff in the pull request: https://github.com/apache/drill/pull/934#discussion_r137941417 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentStatusReporter.java --- @@ -113,4 +120,14 @@ void fail(final UserException

[GitHub] drill issue #926: DRILL-5269 Make DirectSubScan Jackson JSON deserializable

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/926 On the first comment, it sounds like the close is a placeholder, actual close is to be done after some refactoring? This is fine. Just a bit surprising to see a log message, but not actual close.

[GitHub] drill issue #926: DRILL-5269 Make DirectSubScan Jackson JSON deserializable

2017-09-09 Thread vrozov
Github user vrozov commented on the issue: https://github.com/apache/drill/pull/926 @paul-rogers I guess you refer to a system/integration test (execute the query provided in JIRA), not a unit test. ---

[GitHub] drill pull request #934: DRILL-3449 When Foreman node dies, the FragmentExec...

2017-09-09 Thread vrozov
Github user vrozov commented on a diff in the pull request: https://github.com/apache/drill/pull/934#discussion_r137941124 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentStatusReporter.java --- @@ -113,4 +120,14 @@ void fail(final UserException

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939203 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -109,14 +107,21 @@ private

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939168 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java --- @@ -293,7 +299,7 @@ private HashAggregator cre

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939287 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -500,22 +516,45 @@ private void initiali

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939296 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -545,16 +584,19 @@ public AggOutcome doW

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939122 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -92,18 +92,20 @@ // Hash Aggregate Options -

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939184 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -109,14 +107,21 @@ private

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939496 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTable.java --- @@ -58,7 +59,7 @@ public int getHashCod

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939361 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -1335,7 +1470,7 @@ private void updateSt

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939459 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -1178,20 +1273,38 @@ private void checkG

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939319 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -646,6 +687,46 @@ public AggOutcome doWo

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939481 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggregator.java --- @@ -47,10 +47,7 @@ // OK - batch retu

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939223 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -297,10 +302,7 @@ public void outputReco

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939116 --- Diff: common/src/main/java/org/apache/drill/common/exceptions/RetryAfterSpillException.java --- @@ -0,0 +1,32 @@ +/** + * Licensed to the Apa

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939427 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -1178,20 +1273,38 @@ private void checkG

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939336 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -646,6 +687,46 @@ public AggOutcome doWo

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939532 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java --- @@ -158,19 +158,17 @@ public BatchHolder(int

[GitHub] drill pull request #938: DRILL-5694: Handle HashAgg OOM by spill and retry, ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/938#discussion_r137939253 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -382,19 +390,25 @@ private void delayedS

[jira] [Created] (DRILL-5779) HashAgg template is far too large, cause performance hit

2017-09-09 Thread Paul Rogers (JIRA)
Paul Rogers created DRILL-5779: -- Summary: HashAgg template is far too large, cause performance hit Key: DRILL-5779 URL: https://issues.apache.org/jira/browse/DRILL-5779 Project: Apache Drill Iss

[GitHub] drill pull request #937: DRILL-5002: Using hive's date functions on top of d...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/937#discussion_r137938539 --- Diff: contrib/storage-hive/core/src/main/codegen/templates/ObjectInspectorHelper.java --- @@ -204,7 +204,11 @@ public static JBlock getDrillObject(JC

[GitHub] drill pull request #934: DRILL-3449 When Foreman node dies, the FragmentExec...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/934#discussion_r137938397 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentStatusReporter.java --- @@ -113,4 +120,14 @@ void fail(final UserExcep

[GitHub] drill issue #932: DRILL-5758: Fix for repeated columns; enable managed sort ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/932 @Ben-Zvi, added another fix. Please take another look. ---

[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/930#discussion_r137938315 --- Diff: common/src/test/resources/logback-test.xml --- @@ -0,0 +1,111 @@ + + + + + + + +true +

[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/930#discussion_r137938300 --- Diff: common/src/test/resources/logback-test.xml --- @@ -0,0 +1,111 @@ + + + + + + + +true +

[GitHub] drill pull request #930: DRILL-5761: Disable Lilith ClassicMultiplexSocketAp...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/930#discussion_r137938284 --- Diff: common/src/test/resources/logback-test.xml --- @@ -0,0 +1,111 @@ + + + + + + + +true +

[GitHub] drill issue #928: DRILL-5716: Queue-driven memory allocation

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/928 @jinfengni, can you take a quick look at the Foreman changes? Especially the bits that muck about with the physical plan: some of the work is moved from one place to another. Thanks! ---

[GitHub] drill pull request #925: DRILL-5749: solve foreman and netty threads deadloc...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/925#discussion_r137938116 --- Diff: exec/rpc/src/main/java/org/apache/drill/exec/rpc/RequestIdMap.java --- @@ -54,10 +52,14 @@ void channelClosed(Throwable ex) { isOpen.se

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937858 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestMergeJoinAdvanced.java --- @@ -102,10 +103,11 @@ public void testFix2

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937977 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/work/metadata/TestMetadataProvider.java --- @@ -196,9 +200,12 @@ public void tablesWithSyste

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r13793 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java --- @@ -346,17 +347,63 @@ public void deleteAllOptio

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937350 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionMetaData.java --- @@ -0,0 +1,68 @@ +/** + * Licensed to the Apa

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937864 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/limit/TestLimitWithExchanges.java --- @@ -71,7 +72,8 @@ public void testPushLi

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937100 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java --- @@ -17,44 +17,84 @@ */ package org.apac

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937991 --- Diff: exec/java-exec/src/test/java/org/apache/drill/test/RestClientFixture.java --- @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937333 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java --- @@ -17,49 +17,97 @@ */ package org.apache.d

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937210 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/DrillConfigIterator.java --- @@ -58,17 +58,17 @@ public OptionValue next() {

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937938 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java --- @@ -216,79 +216,77 @@ public void injectionOnSpecifi

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937125 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java --- @@ -17,44 +17,84 @@ */ package org.apac

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937945 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java --- @@ -150,66 +150,61 @@ public void pauseOnSpecificBit() {

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937112 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java --- @@ -17,44 +17,84 @@ */ package org.apac

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937347 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionMetaData.java --- @@ -0,0 +1,68 @@ +/** + * Licensed to the Apa

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937392 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java --- @@ -63,32 +88,32 @@ public final Double float_val

[GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/923#discussion_r137937901 --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java --- @@ -56,7 +56,7 @@ public void checkChangedColumn() throws Excepti

[GitHub] drill issue #919: DRILL-5721: Query with only root fragment and no non-root ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/919 @sohami, can you clean up the stray commits and ask @parthchandra to again review the later commits? ---

[GitHub] drill issue #916: DRILL-5377: Five-digit year dates are displayed incorrectl...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/916 Back to my original question. The premise of this bug seems to be that we corrupt Parquet dates and convert perfectly valid 4-digit years into invalid 5-digit years. That is clearly a data corrupt

[GitHub] drill issue #905: DRILL-1162: Fix OOM for hash join operator when the right ...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/905 @jinfengni, have you been able to take a look at this PR? ---

[GitHub] drill issue #903: DRILL-5712: Update the pom files with dependency exclusion...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/903 @sindhurirayavaram, please rebase this onto the latest master; looks like some stray commits have found their way into this PR. ---

[GitHub] drill issue #889: DRILL-5691: enhance scalar sub queries checking for the ca...

2017-09-09 Thread paul-rogers
Github user paul-rogers commented on the issue: https://github.com/apache/drill/pull/889 @arina-ielchiieva, looks like Weijie added a couple of commits since your +1. Can you take a look at them? ---

Re: Checkstyle Unused Imports

2017-09-09 Thread Arina Yelchiyeva
Also I might want to add check style for Apache header (which should be in a form of comment, not Javadoc), agreed code style (like indents etc) and enforced java doc for methods. At least the last two are enforced in Calcite. I used to point to all that stuff during code reviews but if all that wo