[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2049/


--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 06:35:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 5: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Tue, 06 Mar 2018 03:29:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..

IMPALA-6405: Error when string to decimal cast overflows

Before this patch, when there was an error when converting a string to
a decimal, a NULL was returned. In this patch, we change this behavior
so that an error is returned if decimal_v2 is enabled. We also add a
warning if there is an underflow.

The reasoning is that we want stricter behavior in decimal_v2.

Testing:
- Added some EE tests.
- Ran an exhaustive build, which passed.

Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Reviewed-on: http://gerrit.cloudera.org:8080/9339
Reviewed-by: Dan Hecht 
Tested-by: Impala Public Jenkins
---
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/expr-test.cc
M fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M tests/query_test/test_decimal_casting.py
5 files changed, 57 insertions(+), 15 deletions(-)

Approvals:
  Dan Hecht: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 6
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/9300 )

Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..

IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

Dependency changes:
- BE and python use thrift 0.9.3-p3 from native-toolchain.
- FE uses thrift 0.9.3 from apache maven repo.
- Fb303 and http components dependencies are no longer needed in FE and
  are removed.
- The minimum openssl version requirement is increased to 1.0.1.

Configuration change:
- Thrift codegen option movable_type is enabled. New code no longer
  needs to use std::swap to avoid copying.

Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
---
M CMakeLists.txt
M be/src/common/init.cc
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/rpc/thrift-thread.h
M be/src/rpc/thrift-util.cc
M bin/impala-config.sh
M buildall.sh
M common/thrift/CMakeLists.txt
M fe/pom.xml
M infra/python/deps/compiled-requirements.txt
14 files changed, 78 insertions(+), 174 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/9300/8
--
To view, visit http://gerrit.cloudera.org:8080/9300
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 8
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9300 )

Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.h
File be/src/rpc/thrift-server.h:

http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.h@323
PS7, Line 323: TLSv1_0
> To not change behavior, let's use the equivalent of TLSv1_0_plus from our o
Done


http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc
File be/src/rpc/thrift-server.cc:

http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc@77
PS7, Line 77: {"latest", LATEST}}
> Where is this specified? Or is this a new option that we're trying to expos
Removed. Thrift added this enum and I was not sure whether it should be here. 
Should we change the definition of ssl_minimum_version, because, I assume, 
there isn't xxx_plus anymore?


http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc@97
PS7, Line 97: return protocol != SSLv3 && protocol == TLSv1_0;
> return protocol == SSLTLS || protocol == TLSv1_0;
Done


http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc@99
PS7, Line 99: return protocol != SSLv3 && protocol != TLSv1_2;
> return protocol == SSLTLS || protocol == TLSv1_1 && protocol != TLSv1_2;
Shouldn't this be protocol == SSLTLS || protocol == TLSv1_0 || protocol != 
TLSv1_1?



--
To view, visit http://gerrit.cloudera.org:8080/9300
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 06 Mar 2018 02:42:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 4: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2047/


--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 02:10:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5237: Support a quoted string in date/time format

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8508 )

Change subject: IMPALA-5237: Support a quoted string in date/time format
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8508/9/be/src/runtime/timestamp-parse-util.cc
File be/src/runtime/timestamp-parse-util.cc:

http://gerrit.cloudera.org:8080/#/c/8508/9/be/src/runtime/timestamp-parse-util.cc@175
PS9, Line 175:   // quote due to the compatibility with Hive's behavior.
> Why do we display a single quote in the middle of the string if we have to 
> split "'foo''bar'" into 'foo' and 'bar'?

The reason this PS and the previous produced the correct result is that they 
split foo''bar into two tokens foo' and bar.

> Could you please explain why we cannot convert into a single token without 
> creating a new string and copying the data? If we need a pair of alloc and 
> memcpy for new tokens, this is not related to the number of broken tokens.

Suppose you want to take the input string foo''bar and convert the '' escape 
sequence to '. If you want to produce a single output token, you need it to be 
foo'bar, which which you can't represent as a pointer into the original string 
+ a length.



--
To view, visit http://gerrit.cloudera.org:8080/8508
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie34055ac695748bcfb110bfa6ed5308f469ea178
Gerrit-Change-Number: 8508
Gerrit-PatchSet: 9
Gerrit-Owner: Kim Jin Chul 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Kim Jin Chul 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 01:43:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9497 )

Change subject: IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh
..

IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh

Current fe_support.cc:[..]_FeSupport_NativeLibCacheSetNeedsRefresh
always returns false. In the frontend, this is logged, which causes
unneeded, incorrect, and confusing spam.

This method returns false if unable to manage the string input
argument (path). It then invokes the lib_cache's SetNeedsRefresh
which always succeeds (either the path does not exist or, if it exists,
needs refresh is set). This change modifies the return value after
this call to be true instead of false.

Testing:
- verified the spam without the change by looking at the logs from
  query_test/test_udfs.py (~4000 log messages)
- verified that none of these log messages show up with the change
  applied.

Change-Id: I11f34a63a25f5ab6acabcc2f52b7e8f22d8a4da3
Reviewed-on: http://gerrit.cloudera.org:8080/9497
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M be/src/service/fe-support.cc
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/9497
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I11f34a63a25f5ab6acabcc2f52b7e8f22d8a4da3
Gerrit-Change-Number: 9497
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9497 )

Change subject: IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh
..


Patch Set 1: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/9497
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11f34a63a25f5ab6acabcc2f52b7e8f22d8a4da3
Gerrit-Change-Number: 9497
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Tue, 06 Mar 2018 00:53:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9389 )

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..


Patch Set 6: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 00:47:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9389 )

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..

IMPALA-6553: [DOCS] load_catalog_in_background default change

Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Reviewed-on: http://gerrit.cloudera.org:8080/9389
Reviewed-by: John Russell 
Tested-by: Impala Public Jenkins
---
M docs/shared/impala_common.xml
M docs/topics/impala_invalidate_metadata.xml
2 files changed, 38 insertions(+), 10 deletions(-)

Approvals:
  John Russell: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 7
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9389 )

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/205/


--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 06 Mar 2018 00:37:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 5: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:46:03 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL

[Thomas Tauber-Marshall (Code Review)]

Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9484 )

Change subject: IMPALA-6551: Change Kudu TPCDS and TPCH columns to DECIMAL
..


Patch Set 1:

> I am not 100% sure how to best verify the changes. It looks like
 > the create-load-data.sh script (used by buildall.sh) also loads
 > tpc-h for Kudu but not tpc-ds. It also looks like some kudu create
 > statements are in the standard tpch_schema_template.sql file, but
 > the intention going forward seams to be to used the kudu specific
 > templates for both tpc-h and tpc-ds (according to TODO notes in
 > those files).
 >
 > I tested tpc-h when using buildall.sh to load data and also when
 > using load-tpc-kudu.py to load data.
 >
 > Here is a sample verification script I ran just derived from
 > experimenting:
 > ```
 > # Load Test Data
 > export JAVA_HOME=/usr/lib/jvm/default-java
 > source bin/impala-config.sh
 > ./buildall.sh -ninja -skiptests -noclean -format -testdata
 > # Load & Run Kudu TCP-DS
 > ./testdata/bin/load-tpc-kudu.py --clean -s tpch -t tpch_kudu -w
 > tpch --kudu_master localhost
 > rm /tmp/None_query_runtime_info.json
 > rm -rf /tmp/profiles/
 > rm -rf /tmp/result_hashes/
 > ./tests/stress/concurrent_select.py --minicluster-num-impalads 3
 > --max-queries=1000 --tpch-kudu-db=tpch_kudu --cancel-current-queries
 > --timeout-multiplier 1000
 > # Load & Run Kudu TCP-DS
 > ./testdata/bin/load-tpc-kudu.py --clean -s tpcds -t tpcds_kudu -w
 > tpcds --kudu_master localhost
 > rm /tmp/None_query_runtime_info.json
 > rm -rf /tmp/profiles/
 > rm -rf /tmp/result_hashes/
 > ./tests/stress/concurrent_select.py --minicluster-num-impalads 3
 > --max-queries=1000 --tpcds-kudu-db=tpcds_kudu --cancel-current-queries
 > --timeout-multiplier 1000
 > ```

That seems like a pretty reasonable way of verifying the changes.

I'm not sure of the historical reasons that buildall.sh only loads tpch on Kudu 
and relies on the Kudu-specific CREATEs in tpch_schema_template while the 
stress test supports both tpch and tpcds on Kudu and relies on 
tpc*_kudu_template (looks like Dimitris wrote the tpc*_kudu_templates in the 
first place, so maybe he remembers), but it seems outside the scope of this 
patch to change any of that.

One thing that would be interesting to see is a performance comparison of the 
old version vs. the new. I assume that it doesn't make much difference, but it 
would be good to know.

You can run the perf tests with something like:
./bin/run-workload.py -w tpch --table_formats kudu/none
which will create a json results file that can be passed to 
./tests/benchmark/report_benchmark_results.py to create a nicely formatted 
comparison between runs.


--
To view, visit http://gerrit.cloudera.org:8080/9484
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2f7e4464dc6705cadd610a82c459390a9c0dfe4f
Gerrit-Change-Number: 9484
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:39:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..

IMPALA-6405: Error when string to decimal cast overflows

Before this patch, when there was an error when converting a string to
a decimal, a NULL was returned. In this patch, we change this behavior
so that an error is returned if decimal_v2 is enabled. We also add a
warning if there is an underflow.

The reasoning is that we want stricter behavior in decimal_v2.

Testing:
- Added some EE tests.
- Ran an exhaustive build, which passed.

Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
---
M be/src/exprs/decimal-operators-ir.cc
M be/src/exprs/expr-test.cc
M fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java
M testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
M tests/query_test/test_decimal_casting.py
5 files changed, 57 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/9339/5
--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 5
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc@584
PS2, Line 584: ctx->AddWarning("String to Decimal cast underflowed");
> Right, but given we've made an explicit choice to round, it seems to me tha
Postgres does not issue any warnings. I think then it would make sense to not 
issue the warning for both decimal v1 and v2. Updated the code.



--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:31:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9300 )

Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.h
File be/src/rpc/thrift-server.h:

http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.h@323
PS7, Line 323: TLSv1_0
To not change behavior, let's use the equivalent of TLSv1_0_plus from our old 
toolchain. In Thrift-0.9.3, that's SSLTLS.

https://github.com/apache/thrift/blob/0.9.3/lib/cpp/src/thrift/transport/TSSLSocket.cpp#L140


http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc
File be/src/rpc/thrift-server.cc:

http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc@77
PS7, Line 77: {"latest", LATEST}}
Where is this specified? Or is this a new option that we're trying to expose?


http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc@97
PS7, Line 97: return protocol != SSLv3 && protocol == TLSv1_0;
return protocol == SSLTLS || protocol == TLSv1_0;


http://gerrit.cloudera.org:8080/#/c/9300/7/be/src/rpc/thrift-server.cc@99
PS7, Line 99: return protocol != SSLv3 && protocol != TLSv1_2;
return protocol == SSLTLS || protocol == TLSv1_1 && protocol != TLSv1_2;



--
To view, visit http://gerrit.cloudera.org:8080/9300
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:22:24 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc@584
PS2, Line 584: ctx->AddWarning("String to Decimal cast underflowed");
> IMPALA-5014: Part 1 changes the behavior so that when then there is an unde
Right, but given we've made an explicit choice to round, it seems to me that 
warning may be unwarranted.

Could you see what happens in the case of e.g. postgres?



--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:16:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc@584
PS2, Line 584: ctx->AddWarning("String to Decimal cast underflowed");
> Doesn't that mean that the comment for IMPALA-5014: Part 1 isn't doing what
IMPALA-5014: Part 1 changes the behavior so that when then there is an 
underflow, we round instead of truncating when decimal_v2 is enabled. So that 
patch is correct. Before this patch, underflows happened silently. After this 
patch we will issue a warning.

"Underflow" means that not all digits to the right of the dot in the string 
could fit into the resulting decimal. "Overflow" means that not all digits to 
the left of the dot could fit into the resulting decimal.



--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:08:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc@584
PS2, Line 584: ctx->AddWarning("String to Decimal cast underflowed");
> Doesn't that mean that the comment for IMPALA-5014: Part 1 isn't doing what
Also, regarding the warning, it seems legit to have cases where you have 
strings that have a bunch of fractional digits but you only care about the 
significant digit for whatever calculation you are doing.

What do other systems do in this case?



--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:03:01 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc@584
PS2, Line 584: ctx->AddWarning("String to Decimal cast underflowed");
> No, this does not happen only when decimal_v2 is false. (Take a look at the
Doesn't that mean that the comment for IMPALA-5014: Part 1 isn't doing what it 
claims to have done? If we do rounding, then what does it mean to underflow?



--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Mar 2018 23:00:38 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2047/


--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 22:37:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Tim Armstrong (Code Review)]

Hello Michael Ho, Alex Behm, Dan Hecht, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9493

to look at the new patch set (#4).

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..

IMPALA-6595: fix crash in NljBuilder::Close()

The bug is that the right child of a blocking join node could be closed
before the builder if an error was encountered when sending a batch to
the sink. This hits a DCHECK because Buffers owned by the sink may still
be accounted against the child node.

Testing:
Added the test that originally triggered the problem. It reproduced the
failure when based on the IMPALA-4835 patch, but I can't reproduce
the failure after rebase onto master.

Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
---
M be/src/exec/blocking-join-node.cc
M be/src/exec/data-sink.h
M be/src/exec/exec-node.h
M be/src/exec/nested-loop-join-node.cc
M 
testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test
5 files changed, 24 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/9493/4
--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 3: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2046/


--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 22:36:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9493/2/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test
File 
testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test:

http://gerrit.cloudera.org:8080/#/c/9493/2/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test@28
PS2, Line 28: CATCH
>  CATCH
Done



--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 22:37:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 3: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 22:19:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2046/


--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 22:19:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 22:01:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc
File be/src/exec/blocking-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc@165
PS1, Line 165: if (!status->ok()) build_sink->Close(state);
> If we moved the code out of the build thread, that would mean that the clea
I don't remember the details beyond what is written in JIRA comments, but it 
does seem like we can't require the probe side Open() to complete before doing 
the build side Close().



--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:59:20 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6542: Fix inconsistent write path of Parquet min/max statistics

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9381 )

Change subject: IMPALA-6542: Fix inconsistent write path of Parquet min/max 
statistics
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9381/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9381/5//COMMIT_MSG@12
PS5, Line 12: ignores
If two NaN's are inserted, it wouldn't ignore them but write them into the 
stats, no? Is this desired?


http://gerrit.cloudera.org:8080/#/c/9381/5/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
File testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test:

http://gerrit.cloudera.org:8080/#/c/9381/5/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@580
PS5, Line 580: 
Can you add a test that inserts multiple NaNs, with and without following them 
with a non-NaN?



--
To view, visit http://gerrit.cloudera.org:8080/9381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3957806948f7c661af4be5495f2ec92d1e9fc9d6
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:55:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3

[Tianyi Wang (Code Review)]

Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9300 )

Change subject: IMPALA-5690: Part 2: Upgrade thrift to 0.9.3-p3
..


Patch Set 7:

> Patch Set 6:
>
> (1 comment)

Thanks. It seems to be a circular pipe created by CLONE_FILES that caused the 
hang, not waiting for a SIGPIPE. It should be fine even if the pipe is not 
manually closed in that breakpad patch. Ignoring SIGPIPE should be OK in impala.


--
To view, visit http://gerrit.cloudera.org:8080/9300
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I639227721502eaa10398d9490ff6ac63aa71b3a6
Gerrit-Change-Number: 9300
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:50:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 2: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc
File be/src/exec/blocking-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc@165
PS1, Line 165: if (!status->ok()) build_sink->Close(state);
> If we moved the code out of the build thread, that would mean that the clea
Good points. I agree it's better this way.


http://gerrit.cloudera.org:8080/#/c/9493/2/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test
File 
testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test:

http://gerrit.cloudera.org:8080/#/c/9493/2/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test@28
PS2, Line 28: CATCH
 CATCH



--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:49:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9389 )

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..


Patch Set 5:

> Uploaded patch set 6.

Removed extra spaces.


--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:38:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[Alex Rodoni (Code Review)]

Hello John Russell, Balazs Jeszenszky, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9389

to look at the new patch set (#6).

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..

IMPALA-6553: [DOCS] load_catalog_in_background default change

Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
---
M docs/shared/impala_common.xml
M docs/topics/impala_invalidate_metadata.xml
2 files changed, 38 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9389/6
--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6394: Restart HDFS when blocks are under replicated

[Lars Volker (Code Review)]

Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9469 )

Change subject: IMPALA-6394: Restart HDFS when blocks are under replicated
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9469/2/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:

http://gerrit.cloudera.org:8080/#/c/9469/2/testdata/bin/create-load-data.sh@460
PS2, Line 460: return
> I changed it to a for loop and renamed FAIL_COUNT into RESTART_COUNT. I did
You could also call fsck once before entering the loop. I don't feel strongly 
about it though.



--
To view, visit http://gerrit.cloudera.org:8080/9469
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iefd4c2fc6c287f054e385de52bdc42b0bdbd7915
Gerrit-Change-Number: 9469
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:37:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Removed the obsolete Llama options files

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9219 )

Change subject: [DOCS] Removed the obsolete Llama options files
..


Patch Set 5: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/9219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:31:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Removed the obsolete Llama options files

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9219 )

Change subject: [DOCS] Removed the obsolete Llama options files
..

[DOCS] Removed the obsolete Llama options files

Removed the Llama options file.
Removed impala_sqlref.ditamap that is not used.
Removed the reference to impala_sqlref.ditamap in README.md

Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Cherry-picks: not for 2.x
Reviewed-on: http://gerrit.cloudera.org:8080/9219
Reviewed-by: John Russell 
Tested-by: Impala Public Jenkins
---
M docs/README.md
M docs/impala.ditamap
M docs/impala_keydefs.ditamap
D docs/impala_sqlref.ditamap
M docs/shared/impala_common.xml
D docs/topics/impala_reservation_request_timeout.xml
D docs/topics/impala_v_cpu_cores.xml
7 files changed, 0 insertions(+), 263 deletions(-)

Approvals:
  John Russell: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/9219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9389 )

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..


Patch Set 5:

> (4 comments)
 >
 > A couple of comments inline, plus one for another file that I'm not
 > sure how to mark in the source code:
 >
 > In impala_invalidate_metadata.xml, there's another reference to
 > load_catalog_in_background:
 >
 > table. (This checking does not apply if you have set the
 > catalogd configuration option
 > --load_catalog_in_background=false.) Impala
 > reports any lack of write permissions as an
 >
 > Since the user no longer needs to explicitly set a value of false,
 > perhaps soften this wording a bit and say it doesn't apply when the
 > setting is false, which it is by default.

DONE


--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:29:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Tim Armstrong (Code Review)]

Hello Michael Ho, Alex Behm, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9493

to look at the new patch set (#2).

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..

IMPALA-6595: fix crash in NljBuilder::Close()

The bug is that the right child of a blocking join node could be closed
before the builder if an error was encountered when sending a batch to
the sink. This hits a DCHECK because Buffers owned by the sink may still
be accounted against the child node.

Testing:
Added the test that originally triggered the problem. It reproduced the
failure when based on the IMPALA-4835 patch, but I can't reproduce
the failure after rebase onto master.

Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
---
M be/src/exec/blocking-join-node.cc
M be/src/exec/data-sink.h
M be/src/exec/exec-node.h
M be/src/exec/nested-loop-join-node.cc
M 
testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test
5 files changed, 23 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/9493/2
--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9493/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9493/1//COMMIT_MSG@11
PS1, Line 11: DCHECK
> which DCHECK do we hit? I can't tell from the JIRA information. I'm wonderi
Commented on the JIRA with the DCHECK.

Added a DCHECK in NestedLoopJoin::Close() that detects it.


http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc
File be/src/exec/blocking-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc@165
PS1, Line 165: if (!status->ok()) build_sink->Close(state);
> Fix looks ok.
If we moved the code out of the build thread, that would mean that the cleanup 
only happens after the probe side is opened, which could delay the closing and 
release of resources an arbitrarily long time, which I don't think is worth the 
simplification.

The !status->ok() bit looks like it came from IMPALA-1863. That JIRA refers to 
some kind of distributed deadlock. From the comments it sounds like it's 
necessary to do the Close() on the build thread. Maybe Dan remembers the 
details.


http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc@172
PS1, Line 172:   state->resource_pool()->ReleaseThreadToken(false);
> If the status is non-OK do we release the thread token twice? In ProcessBui
ProcessBuildInputAndOpenProbe() only releases the thread token when thread 
creation fails - it was added in "IMPALA-5750: Catch exceptions from boost 
thread creation"



--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:29:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[Alex Rodoni (Code Review)]

Hello John Russell, Balazs Jeszenszky, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9389

to look at the new patch set (#5).

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..

IMPALA-6553: [DOCS] load_catalog_in_background default change

Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
---
M docs/shared/impala_common.xml
M docs/topics/impala_invalidate_metadata.xml
2 files changed, 38 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9389/5
--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9389 )

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml@3448
PS4, Line 3448:  id="ul_h2v_pjf_ycb"
> Remove this unneeded id= attribute. There is an Oxygen setting that control
Done


http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml@3450
PS4, Line 3450: will be
> General style tip: avoid future tense where possible. In this case, say the
Done


http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml@3458
PS4, Line 3458: will aim to
> Remove future tense again, plus "aim to" is a little bit colloquial. Either
Done


http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml@3463
PS4, Line 3463:  id="ul_fx4_hlf_ycb"
> Another unnecessary id= to remove. The presence of IDs suggests that these
Done



--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:19:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Removed the obsolete Llama options files

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9219 )

Change subject: [DOCS] Removed the obsolete Llama options files
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/204/


--
To view, visit http://gerrit.cloudera.org:8080/9219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:13:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Removed the obsolete Llama options files

[John Russell (Code Review)]

John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9219 )

Change subject: [DOCS] Removed the obsolete Llama options files
..


Patch Set 5: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:13:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9497 )

Change subject: IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh
..


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2045/


--
To view, visit http://gerrit.cloudera.org:8080/9497
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11f34a63a25f5ab6acabcc2f52b7e8f22d8a4da3
Gerrit-Change-Number: 9497
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:12:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9497 )

Change subject: IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9497
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11f34a63a25f5ab6acabcc2f52b7e8f22d8a4da3
Gerrit-Change-Number: 9497
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Comment-Date: Mon, 05 Mar 2018 21:11:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6515: [DOCS] HAproxy with sticky session requires the check option

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9293 )

Change subject: IMPALA-6515: [DOCS] HAproxy with sticky session requires the 
check option
..


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9293/4/docs/topics/impala_proxy.xml
File docs/topics/impala_proxy.xml:

http://gerrit.cloudera.org:8080/#/c/9293/4/docs/topics/impala_proxy.xml@489
PS4, Line 489: attention
> type="important" is the value we typically use for calling any extra attent
Done


http://gerrit.cloudera.org:8080/#/c/9293/4/docs/topics/impala_proxy.xml@493
PS4, Line 493: the Impalad server
> The way this is typically worded elsewhere is "the impaladhttp://gerrit.cloudera.org:8080/#/c/9293/4/docs/topics/impala_proxy.xml@493
PS4, Line 493: that Hue tries to connect
> Grammar: "to which Hue tries to connect".
Done



--
To view, visit http://gerrit.cloudera.org:8080/9293
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I47165df6624f958c2e0542e2627d4f5377789ab8
Gerrit-Change-Number: 9293
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Mar 2018 20:54:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6515: [DOCS] HAproxy with sticky session requires the check option

[Alex Rodoni (Code Review)]

Hello John Russell, Alan Choi, Sailesh Mukil,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9293

to look at the new patch set (#5).

Change subject: IMPALA-6515: [DOCS] HAproxy with sticky session requires the 
check option
..

IMPALA-6515: [DOCS] HAproxy with sticky session requires the check option

Change-Id: I47165df6624f958c2e0542e2627d4f5377789ab8
---
M docs/topics/impala_proxy.xml
1 file changed, 11 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/9293/5
--
To view, visit http://gerrit.cloudera.org:8080/9293
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I47165df6624f958c2e0542e2627d4f5377789ab8
Gerrit-Change-Number: 9293
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9497


Change subject: IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh
..

IMPALA-6599: fixes return for NativeLibCacheSetNeedsRefresh

Current fe_support.cc:[..]_FeSupport_NativeLibCacheSetNeedsRefresh
always returns false. In the frontend, this is logged, which causes
unneeded, incorrect, and confusing spam.

This method returns false if unable to manage the string input
argument (path). It then invokes the lib_cache's SetNeedsRefresh
which always succeeds (either the path does not exist or, if it exists,
needs refresh is set). This change modifies the return value after
this call to be true instead of false.

Testing:
- verified the spam without the change by looking at the logs from
  query_test/test_udfs.py (~4000 log messages)
- verified that none of these log messages show up with the change
  applied.

Change-Id: I11f34a63a25f5ab6acabcc2f52b7e8f22d8a4da3
---
M be/src/service/fe-support.cc
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/9497/1
--
To view, visit http://gerrit.cloudera.org:8080/9497
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I11f34a63a25f5ab6acabcc2f52b7e8f22d8a4da3
Gerrit-Change-Number: 9497
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-6230, IMPALA-6468: Fix the output type of round() and related fns

[Zach Amsden (Code Review)]

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9346 )

Change subject: IMPALA-6230, IMPALA-6468: Fix the output type of round() and 
related fns
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9346/1/be/src/exprs/math-functions-ir.cc
File be/src/exprs/math-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9346/1/be/src/exprs/math-functions-ir.cc@120
PS1, Line 120:   return DoubleVal(trunc(
> Do we have a test that shows the better behavior for trunc() versus the pre
pow(10.0, scale.val) is likely expensive to compute and it isn't clear that the 
compiler will know the function is "pure".  Should we instead do a table lookup 
and bounds check to make sure the scale parameter is sane?


http://gerrit.cloudera.org:8080/#/c/9346/1/be/src/exprs/math-functions-ir.cc@121
PS1, Line 121:   v.val * pow(10.0, scale.val) + ((v.val < 0) ? -0.5 : 0.5)) 
/ pow(10.0, scale.val));
> Should we also check for overflows here , set FunctionContext with an error
Can you undo the code movement here so that the change is more clear?


http://gerrit.cloudera.org:8080/#/c/9346/1/be/src/exprs/math-functions.h
File be/src/exprs/math-functions.h:

http://gerrit.cloudera.org:8080/#/c/9346/1/be/src/exprs/math-functions.h@80
PS1, Line 80:   static DoubleVal RoundUpTo(FunctionContext*, const DoubleVal&, 
const BigIntVal&);
Why the ordering change?  Also, why allow BigIntVal range here, isn't that just 
asking for trouble with overflow?



--
To view, visit http://gerrit.cloudera.org:8080/9346
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77541678012edab70b182378b11ca8753be53f97
Gerrit-Change-Number: 9346
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Mon, 05 Mar 2018 20:16:54 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6394: Restart HDFS when blocks are under replicated

[Tianyi Wang (Code Review)]

Tianyi Wang has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/9469 )

Change subject: IMPALA-6394: Restart HDFS when blocks are under replicated
..

IMPALA-6394: Restart HDFS when blocks are under replicated

HDFS sometimes fails to fully replicate all the blocks in 30 seconds
and no progress is made. This patch tries to restart HDFS several times
before aborting the data loading.

Change-Id: Iefd4c2fc6c287f054e385de52bdc42b0bdbd7915
---
M testdata/bin/create-load-data.sh
1 file changed, 12 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/9469/3
--
To view, visit http://gerrit.cloudera.org:8080/9469
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iefd4c2fc6c287f054e385de52bdc42b0bdbd7915
Gerrit-Change-Number: 9469
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tianyi Wang 

[Impala-ASF-CR] [DOCS] Removed the obsolete Llama options files

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9219 )

Change subject: [DOCS] Removed the obsolete Llama options files
..


Patch Set 5:

> There are still  tags referencing the removed files, which
 > causes problems in the DITA-OT builds. If you remove these 2 lines,
 > the HTML and PDF will build cleanly:
 >
 > $ grep impala_reservation_request_timeout.xml ../*.ditamap
 > ../impala_keydefs.ditamap:   href="topics/impala_reservation_request_timeout.xml"
 > keys="reservation_request_timeout"/>
 > $ grep impala_v_cpu_cores.xml ../*.ditamap
 > ../impala_keydefs.ditamap:   keys="v_cpu_cores"/>

DONE


--
To view, visit http://gerrit.cloudera.org:8080/9219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 20:15:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Removed the obsolete Llama options files

[Alex Rodoni (Code Review)]

Hello Michael Brown, John Russell, Tim Armstrong, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9219

to look at the new patch set (#5).

Change subject: [DOCS] Removed the obsolete Llama options files
..

[DOCS] Removed the obsolete Llama options files

Removed the Llama options file.
Removed impala_sqlref.ditamap that is not used.
Removed the reference to impala_sqlref.ditamap in README.md

Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Cherry-picks: not for 2.x
---
M docs/README.md
M docs/impala.ditamap
M docs/impala_keydefs.ditamap
D docs/impala_sqlref.ditamap
M docs/shared/impala_common.xml
D docs/topics/impala_reservation_request_timeout.xml
D docs/topics/impala_v_cpu_cores.xml
7 files changed, 0 insertions(+), 263 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/9219/5
--
To view, visit http://gerrit.cloudera.org:8080/9219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6553: [DOCS] load catalog in background default change

[John Russell (Code Review)]

John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9389 )

Change subject: IMPALA-6553: [DOCS] load_catalog_in_background default change
..


Patch Set 4:

(4 comments)

A couple of comments inline, plus one for another file that I'm not sure how to 
mark in the source code:

In impala_invalidate_metadata.xml, there's another reference to 
load_catalog_in_background:

  table. (This checking does not apply if you have set the 
catalogd configuration option
  --load_catalog_in_background=false.) Impala reports any 
lack of write permissions as an

Since the user no longer needs to explicitly set a value of false, perhaps 
soften this wording a bit and say it doesn't apply when the setting is false, 
which it is by default.

http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml@3448
PS4, Line 3448:  id="ul_h2v_pjf_ycb"
Remove this unneeded id= attribute. There is an Oxygen setting that controls 
which elements automatically get id= added - please make to remove  and 
 from that setting on your machine.


http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml@3450
PS4, Line 3450: will be
General style tip: avoid future tense where possible. In this case, say the 
metadata _is_ loaded when...


http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml@3458
PS4, Line 3458: will aim to
Remove future tense again, plus "aim to" is a little bit colloquial. Either 
"the catalog service loads metadata..." if it always happens, or "the catalog 
service attemps to load metadata..." if it might or might not happen.


http://gerrit.cloudera.org:8080/#/c/9389/4/docs/shared/impala_common.xml@3463
PS4, Line 3463:  id="ul_fx4_hlf_ycb"
Another unnecessary id= to remove. The presence of IDs suggests that these 
specific lists might be conref'ed someplace else, but actually only the 
enclosing paragraph is conref'ed. Therefore only that paragraph needs an ID. 
(If someone in future wants to remove these lists or change them into some 
different kind of element, it's helpful to know, based on the lack of id= 
attribute, that there are no conrefs pointing to the  and  elements.)



--
To view, visit http://gerrit.cloudera.org:8080/9389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I548b2d1532c12f8d3c795a940b7f980482ecf09b
Gerrit-Change-Number: 9389
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Balazs Jeszenszky 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 20:01:53 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc
File be/src/exec/blocking-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc@172
PS1, Line 172:   state->resource_pool()->ReleaseThreadToken(false);
If the status is non-OK do we release the thread token twice? In 
ProcessBuildInputAndOpenProbe() we also release a thread token if status is 
non-OK.



--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:51:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc
File be/src/exprs/decimal-operators-ir.cc:

http://gerrit.cloudera.org:8080/#/c/9339/2/be/src/exprs/decimal-operators-ir.cc@584
PS2, Line 584: ctx->AddWarning("String to Decimal cast underflowed");
I guess this case can only happen with DECIMAL_V2=false?  Because of:
IMPALA-5014: Part 1: Round when casting string to decimal.
Maybe clarify with DCHECK(!decimal_v2);
This warning might be very loud. Are we sure it won't occur too frequently?


http://gerrit.cloudera.org:8080/#/c/9339/2/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test
File testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test:

http://gerrit.cloudera.org:8080/#/c/9339/2/testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test@459
PS2, Line 459: decimal_v2=0;
nit: existing cases use decimal_v2=false/true. Would be good to be consistent 
to make it easier to grep/read.



--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:48:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Removed the obsolete Llama options files

[John Russell (Code Review)]

John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9219 )

Change subject: [DOCS] Removed the obsolete Llama options files
..


Patch Set 4:

There are still  tags referencing the removed files, which causes 
problems in the DITA-OT builds. If you remove these 2 lines, the HTML and PDF 
will build cleanly:

$ grep impala_reservation_request_timeout.xml ../*.ditamap
../impala_keydefs.ditamap:  
$ grep impala_v_cpu_cores.xml ../*.ditamap   
../impala_keydefs.ditamap:  


--
To view, visit http://gerrit.cloudera.org:8080/9219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:48:11 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6405: Error when string to decimal cast overflows

[Zach Amsden (Code Review)]

Zach Amsden has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9339 )

Change subject: IMPALA-6405: Error when string to decimal cast overflows
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9339
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icffccac1c1c2361447ae4b0de9b6c2ec7de071db
Gerrit-Change-Number: 9339
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Zach Amsden 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:46:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Removed the obsolete Llama options files

[John Russell (Code Review)]

John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9219 )

Change subject: [DOCS] Removed the obsolete Llama options files
..


Patch Set 4:

For structural changes, it's a good idea to run a DITA-OT build in addition to 
the normal XML validation steps (validating the map in Oxygen, pre-commit hook 
running xmllint, etc.).

Here's what I get when I run 'make' from the docs/ directory (only showing the 
first few lines):

$ make
dita -i impala.ditamap -f html5 -o build/html/ -filter impala_html.ditaval
 [gen-list] [DOTX008E][ERROR] File 
'file:/Users/jrussell/Documents/homework/apache_impala/docs/topics/impala_reservation_request_timeout.xml'
 does not exist or cannot be loaded.
 [gen-list] [DOTX008E][ERROR] File 
'file:/Users/jrussell/Documents/homework/apache_impala/docs/topics/impala_v_cpu_cores.xml'
 does not exist or cannot be loaded.
[move-meta] File 
/var/folders/4h/fq2d6grs4s5746pc6hh9x2t0gp/T/temp20180305114138982/topics/impala_v_cpu_cores.xml
 was not found.
[move-meta] File 
/var/folders/4h/fq2d6grs4s5746pc6hh9x2t0gp/T/temp20180305114138982/topics/impala_reservation_request_timeout.xml
 was not found.

There must be some more references to these files left behind, in  tags 
or other DITA maps. I'll dig a little further.


--
To view, visit http://gerrit.cloudera.org:8080/9219
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If0c2b8160af9c95ec1e1b744b558d9537dd2550d
Gerrit-Change-Number: 9219
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:44:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6515: [DOCS] HAproxy with sticky session requires the check option

[John Russell (Code Review)]

John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9293 )

Change subject: IMPALA-6515: [DOCS] HAproxy with sticky session requires the 
check option
..


Patch Set 3:

(3 comments)

Just a couple of minor nits from me. But I would like for Alan or someone more 
familiar with the HAProxy stuff to be the one who actually gives the +2 from 
the technical side.

http://gerrit.cloudera.org:8080/#/c/9293/4/docs/topics/impala_proxy.xml
File docs/topics/impala_proxy.xml:

http://gerrit.cloudera.org:8080/#/c/9293/4/docs/topics/impala_proxy.xml@489
PS4, Line 489: attention
type="important" is the value we typically use for calling any extra attention 
to  elements.


http://gerrit.cloudera.org:8080/#/c/9293/4/docs/topics/impala_proxy.xml@493
PS4, Line 493: that Hue tries to connect
Grammar: "to which Hue tries to connect".


http://gerrit.cloudera.org:8080/#/c/9293/4/docs/topics/impala_proxy.xml@493
PS4, Line 493: the Impalad server
The way this is typically worded elsewhere is "the impalad 
daemon" to be clear about what has to be running.



--
To view, visit http://gerrit.cloudera.org:8080/9293
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I47165df6624f958c2e0542e2627d4f5377789ab8
Gerrit-Change-Number: 9293
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alan Choi 
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:34:36 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9384 )

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl
File www/rpcz.tmpl:

http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@108
PS11, Line 108: Thr
> Thrift RPC ?
Done


http://gerrit.cloudera.org:8080/#/c/9384/11/www/rpcz.tmpl@230
PS11, Line 230:
  :   for (var i = 0; i < json["per_conn_metrics"].length; ++
> Does this rely on the order of the fields in the json document ? Seems a bi
I tried changing the order of the JSON and it turns out that it does depend on 
the order of the JSON.

To fix this, I made an explicit ordering of keys in the function. However, the 
tradeoff now is that we have to update this function everytime we add/remove 
fields.

Also, we cannot do it like update_krpc_services(), since we have to use the 
DataTables API to update the table.



--
To view, visit http://gerrit.cloudera.org:8080/9384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 12
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:33:52 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

[Sailesh Mukil (Code Review)]

Hello Michael Ho, Lars Volker, Dan Hecht,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9384

to look at the new patch set (#12).

Change subject: IMPALA-6347: Monitor queue depth size for outgoing RPCs for 
Reactor threads
..

IMPALA-6347: Monitor queue depth size for outgoing RPCs for Reactor threads

On systems with slow networking large queuing can occur in the Reactor
threads. It would be good to quantify how much queueing occurred.

This patch extracts the OutboundTransfer queue size information from
KRPC and displays it on the /rpcz webpage.

Testing: Tested by running queries both on a local mini-cluster and a
remote cluster and manually confirming from the webpages that connections
are made between the different nodes that exchange with each other and show
number of queued calls.

Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
---
M be/src/rpc/rpc-mgr.cc
M www/rpcz.tmpl
2 files changed, 58 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/9384/12
--
To view, visit http://gerrit.cloudera.org:8080/9384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I84fea531e98d3e84fcc57bf7533655218bc91f4c
Gerrit-Change-Number: 9384
Gerrit-PatchSet: 12
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Dan Hecht (Code Review)]

Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9493/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9493/1//COMMIT_MSG@11
PS1, Line 11: DCHECK
which DCHECK do we hit? I can't tell from the JIRA information. I'm wondering 
if adding a more direct DCHECK is warranted.



--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 05 Mar 2018 19:10:32 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6324: Support reading RLE-encoded boolean values in Parquet scanner

[Csaba Ringhofer (Code Review)]

Hello Lars Volker, Gabor Kaszab, Tim Armstrong,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9403

to look at the new patch set (#3).

Change subject: IMPALA-6324: Support reading RLE-encoded boolean values in 
Parquet scanner
..

IMPALA-6324: Support reading RLE-encoded boolean values in Parquet scanner

Impala already supported RLE encoding for levels and dictionary pages, so
the only task was to integrate it into BoolColumnReader.

There might be a small performance impact on PLAIN encoded booleans,
because of the additional branch when the cache of BoolColumnReader is
filled. As the cache size is 128, I considered this to be outside the
"hot loop", but some performance measurement may be needed to validate
this.

Testing:

As Impala cannot write RLE encoded bool columns at the moment, parquet-mr
was used to create a test file, testdata/data/rle_encoded_bool.parquet

tests/query_test/test_scanners.py#test_rle_encoded_bools creates a table
that uses this file, and tries to query from it.

Change-Id: I4644bf8cf5d2b7238b05076407fbf78ab5d2c14f
---
M be/src/exec/parquet-column-readers.cc
M be/src/exec/parquet-column-readers.h
M be/src/util/rle-encoding.h
M testdata/data/README
A testdata/data/rle_encoded_bool.parquet
A 
testdata/workloads/functional-query/queries/QueryTest/parquet-rle-encoded-bool.test
M tests/query_test/test_scanners.py
7 files changed, 102 insertions(+), 43 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/03/9403/3
--
To view, visit http://gerrit.cloudera.org:8080/9403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4644bf8cf5d2b7238b05076407fbf78ab5d2c14f
Gerrit-Change-Number: 9403
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4795: Allow fetching function obj from catalog using signature

[Bikramjeet Vig (Code Review)]

Bikramjeet Vig has abandoned this change. ( 
http://gerrit.cloudera.org:8080/9225 )

Change subject: IMPALA-4795: Allow fetching function obj from catalog using 
signature
..


Abandoned

abandoned until IMPALA-6215 goes in
--
To view, visit http://gerrit.cloudera.org:8080/9225
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I3c19edd56e645b8bee918a111c76fe3f260142fe
Gerrit-Change-Number: 9225
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9286 )

Change subject: IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters
..

IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters

Noted that after enabling ha-proxy in a kerberized cluster,
users can no-longer connect to individual impala daemons directly
from impala shell.

Change-Id: I1ce9930e3f658c52502a2ba95f93647e2706d58c
Reviewed-on: http://gerrit.cloudera.org:8080/9286
Reviewed-by: Sailesh Mukil 
Reviewed-by: John Russell 
Tested-by: Impala Public Jenkins
---
M docs/topics/impala_proxy.xml
1 file changed, 12 insertions(+), 4 deletions(-)

Approvals:
  Sailesh Mukil: Looks good to me, but someone else must approve
  John Russell: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/9286
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1ce9930e3f658c52502a2ba95f93647e2706d58c
Gerrit-Change-Number: 9286
Gerrit-PatchSet: 7
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9286 )

Change subject: IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters
..


Patch Set 6: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/9286
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce9930e3f658c52502a2ba95f93647e2706d58c
Gerrit-Change-Number: 9286
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Mar 2018 18:42:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9493 )

Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc
File be/src/exec/blocking-join-node.cc:

http://gerrit.cloudera.org:8080/#/c/9493/1/be/src/exec/blocking-join-node.cc@165
PS1, Line 165: if (!status->ok()) build_sink->Close(state);
Fix looks ok.

I wonder whether this issue stems from the subtle difference in how the 
sync/async paths handle errors and CanCloseBuildEarly().

I'm thinking one way to make this more uniform would be to move this code into 
the ProcessBuildInputAndOpenProbe(). If we get a non-ok status then we return 
from ProcessBuildInputAndOpenProbe() and let cleanup be handled by the Close() 
of that join node (which correctly closes the sink before children).

Otherwise, we check CanCloseBuildEarly() and clean up as necessary. That way 
the sync/async paths look more similar and have uniform cleanup (just done by 
Close() of the exec node).



--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 05 Mar 2018 18:41:17 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9286 )

Change subject: IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-docs-submit/203/


--
To view, visit http://gerrit.cloudera.org:8080/9286
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce9930e3f658c52502a2ba95f93647e2706d58c
Gerrit-Change-Number: 9286
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Mar 2018 18:35:59 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters

[John Russell (Code Review)]

John Russell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9286 )

Change subject: IMPALA-6509: [DOCS] Note for haproxy for Kerberized clusters
..


Patch Set 6: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9286
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce9930e3f658c52502a2ba95f93647e2706d58c
Gerrit-Change-Number: 9286
Gerrit-PatchSet: 6
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: John Russell 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Mar 2018 18:35:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6595: fix crash in NljBuilder::Close()

[Tim Armstrong (Code Review)]

Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9493


Change subject: IMPALA-6595: fix crash in NljBuilder::Close()
..

IMPALA-6595: fix crash in NljBuilder::Close()

The bug is that the right child of a blocking join node could be closed
before the builder if an error was encountered when sending a batch to
the sink. This hits a DCHECK because Buffers owned by the sink may still
be accounted against the child node.

Testing:
Added the test that originally triggered the problem. It reproduced the
failure when based on the IMPALA-4835 patch, but I can't reproduce
the failure after rebase onto master.

Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
---
M be/src/exec/blocking-join-node.cc
M 
testdata/workloads/functional-query/queries/QueryTest/single-node-nlj-exhaustive.test
2 files changed, 16 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/9493/1
--
To view, visit http://gerrit.cloudera.org:8080/9493
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie46b87a4889d7cee907124796c830db41125cf15
Gerrit-Change-Number: 9493
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 

[Impala-ASF-CR] IMPALA-4430: Update build scripts to die hard when IMPALA HOME has spaces

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9385 )

Change subject: IMPALA-4430: Update build scripts to die hard when IMPALA_HOME 
has spaces
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9385/5/bin/impala-config.sh
File bin/impala-config.sh:

http://gerrit.cloudera.org:8080/#/c/9385/5/bin/impala-config.sh@48
PS5, Line 48:  "'$IMPALA_HOME'" =~ [[:blank:]]
See below on where to put this, but that said, make sure any checks you use are 
zsh-compliant. If you need advice for zsh, I'd suggest checking with Lars 
Volker.


http://gerrit.cloudera.org:8080/#/c/9385/5/bin/impala-config.sh@56
PS5, Line 56: else
:   echo "IMPALA_HOME cannot have spaces in the path"
:   exit 1
: fi
Not sure it makes sense to put this check here. The L56 condition needs to 
still happen if L51 or L53 yields an IMPALA_HOME with spaces. I'd suggest 
putting this check below the L48-53 block (on the left side of the diff) as a 
separate check.



--
To view, visit http://gerrit.cloudera.org:8080/9385
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I08b3d2b3f3e14c568d1672ee86ff2c52e8017b81
Gerrit-Change-Number: 9385
Gerrit-PatchSet: 5
Gerrit-Owner: Nithya Janarthanan 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Nithya Janarthanan 
Gerrit-Comment-Date: Mon, 05 Mar 2018 17:38:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3866 Improve error reporting for scratch write errors

[Gabor Kaszab (Code Review)]

Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9420 )

Change subject: IMPALA-3866 Improve error reporting for scratch write errors
..


Patch Set 2:

(5 comments)

Thx Attila for taking a look! Let me answer the ones I could from the top of my 
head and get back with the rest tomorrow(ish).

http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@15
PS2, Line 15: In addition there were two functions, NewFile() and
: FileAllocateSpace() that always returned Status::OK(). I made them
: void and removed the status checks from the call sites.
> Any reason you included these changes?
I observed this behaviour during the investigation and seemed a good idea to 
make them void. Somewhat related to error message enhancement, but more of a 
dead code removal. Do you think I should leave them as they are?


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc
File be/src/runtime/io/disk-io-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@384
PS2, Line 384: ExecuteWriteFailureTest
> Mayb it woyld be cleaner if you passed "/tmp/disk_io_mgr_test.txt" to Execu
The reason I implemented this way is that In case I followed how InvalidWrite 
executes 2 test then I would end up having 5 writes running in parallel and 
then I couldn't guarantee the execution order. ExecuteWriteFailureTest waits 
until the triggered read finishes.


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h
File be/src/runtime/io/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@440
PS2, Line 440: Function names can be "open", "fdopen", "fseek", "fwrite"
 :   // and "fclose".
> Why not use an enum instead?
I was thinking of this as well. The reason I chose strings is that I'd like to 
include the function name to the error text, so in case I had an enum I would 
also need some map to translate it to string, right?


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc
File be/src/runtime/io/disk-io-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@765
PS2, Line 765: fd < 0 || DebugFaultInjection("open")
> Maybe it would be cleaner if we used inheritance and virtual functions to i
Seems a good idea not to mess the prod code for fault injection. I'll give this 
a try, thx!


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.cc
File be/src/runtime/io/errno-to-error-status-converter.cc:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/errno-to-error-status-converter.cc@25
PS2, Line 25: unordered_map 
ErrnoToErrorStatusConverter::errnoToErrorTextMap_(
: {{EACCES, "Access denied for the process' user"},
:  {EINTR,   "Internal error occured."},
:  {EINVAL,  "Invalid inputs."},
:  {EMFILE,  "Process level opened file descriptor count is 
reached."},
:  {ENAMETOOLONG,
:  "Either the path length or a path component exceeds the 
maximum length."},
:  {ENFILE,  "OS level opened file descriptor count is 
reached."},
:  {ENOENT,  "The given path doesn't exist."},
:  {ENOSPC,  "No space left on device."},
:  {ENOTDIR, "It is not a directory."},
:  {EOVERFLOW, "File size can't be represented."},
:  {EROFS,   "The file system is read only."},
:  {EAGAIN,  "Resource temporarily unavailable."},
:  {EBADF,   "The given file descriptor is invalid."},
:  {ENOMEM,  "Not enough memory."},
:  {EFBIG,   "Maximum file size reached."},
:  {EIO, "Disk level I/O error occured."},
:  {ENXIO,   "Device doesn't exist."}});
> Where did you get the content of the map from?
Gathered the possible errno's from these pages:
http://pubs.opengroup.org/onlinepubs/009696699/functions/open.html
http://pubs.opengroup.org/onlinepubs/009696699/functions/fdopen.html
http://pubs.opengroup.org/onlinepubs/009696699/functions/fseek.html
http://pubs.opengroup.org/onlinepubs/009696699/functions/fwrite.html
http://pubs.opengroup.org/onlinepubs/009696699/functions/fclose.html

I also collected inputs from Tim and Jeszy about these. I sent the related doc.



--
To view, visit http://gerrit.cloudera.org:8080/9420
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5aa7b424209b1a5ef8dc7d04c5ba58788e91aad7
Gerrit-Change-Number: 9420
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Attila Jeges 

[Impala-ASF-CR] IMPALA-4795: Allow fetching function obj from catalog using signature

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9225 )

Change subject: IMPALA-4795: Allow fetching function obj from catalog using 
signature
..


Patch Set 1:

Can we abandon this for now until IMPALA-6215 goes in?


--
To view, visit http://gerrit.cloudera.org:8080/9225
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c19edd56e645b8bee918a111c76fe3f260142fe
Gerrit-Change-Number: 9225
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 17:11:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3866 Improve error reporting for scratch write errors

[Attila Jeges (Code Review)]

Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9420 )

Change subject: IMPALA-3866 Improve error reporting for scratch write errors
..


Patch Set 2:

(32 comments)

http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@9
PS2, Line 9: anhanced
typo: enhanced


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@11
PS2, Line 11: function
typo: functions


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@12
PS2, Line 12: Once
If


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@13
PS2, Line 13: than
typo: then


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@15
PS2, Line 15: In addition there were two functions, NewFile() and
: FileAllocateSpace() that always returned Status::OK(). I made them
: void and removed the status checks from the call sites.
Any reason you included these changes?


http://gerrit.cloudera.org:8080/#/c/9420/2//COMMIT_MSG@20
PS2, Line 20: funtions
typo: functions


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc
File be/src/runtime/io/disk-io-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@242
PS2, Line 242: make
making


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@242
PS2, Line 242: //
Probably you should move L242-L255 after L236


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@245
PS2, Line 245: phase_to_fail - Gives the name
'phase_to_fail' specifies the name


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@246
PS2, Line 246: 'fdopen'
"fdopen"


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@250
PS2, Line 250: ExecuteWriteFailureTest
ExecuteWriteFailureTest()


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@376
PS2, Line 376: WriteErrorInFdopen
The name is misleading as errors in functions other than fdopen() are also 
tested.


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@384
PS2, Line 384: ExecuteWriteFailureTest
Even better, you should get rid off ExecuteWriteFailureTest() completely.

Move L415-L421 to the beginning of WriteErrorInFdopen and also L427-L433 to the 
end of WriteErrorInFdopen, also set num_of_writes to 5.

Maybe I'm missing something here. Any reason you chose to implement tests this 
way?


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@384
PS2, Line 384: ExecuteWriteFailureTest
Mayb it woyld be cleaner if you passed "/tmp/disk_io_mgr_test.txt" to 
ExecuteWriteFailureTest() as a parameter.


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr-test.cc@418
PS2, Line 418: tmp_file.c_str()
tmp_file would probably work too?


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h
File be/src/runtime/io/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@440
PS2, Line 440: Function names can be "open", "fdopen", "fseek", "fwrite"
 :   // and "fclose".
Why not use an enum instead?


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@445
PS2, Line 445: the given phase parameter
'phase' parameter


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.h@448
PS2, Line 448: string
const string& ?


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc
File be/src/runtime/io/disk-io-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@765
PS2, Line 765: fd < 0 || DebugFaultInjection("open")
Maybe it would be cleaner if we used inheritance and virtual functions to 
implement fault injection?

class DiskIoMgr {
virtual bool chk_open(int fd) { return fd >= 0; }
};

All the fault injection related members can be moved to an inherited test class:

class DiskIoMgrWithFaultInjection : public DiskIoMgr {
virtual bool chk_open(int fd) { return fd >= 0 && !DebugFaultInjection("open"); 
}

bool DebugFaultInjection(..);
boost::optional > faultInjectionToWrite_;

};


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@782
PS2, Line 782: (ret_status.ok() && success != 0) || 
DebugFaultInjection("fclose")
Shouldn't this be:

ret_status.ok() && (success != 0 || DebugFaultInjection("fclose"))


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@812
PS2, Line 812: string
const string& ?


http://gerrit.cloudera.org:8080/#/c/9420/2/be/src/runtime/io/disk-io-mgr.cc@812
PS2, Line 812: DebugFaultInjection
It would be safer to pass in 'errno' as a parameter. Remember that any system 
call inside this function may reset errno.

[Impala-ASF-CR] IMPALA-6324: Support reading RLE-encoded boolean values in Parquet scanner

[Gabor Kaszab (Code Review)]

Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9403 )

Change subject: IMPALA-6324: Support reading RLE-encoded boolean values in 
Parquet scanner
..


Patch Set 2:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/9403/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9403/2//COMMIT_MSG@24
PS2, Line 24: query into it
nit: query from it?


http://gerrit.cloudera.org:8080/#/c/9403/2/be/src/exec/parquet-column-readers.cc
File be/src/exec/parquet-column-readers.cc:

http://gerrit.cloudera.org:8080/#/c/9403/2/be/src/exec/parquet-column-readers.cc@185
PS2, Line 185:   return num_cached_levels > 0;
instead?
return num_cached_levels != NULL

num_cached_levels > 0 doesn't indicate for me that num_cached_levels is a 
pointer.


http://gerrit.cloudera.org:8080/#/c/9403/2/be/src/exec/parquet-column-readers.cc@665
PS2, Line 665: switch(page_encoding_) {
Would it have any impact if you did a reset on both bool_values_ and 
rle_decoder_ here unconditionally?
If this has to stay, could you enhance the function's comment to reflect?

As I see you branch on page_encoding_ anyway where you use them.


http://gerrit.cloudera.org:8080/#/c/9403/2/be/src/exec/parquet-column-readers.cc@714
PS2, Line 714:   num_unpacked_values_ = page_encoding_ == 
parquet::Encoding::PLAIN
nit: I find a simple if more readable than the ternary operator. Might be just 
my preference, though.


http://gerrit.cloudera.org:8080/#/c/9403/2/be/src/util/rle-encoding.h
File be/src/util/rle-encoding.h:

http://gerrit.cloudera.org:8080/#/c/9403/2/be/src/util/rle-encoding.h@139
PS2, Line 139:   int32_t GetValues(int32_t num_values_to_consume, T* values);
As I see the return value here serves 2 different purposes:
 1) As a general return value to indicate success or failure
 2) An out parameter to get the num_values_to_consume.
I find this a bit confusing. Can't you have a return value for the purpose of 
1) and an out param for 2) ?
What do you think?


http://gerrit.cloudera.org:8080/#/c/9403/2/be/src/util/rle-encoding.h@666
PS2, Line 666:   int32_t num_unpacked_values = 0;
The names of num_unpacked_values and num_values_to_consume are not in line in 
my opinion. Either the first one should be num_consumed_values or the second 
one num_values_to_unpack.


http://gerrit.cloudera.org:8080/#/c/9403/2/testdata/data/README
File testdata/data/README:

http://gerrit.cloudera.org:8080/#/c/9403/2/testdata/data/README@152
PS2, Line 152: rle_encoded_bool.parquet:
According to parquet-rle-encoded-bool.test there are 140 True values in column 
b. Could you please mention in the comment this? (and the total row count would 
also seem reasonable for future use).


http://gerrit.cloudera.org:8080/#/c/9403/2/testdata/data/README@154
PS2, Line 154: as I found no other way to force RLE
 : encoding of booleans in Parquet v1
nit: This part of the sentence doesn't provide extra information for the reader 
in my opinion.


http://gerrit.cloudera.org:8080/#/c/9403/2/testdata/data/README@155
PS2, Line 155: It became the default encoding in Parquet v2, but
 : Impala is not able to read Parquet v2 files yet.
nit: I'm not sure this information is required here either. (what if someone 
reads this after Parquet v2 reading is implemented in Impala?)


http://gerrit.cloudera.org:8080/#/c/9403/2/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/9403/2/tests/query_test/test_scanners.py@649
PS2, Line 649: "refresh {0}.rle_encoded_bool".format(unique_database));
For my understanding: Why is this refresh needed here? I checked other tests 
that include .parquet files (e.g. test_zero_rows) but those doesn't do this 
refresh step.



--
To view, visit http://gerrit.cloudera.org:8080/9403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4644bf8cf5d2b7238b05076407fbf78ab5d2c14f
Gerrit-Change-Number: 9403
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 05 Mar 2018 16:04:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Draft: Add JSON output to MemTracker::LogUsage()

[Csaba Ringhofer (Code Review)]

Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8955 )

Change subject: Draft: Add JSON output to MemTracker::LogUsage()
..


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:

http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.h@339
PS1, Line 339:   void LogUsage(int max_recursive_depth, std::stringstream* ss, 
const std::string& prefix,
 :   rapidjson::Value* val, rapidjson::Document* doc, int64_t* 
logged_consumption);
 :   /// Convenience wrapper around the previous declaration to 
make it easier to use in
 :   /// DCHECKs.
 :   std::string LogUsage(int max_recursive_depth);
I saw a place where the old interface is used with prefix and 
logged_consumption: 
https://github.com/apache/impala/blob/3302abaa32cf3290cb038fbc1dbd0a821f5cbbc7/be/src/runtime/bufferpool/reservation-tracker-test.cc#L543


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@256
PS1, Line 256: void MemTracker::LogUsage(int max_recursive_depth, 
std::stringstream* ss,
I think that this became a bit too complicated for a log function. My idea for 
an alternative solution is creating a struct with the variables we want to log, 
and split this function to two somewhat less complex ones: one that walks the 
tree and collects these structs (essentially creating a snapshot), and one that 
serializes them into string/json. While this would mean some extra copies, a 
potential performance benefit could be that string formatting would happen when 
no child_trackers_lock_ is locked. If the structs would have a depth field, 
then it would be enough to collect them to a vector (without loosing the tree 
structure).


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@301
PS1, Line 301:   val->AddMember("gauges", gauges, doc->GetAllocator());
if (val != nullptr)


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@309
PS1, Line 309:   string child_trackers_usage;
This variable is no longer used.


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@315
PS1, Line 315: val->AddMember("children", children, doc->GetAllocator());
if (val != nullptr)


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@340
PS1, Line 340:   vector usage_strings;
This variable is no longer used.



--
To view, visit http://gerrit.cloudera.org:8080/8955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a59c39b0a85c78f1e8db10a93cbb814dc6adfa0
Gerrit-Change-Number: 8955
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 05 Mar 2018 13:24:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6542: Fix inconsistent write path of Parquet min/max statistics

[Gabor Kaszab (Code Review)]

Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9381 )

Change subject: IMPALA-6542: Fix inconsistent write path of Parquet min/max 
statistics
..


Patch Set 5: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/9381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3957806948f7c661af4be5495f2ec92d1e9fc9d6
Gerrit-Change-Number: 9381
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 05 Mar 2018 11:13:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
..

IMPALA-2567: Enable KRPC by default

This change enables the switch to use KRPC by default.
This change also fixes a bug in KrpcDataStreamMgr to
check if maintenance thread was started before calling
Join() on it. This shows up in BE tests as the maintenance
thread isn't started in them.

Testing done: exhaustive build.

Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Reviewed-on: http://gerrit.cloudera.org:8080/9461
Reviewed-by: Michael Ho 
Tested-by: Impala Public Jenkins
---
M be/src/runtime/exec-env.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
M tests/common/test_skip.py
M tests/conftest.py
8 files changed, 25 insertions(+), 26 deletions(-)

Approvals:
  Michael Ho: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/9461
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
..


Patch Set 6: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/9461
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 05 Mar 2018 08:57:39 +
Gerrit-HasComments: No