[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-1702: Enforce unique table ID
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 188: private final HashMap tblIdMap_ =
> yeah, but ideally any table change should trigger a new analyzing just like
You raise a good point. Maybe we should consider how to ensure at least 
table-level consistency during analysis. For example, one way could be to add 
referenced Tables into the Analyzer such that subsequent references will see 
the same Table object (instead of going to the Catalog every time).

Seems relatively easy to do.

Dimitris also has a good point that distinguishing the drop+add case from the 
invalidate case may be hard, but I think the proposed Analyzer change above 
should handle that, right?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4091: Fix backend unit to log in logs/be tests.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4091: Fix backend unit to log in logs/be_tests.
..


Patch Set 4: Code-Review+2

rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaff0acf09bf192d54baeb0bb347e895fce6ed23b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4091: Fix backend unit to log in logs/be tests.

[Alex Behm (Code Review)]

Hello Lars Volker,

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

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

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

Change subject: IMPALA-4091: Fix backend unit to log in logs/be_tests.
..

IMPALA-4091: Fix backend unit to log in logs/be_tests.

1. Many backend unit tests did not follow proper initialization
using InitCommonRuntime(), and as a result did not write their
logs into the logs/be_tests directory.

2. Added an IMPALA_TEST_MAIN() macro that stamps out the common
main() function used in most gtest unit tests.

3. Tests added via ADD_UDF_TEST in a CMakeLists.txt did not
have the logging dir set up properly.

Testing: I validated that every test produces a corresponding
.INFO file in logs/be_tests. The only exception is promise-test
for which I added a TODO since the fix seems non-trivial.

Change-Id: Iaff0acf09bf192d54baeb0bb347e895fce6ed23b
---
M be/CMakeLists.txt
M be/src/codegen/instruction-counter-test.cc
M be/src/common/atomic-test.cc
M be/src/common/init.h
M be/src/exec/delimited-text-parser-test.cc
M be/src/exec/hdfs-avro-scanner-test.cc
M be/src/exec/incr-stats-util-test.cc
M be/src/exec/incr-stats-util.cc
M be/src/exec/old-hash-table-test.cc
M be/src/exec/parquet-plain-test.cc
M be/src/exec/parquet-version-test.cc
M be/src/exec/read-write-util-test.cc
M be/src/exec/row-batch-list-test.cc
M be/src/exec/zigzag-test.cc
M be/src/experiments/string-search-sse-test.cc
M be/src/exprs/aggregate-functions-test.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-util-test.cc
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/collection-value-builder-test.cc
M be/src/runtime/decimal-test.cc
M be/src/runtime/free-pool-test.cc
M be/src/runtime/free-pool.h
M be/src/runtime/hdfs-fs-cache-test.cc
M be/src/runtime/mem-pool-test.cc
M be/src/runtime/mem-tracker-test.cc
M be/src/runtime/multi-precision-test.cc
M be/src/runtime/parallel-executor-test.cc
M be/src/runtime/raw-value-test.cc
M be/src/runtime/row-batch-serialize-test.cc
M be/src/runtime/string-buffer-test.cc
M be/src/runtime/string-value-test.cc
M be/src/runtime/thread-resource-mgr-test.cc
M be/src/runtime/timestamp-test.cc
M be/src/scheduling/backend-config-test.cc
M be/src/scheduling/simple-scheduler-test.cc
M be/src/service/hs2-util-test.cc
M be/src/service/query-options-test.cc
M be/src/service/session-expiry-test.cc
M be/src/testutil/gtest-util.h
M be/src/udf/uda-test.cc
M be/src/udf/udf-test.cc
M be/src/util/benchmark-test.cc
M be/src/util/bit-util-test.cc
M be/src/util/bitmap-test.cc
M be/src/util/blocking-queue-test.cc
M be/src/util/bloom-filter-test.cc
M be/src/util/coding-util-test.cc
M be/src/util/debug-util-test.cc
M be/src/util/decompress-test.cc
M be/src/util/dict-test.cc
M be/src/util/error-util-test.cc
M be/src/util/filesystem-util-test.cc
M be/src/util/fixed-size-hash-table-test.cc
M be/src/util/internal-queue-test.cc
M be/src/util/logging-support-test.cc
M be/src/util/lru-cache-test.cc
M be/src/util/metrics-test.cc
M be/src/util/parse-util-test.cc
M be/src/util/perf-counters-test.cc
M be/src/util/pretty-printer-test.cc
M be/src/util/promise-test.cc
M be/src/util/redactor-config-parser-test.cc
M be/src/util/redactor-test.cc
M be/src/util/redactor-unconfigured-test.cc
M be/src/util/rle-test.cc
M be/src/util/runtime-profile-test.cc
M be/src/util/runtime-profile.cc
M be/src/util/string-parser-test.cc
M be/src/util/symbols-util-test.cc
M be/src/util/thread-pool-test.cc
M be/src/util/uid-util-test.cc
M be/src/util/webserver-test.cc
73 files changed, 169 insertions(+), 421 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaff0acf09bf192d54baeb0bb347e895fce6ed23b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Lars Volker 

[Impala-ASF-CR] IMPALA-4110: Clean up issues found by Apache RAT.

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-4110: Clean up issues found by Apache RAT.
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4361/6/be/.impala.doxy
File be/.impala.doxy:

Line 17
should we really delete all these existing comments?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bfe77f9a871018e7a67553ed270e2df53006962
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3718: Support subset of functional-query for Kudu

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3718: Support subset of functional-query for Kudu
..


Patch Set 4: Code-Review+2

Fixed the commit msg, carrying the +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada88e078352e4462745d9a9a1b5111260d21acc
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2013: Reintroduce steps for checking HBase health in run-hbase.sh

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-2013: Reintroduce steps for checking HBase health in 
run-hbase.sh
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4348/8/testdata/bin/check-hbase-nodes.py
File testdata/bin/check-hbase-nodes.py:

Line 88: LOGGER.debug("Success: " + str(zk_client))
> That's a good catch, but I think it might be OK as-is. This is the output o
Wfm.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b81f3cfb6ea0ba7b18ce5fcd5d268f515c8b0c3
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Ishaan Joshi 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4008: don't bake in hash table and hash join pointers

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4008: don't bake in hash table and hash join pointers
..


Patch Set 6: Code-Review+2

rebase

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie353666dbb5c958f0094d169306fe930ec3014c5
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-1702: Enforce unique table ID
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 188: private final HashMap tblIdMap_ =
> Isn't that ok? Once you drop+create a table with same name, it isn't essent
I think that behavior is actually desired. If a table with the same name was 
dropped+added it *should* have a different table id.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-1702: Enforce unique table ID
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 188: private final HashMap tblIdMap_ =
> Alex, Bharath, another possibility is that given our asymmetry architecture
Isn't that ok? Once you drop+create a table with same name, it isn't 
essentially the "same" table anymore, for ex: its schema could've changed. IMO 
thats fine. I think this is what Alex meant when he mentioned that drop+create 
should create a new table ID.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4110: Apache RAT script on Impala tarballs.

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: IMPALA-4110: Apache RAT script on Impala tarballs.
..


Patch Set 1:

Separated from https://gerrit.cloudera.org/#/c/4361/ following a request by 
Alex.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic95bd38fbb90f9a901602dd91cee541b16bf4714
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4110: PREVIEW: Make RAT run on Impala tarballs.

[Jim Apple (Code Review)]

Jim Apple has uploaded a new patch set (#4).

Change subject: IMPALA-4110: PREVIEW: Make RAT run on Impala tarballs.
..

IMPALA-4110: PREVIEW: Make RAT run on Impala tarballs.

While I'm here, fix as many of the warnings as possible.

The instructions on how to run this are in check-rat-report.py:

1. Either download a release candidate or create an archive from your
   git repo using `git archive --prefix=Foo/ -o baz.tar.gz HEAD`

2. Untar and run `java -jar apache-rat-0.12.jar -x . >baz.xml`. Only
   RAT version 0.12 is known to work at this time.

3. `bin/check-rat-report.py bin/rat_exclude_files.txt baz.xml`

Change-Id: I5bfe77f9a871018e7a67553ed270e2df53006962
---
M LICENSE.txt
M be/.astylerc
M be/.impala.doxy
M be/src/benchmarks/expr-benchmark.cc
M be/src/experiments/data-provider.cc
M be/src/experiments/hashing/cache-hash-test.cc
M be/src/experiments/hashing/growing-test.cc
M be/src/experiments/hashing/interface/cache-hash-test.cc
M be/src/experiments/hashing/interface/growing-test.cc
M be/src/experiments/hashing/multilevel/cache-hash-test.cc
M be/src/experiments/hashing/multilevel/growing-test.cc
M be/src/experiments/hashing/prefetch/cache-hash-test.cc
M be/src/experiments/hashing/prefetch/growing-test.cc
M be/src/experiments/hashing/split-benchmarks/cache-hash-test.cc
M be/src/experiments/hashing/split-benchmarks/growing-test.cc
M be/src/experiments/hashing/split-benchmarks/partitioning-throughput-test.cc
M be/src/experiments/hashing/streaming/cache-hash-test.cc
M be/src/experiments/hashing/streaming/growing-test.cc
M be/src/testutil/certificates-info.txt
M be/src/util/asan.h
M bin/README-RUNNING-BENCHMARKS
D bin/cpplint.py
M bin/impala-ipython
M bin/impala-py.test
M bin/impala-python
M cmake_modules/FindAvro.cmake
M cmake_modules/FindBreakpad.cmake
M cmake_modules/FindBzip2.cmake
M cmake_modules/FindGFlags.cmake
M cmake_modules/FindGLog.cmake
M cmake_modules/FindHDFS.cmake
M cmake_modules/FindJNI.cmake
M cmake_modules/FindLdap.cmake
M cmake_modules/FindLlvm.cmake
M cmake_modules/FindLlvmBinaries.cmake
M cmake_modules/FindLz4.cmake
M cmake_modules/FindOpenSSL.cmake
M cmake_modules/FindPProf.cmake
M cmake_modules/FindRapidJson.cmake
M cmake_modules/FindRe2.cmake
M cmake_modules/FindSnappy.cmake
M cmake_modules/FindThrift.cmake
M cmake_modules/FindZlib.cmake
M fe/src/main/java/com/cloudera/impala/analysis/AggregateInfoBase.java
M fe/src/main/java/com/cloudera/impala/analysis/ExprSubstitutionMap.java
M fe/src/main/java/com/cloudera/impala/analysis/PartitionSpec.java
M fe/src/main/java/com/cloudera/impala/catalog/ArrayType.java
M fe/src/main/java/com/cloudera/impala/catalog/MapType.java
M fe/src/main/java/com/cloudera/impala/catalog/StructType.java
M fe/src/main/java/com/cloudera/impala/planner/Planner.java
M fe/src/test/resources/authz-policy.ini.template
M fe/src/test/resources/hive-log4j.properties.template
M fe/src/test/resources/log4j.properties.template
M infra/python/deps/download_requirements
M infra/python/deps/requirements.txt
M testdata/avro_schema_resolution/create_table.sql
M testdata/bin/cache_tables.py
M testdata/bin/create-mini.sql
M testdata/bin/load-dependent-tables.sql
M testdata/bin/load-hive-builtins.sh
M testdata/bin/run-hive.sh
M testdata/cluster/node_templates/cdh5/etc/init.d/kms
M testdata/cluster/node_templates/cdh5/etc/init.d/kudu-common
M testdata/cluster/node_templates/cdh5/etc/init.d/kudu-master
M testdata/cluster/node_templates/cdh5/etc/init.d/kudu-tserver
M testdata/cluster/node_templates/cdh5/etc/init.d/llama-application
M testdata/cluster/node_templates/common/etc/init.d/common.tmpl
M testdata/cluster/node_templates/common/etc/init.d/hdfs-common
M testdata/cluster/node_templates/common/etc/init.d/hdfs-datanode
M testdata/cluster/node_templates/common/etc/init.d/hdfs-namenode
M testdata/cluster/node_templates/common/etc/init.d/yarn-common
M testdata/cluster/node_templates/common/etc/init.d/yarn-nodemanager
M testdata/cluster/node_templates/common/etc/init.d/yarn-resourcemanager
D testdata/data/mstr/eatwh1/EATWH1-DDL.sql
D testdata/data/mstr/eatwh1/LoadTablesEATWH1.sql
D testdata/data/mstr/eatwh1/csv/COST_MARKET_CLASS.TXT
D testdata/data/mstr/eatwh1/csv/COST_MARKET_DEP.TXT
D testdata/data/mstr/eatwh1/csv/COST_MARKET_DIV.TXT
D testdata/data/mstr/eatwh1/csv/COST_REGION_CLASS.TXT
D testdata/data/mstr/eatwh1/csv/COST_REGION_ITEM.TXT
D testdata/data/mstr/eatwh1/csv/COST_STORE_DEP.TXT
D testdata/data/mstr/eatwh1/csv/COST_STORE_ITEM.TXT
D testdata/data/mstr/eatwh1/csv/FT1.TXT
D testdata/data/mstr/eatwh1/csv/FT10.TXT
D testdata/data/mstr/eatwh1/csv/FT11.TXT
D testdata/data/mstr/eatwh1/csv/FT12.TXT
D testdata/data/mstr/eatwh1/csv/FT13.TXT
D testdata/data/mstr/eatwh1/csv/FT14.TXT
D testdata/data/mstr/eatwh1/csv/FT15.TXT
D testdata/data/mstr/eatwh1/csv/FT17.TXT
D testdata/data/mstr/eatwh1/csv/FT2.TXT
D testdata/data/mstr/eatwh1/csv/FT3.TXT
D testdata/data/mstr/eatwh1/csv/FT4.TXT
D 

[Impala-ASF-CR] IMPALA-3491: Use unique db in test scanners.py and test aggregation.py

[Internal Jenkins (Code Review)]

Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-3491: Use unique db in test_scanners.py and 
test_aggregation.py
..


IMPALA-3491: Use unique db in test_scanners.py and test_aggregation.py

Testing: Ran the tests locally in a loop on exhaustive.
Did a private debug/exhaustive run.

Change-Id: Ided0848c138bdc1d43694a1010c48e23ee1c
Reviewed-on: http://gerrit.cloudera.org:8080/4339
Reviewed-by: Alex Behm 
Tested-by: Internal Jenkins
---
M 
testdata/workloads/functional-query/queries/QueryTest/aggregation_no_codegen_only.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_scanners.py
3 files changed, 20 insertions(+), 35 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ided0848c138bdc1d43694a1010c48e23ee1c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] IMPALA-3949: Log the error message in FileSystemUtil.copyToLocal()

[Internal Jenkins (Code Review)]

Internal Jenkins has posted comments on this change.

Change subject: IMPALA-3949: Log the error message in 
FileSystemUtil.copyToLocal()
..


Patch Set 10: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5664a75aa837951de1d5dcc90e43bd8f313736b8
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Brock Noland 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-1702: Enforce unique table ID

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-1702: Enforce unique table ID
..


Patch Set 2:

(1 comment)

I think we should avoid both forms of id inconsistency: Different tables with 
the same id, or the same table with different ids.

http://gerrit.cloudera.org:8080/#/c/4349/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:

Line 188: private final HashMap tblIdMap_ =
Why do we need another map? Don't we already have a map of existing tables that 
we can use to get the current id?

I think it could actually be problematic to only rely on the TTableName for 
identification. The user could drop+add the same table and I think in that case 
a new id should be assigned. Thoughts?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifad648b72684ae495ec387590ab1bc58ce5b39e2
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Huaisi Xu 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Huaisi Xu 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3980: qgen: re-enable Hive as a target database

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3980: qgen: re-enable Hive as a target database
..


Patch Set 4:

I'd like you to run the small suite of cluster tests, but you can't yet until 
you rebase on top of https://gerrit.cloudera.org/#/c/4404/ I realize there 
aren't that many, and they may not find anything, but it's a habit we need to 
start. Thanks.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: stak...@cloudera.com
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: stak...@cloudera.com
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4122: qgen: fix bitrotted cluster unit tests

[Michael Brown (Code Review)]

Michael Brown has uploaded a new change for review.

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

Change subject: IMPALA-4122: qgen: fix bitrotted cluster unit tests
..

IMPALA-4122: qgen: fix bitrotted cluster unit tests

There's a small set of pytest-style tests and associated conftest for
testing some of the cluster-related test infrastructure Python objects.
Going forward, I want unit tests for the query generator to be run as
part of patch acceptance (CI isn't necessary at this time).

This patch fixes a bitrotted test and moves the tests into the
tests-for-qgen directory. The moves were performed thusly:

 $ git mv conftest.py tests/
 $ git mv cluster_tests.py tests/test_cluster.py

Change-Id: I3e855e265ae245ebe3691d077284ac5761909e00
---
R tests/comparison/tests/conftest.py
R tests/comparison/tests/test_cluster.py
2 files changed, 6 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3e855e265ae245ebe3691d077284ac5761909e00
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] IMPALA-4008: don't bake in hash table and hash join pointers

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4008: don't bake in hash table and hash join pointers
..


Patch Set 3:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/4326/3/be/src/exec/hash-table.h
File be/src/exec/hash-table.h:

PS3, Line 300: ExprValuesHash
> CurExprValuesHash()  [see below for motiviation]
Done


PS3, Line 303: SetExprValuesHash
> Let's rename this to more closely match cur_expr_values() rather than match
Done


PS3, Line 309: cur_expr_values_null
> comment explaining that there is one byte per null value (i.e. these are us
Done.

I'm not actually sure why we need this for codegen, since bool and uint8_t 
should both be represented as int8 internally in LLVM, but I don't want to get 
distracted digging into that, so I left a TODO.

There's some weirdness with vector where it uses bitfields but otherwise 
C++ bools should be 8 bits - maybe that was the reason.


PS3, Line 403: in
> the first time I read this, I read it as this functions stores the values t
Makes sense - done.


PS3, Line 403: in
> same
Done


PS3, Line 413: in
> same
Done


PS3, Line 431: This will be replaced by
 :   /// codegen.
> nit: move this sentence to end of the comment so it's standardized with sim
Done


PS3, Line 438: with 'cur_expr_values_'
> "to compare against the current row."  (since it also uses cur_expr_values_
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie353666dbb5c958f0094d169306fe930ec3014c5
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps

[Tim Armstrong (Code Review)]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4111: backend death tests should not produce minidumps
..


Patch Set 4:

Somehow Lars' earlier comments got deleted. Reproducing from my email:



Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4353/3/be/src/util/minidump.h
File be/src/util/minidump.h:

Lars Volker has posted comments on this change.

Change subject: IMPALA-4111: backend death tests should not produce minidumps
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4353/3/be/src/util/minidump.h
File be/src/util/minidump.h:

PS3, Line 29:  for death test
nit: maybe remove or reword like "for example during death test" or "during 
tests"

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4026: Implement double-buffering for BlockingQueue.

[Michael Ho (Code Review)]

Michael Ho has posted comments on this change.

Change subject: IMPALA-4026: Implement double-buffering for BlockingQueue.
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4350/3/be/src/util/blocking-queue.h
File be/src/util/blocking-queue.h:

Line 99:   bool BlockingPut(const T& val) {
> I think there's a pretty bad bug with how the signalling works. 
Yes, I have thought about it before. I have tested various locations for 
notification:

1. notify_one once an entry is consumed from get_list by BlockingGet()
2. notify_all when the get_list is empty
3. notify_one when the get_list is empty

Apparently, option (3) seems to give the best performance.

I have investigated option (2) and it appears that the thundering herd effect 
causes all scanner threads to start immediately leading to contention in memory 
allocation (e.g. memset takes 6x longer), causing some TPCH queries to regress.

I suspect the slow down in option (1) may have to do the extra signaling 
overhead per entry or it could be a side effect of the poor struct layout 
before so it may be worth retrying again.

In practice, a consumer can consume a row batch faster than a scanner thread 
can produce it so the effect of option (3) could be that it scatters out when 
the scanner threads get unblocked.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f4cf351455efefb0f3bb791cf9bc82d1421d54
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Chen Huang 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: Yes

[Impala-ASF-CR](asf-site) IMPALA-4068: Focus pages on Apache Impala, not Cloudera Impala.

[Jim Apple (Code Review)]

Jim Apple has posted comments on this change.

Change subject: IMPALA-4068: Focus pages on Apache Impala, not Cloudera Impala.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4318/2/community.html
File community.html:

Line 118:   https://cwiki.apache.org/confluence/display/IMPALA/Contributing+to+Impala;>Contribute
> Good idea!
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0492bff6306089f0745bbff46f070cf3ad2d9c82
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 
Gerrit-HasComments: Yes

[Impala-ASF-CR](asf-site) IMPALA-4068: Focus pages on Apache Impala, not Cloudera Impala.

[Jim Apple (Code Review)]

Hello Henry Robinson,

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

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

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

Change subject: IMPALA-4068: Focus pages on Apache Impala, not Cloudera Impala.
..

IMPALA-4068: Focus pages on Apache Impala, not Cloudera Impala.

While I'm in here, remove outdated roadmap.

Change-Id: I0492bff6306089f0745bbff46f070cf3ad2d9c82
---
M bylaws.html
M community.html
R impala-docs.html
M index.html
M overview.html
5 files changed, 26 insertions(+), 61 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0492bff6306089f0745bbff46f070cf3ad2d9c82
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: asf-site
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Henry Robinson 
Gerrit-Reviewer: Jim Apple 

[Impala-ASF-CR] IMPALA-3491: Use unique db in test scanners.py and test aggregation.py

[Alex Behm (Code Review)]

Alex Behm has posted comments on this change.

Change subject: IMPALA-3491: Use unique db in test_scanners.py and 
test_aggregation.py
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ided0848c138bdc1d43694a1010c48e23ee1c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Michael Brown 
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2013: Reintroduce steps for checking HBase health in run-hbase.sh

[David Knupp (Code Review)]

David Knupp has uploaded a new patch set (#8).

Change subject: IMPALA-2013: Reintroduce steps for checking HBase health in 
run-hbase.sh
..

IMPALA-2013: Reintroduce steps for checking HBase health in run-hbase.sh

We used to include a step in run-hbase.sh for calling a python
script that queried Zookeeper to see if the HBase master was up.
The original script was problematic, so we stopped using it during
our mini-cluster HBase start up procedure.

HBase start up issues continue to plague us, however. This patch
reintroduces a Zookeeper check, with the following updates:

- replace the original script with check-hbase-nodes.py
- query the correct node /hbase/master, not just /hbase/rs
- use the python Zookeeper library kazoo, rather than calling
  out to the shell and parsing the return string
- since we are moving toward testing on a remote cluster, also
  add the capability to pass in the address for the host that
  provides the Zookeeper and HBase services
- add an additional check that the HDFS service is running,
  because of an edge case where the HBase master can briefly
  start without a cluster running.

In addition to the expected tests, this script was also tested
under the conditions of IMPALA-4088, whereby the HBase RegionServer
is running, but the master fails because another listening process
has already taken its TCP port (60010) during startup.

Change-Id: I9b81f3cfb6ea0ba7b18ce5fcd5d268f515c8b0c3
---
M infra/python/deps/requirements.txt
A testdata/bin/check-hbase-nodes.py
M testdata/bin/run-hbase.sh
D testdata/bin/wait-for-hbase-master.py
4 files changed, 173 insertions(+), 59 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9b81f3cfb6ea0ba7b18ce5fcd5d268f515c8b0c3
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Ishaan Joshi 
Gerrit-Reviewer: Michael Brown 

[Impala-ASF-CR] IMPALA-3973: add position and occurrence to instr()

[Zoltan Ivanfi (Code Review)]

Hello Lars Volker, Matthew Jacobs, Internal Jenkins, Dan Hecht,

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

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

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

Change subject: IMPALA-3973: add position and occurrence to instr()
..

IMPALA-3973: add position and occurrence to instr()

Change-Id: Ie9648de458d243306fa14adc5e7f7002bf6f67fd
---
M be/src/experiments/CMakeLists.txt
R be/src/experiments/string-search-sse-test.cc
M be/src/exprs/expr-test.cc
M be/src/exprs/string-functions-ir.cc
M be/src/exprs/string-functions.h
M be/src/runtime/CMakeLists.txt
A be/src/runtime/string-search-test.cc
M be/src/runtime/string-search.h
M common/function-registry/impala_functions.py
9 files changed, 343 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/94/4094/16
-- 
To view, visit http://gerrit.cloudera.org:8080/4094
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie9648de458d243306fa14adc5e7f7002bf6f67fd
Gerrit-PatchSet: 16
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Zoltan Ivanfi