date:20190614

Hello Tim Armstrong, Todd Lipcon, Impala Public Jenkins,

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

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

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

Change subject: [WIP] IMPALA-8630: Hash the full path when calculating 
consistent remote placement
..

[WIP] IMPALA-8630: Hash the full path when calculating consistent remote 
placement

Consistent remote placement currently uses the relative filename within
a partition for the consistent hash. If the relative filenames for
different partitions have a simple naming scheme, then multiple
partitions may have files of the same name. This is true for some
tables written by Hive (e.g. in our minicluster the tpcds.store_sales
has this problem). This can lead to unbalanced placement of remote
ranges.

This adds a partition_path_hash to the THdfsFileSplit and
THdfsFileSplitGeneratorSpec, calculated in the frontend (which has all of
the partition information). The scheduler hashes this in addition to
the relative path.

This is a proof of concept and is not ready to go.

Work remaining (if this is the approach we take):
 - Add some scheduler tests to simulate this approach
 - Look into an end-to-end test based on the Docker tests (which do remote IO)

Alternatives:
 - Hash the full path when reading the file metadata in the catalog
   and store it in FbFileDesc.
 - Construct the DescriptorTbl in the scheduler and use that to
   reconstruct the full path or partition path for a file and hash it.

Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
---
M be/src/scheduling/scheduler-test-util.cc
M be/src/scheduling/scheduler-test-util.h
M be/src/scheduling/scheduler-test.cc
M be/src/scheduling/scheduler.cc
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
6 files changed, 87 insertions(+), 25 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
Gerrit-Change-Number: 13545
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon

[Impala-ASF-CR] [WIP] IMPALA-8630: Hash the full path when calculating consistent remote placement

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13545 )

Change subject: [WIP] IMPALA-8630: Hash the full path when calculating 
consistent remote placement
..


Patch Set 2:

(1 comment)

I'm going to go ahead and post the version that sidesteps the catalog.

http://gerrit.cloudera.org:8080/#/c/13545/2/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/13545/2/be/src/scheduling/scheduler.cc@801
PS2, Line 801:   uint32_t hash = 
HashUtil::Hash(&hdfs_file_split->full_path_hash,
 :   sizeof(hdfs_file_split->full_path_hash), 0);
> why hash the hash instead of just making the hash here be a uint32_t direct
Fixed this. Java hashCode() returns a signed integer and backend uses unsigned. 
I was punting on deciding the right place to cast.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
Gerrit-Change-Number: 13545
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 15 Jun 2019 00:42:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8599: Create a Maven module for query event hook API

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8599: Create a Maven module for query event hook API
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84c422d83c19b75c3d1d7a772b971f4f7704d44c
Gerrit-Change-Number: 13653
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 15 Jun 2019 00:27:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] fe: improve logging for metadata loading

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: fe: improve logging for metadata loading
..


Patch Set 4:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3634/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8
Gerrit-Change-Number: 13463
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Sat, 15 Jun 2019 00:00:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-8630: Hash the full path when calculating consistent remote placement

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: [WIP] IMPALA-8630: Hash the full path when calculating 
consistent remote placement
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3633/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
Gerrit-Change-Number: 13545
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 23:56:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-8630: Hash the full path when calculating consistent remote placement

Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13545 )

Change subject: [WIP] IMPALA-8630: Hash the full path when calculating 
consistent remote placement
..


Patch Set 2:

I was missing an option: Don't store it in FbFileDesc, but hash the partition 
path it in the frontend. HdfsScanNode.java already has the partition 
information in order to pass in partition_id, so it can hash the partition path 
(FeFsPartition.getLocation()) and put it in the THdfsFileSplit. The backend 
doesn't need to rebuild the descriptor table, it just hashes one more thing. 
Nothing happens in the catalog.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
Gerrit-Change-Number: 13545
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 23:51:53 +
Gerrit-HasComments: No

[Impala-ASF-CR] [WIP] IMPALA-8630: Hash the full path when calculating consistent remote placement

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

Change subject: [WIP] IMPALA-8630: Hash the full path when calculating 
consistent remote placement
..


Patch Set 2:

(1 comment)

This did work out pretty cleanly.

http://gerrit.cloudera.org:8080/#/c/13545/2/common/fbs/CatalogObjects.fbs
File common/fbs/CatalogObjects.fbs:

http://gerrit.cloudera.org:8080/#/c/13545/2/common/fbs/CatalogObjects.fbs@81
PS2, Line 81:   // A hash of the absolute file path
I wonder if this even makes the struct bigger, given padding. Or if we can 
reorder things a bit like the above TODO suggests to reduce the padding.

If we can remove a TODO and avoid the space/time regression concern, could be a 
win.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
Gerrit-Change-Number: 13545
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 23:43:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8536: Add Scalable Pool Configuration to Admission Controller.

2019-06-14 Thread Lars Volker (Code Review)

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

Change subject: IMPALA-8536: Add Scalable Pool Configuration to Admission 
Controller.
..


Patch Set 13:

(32 comments)

Mostly "dotting the i's", but had some questions around the admission 
controller logic

http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/runtime/exec-env.cc@214
PS13, Line 214:   request_pool_service_.get(), metrics_.get(), 
configured_backend_address_));
nit: continue indent at 4 spaces


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller-test.cc
File be/src/scheduling/admission-controller-test.cc:

http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller-test.cc@50
PS13, Line 50: long
nit: int64_t


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller-test.cc@98
PS13, Line 98:   // say that every host has 200MB, enough that this isn't a 
problem.
This comment might not be true if the caller passed a different value


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller-test.cc@236
PS13, Line 236:   auto fair_flag = ScopedFlagSetter::Make(
If you use a FlagSaver in the test class, this can be simplified to just 
writing to the flags. See 
https://github.com/apache/impala/blob/master/be/src/runtime/io/data-cache-test.cc#L120
 for an example


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:

http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h@30
PS13, Line 30: #include "cluster-membership-mgr.h"
Add directory, sort into list below


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h@395
PS13, Line 395: Update
nit: Updates (here and below)


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h@660
PS13, Line 660: with the given schedule
Stale comment?


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h@660
PS13, Line 660: Return
Returns (see surrounding code and style guide)


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h@667
PS13, Line 667: with the given
  :   /// schedule.
Stale comment?


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h@677
PS13, Line 677: at once
"at once" seems a bit murky, but I understand that there's no "per time 
interval" limitation here. I actually suspect that the whole max_to_dequeue 
limitation has no effect and we might want to remove it altogether. Thoughts?


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h@694
PS13, Line 694: can run be queued
typo


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.h@694
PS13, Line 694: with the
  :   /// given schedule.
stale comment?


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:

http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.cc@422
PS13, Line 422: (
other places have a space before the opening (


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.cc@492
PS13, Line 492:  schedule, pool_cfg, cluster_size, 
not_admitted_reason)) {
nit: The indent looks strange, wrap the whole block in {} if the "if" condition 
doesn't fit into a single line?


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.cc@587
PS13, Line 587:   PrintBytes(max_mem), 
GetMaxMemForPoolDescription(pool_cfg, cluster_size));
nit: continue at 4 spaces (see above)


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.cc@1037
PS13, Line 1037:*schedule, pool_config, cluster_size, true, 
¬_admitted_reason)) {
nit: continue indent at 4 spaces


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.cc@1072
PS13, Line 1072: GetMaxRequestsForPoolDescription(pool_config, 
cluster_size),
nit: fix indent


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.cc@1083
PS13, Line 1083: DCHECK(queue_size_ratio <= 1.0);
I suspect that this DCHECK could fail if there's a local inrush of queries 
before the stats get updated. Should we change the max() in the line before to 
max(local_queued, agg_queued)?


http://gerrit.cloudera.org:8080/#/c/13307/13/be/src/scheduling/admission-controller.cc@1084
PS13, Line 1084: // TODO: Floating point arithmetic may result in dequeuing 
one less request than
I think this comment might be wrong/sta

[Impala-ASF-CR] [WIP] IMPALA-8630: Hash the full path when calculating consistent remote placement

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13545 )

Change subject: [WIP] IMPALA-8630: Hash the full path when calculating 
consistent remote placement
..


Patch Set 2:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/13545/2//COMMIT_MSG@27
PS2, Line 27: The alternative is to construct the DescriptorTbl in the scheduler
the advantage of the alternative is saving some extra memory per file 
descriptor. The FileDescs are persistent per-file so this can add up to many MB 
of increased consumption on a big catalogd. That said, with the recent work to 
reduce consumption and add eviction on catalogd, maybe it's not such a big deal 
as it used to be (eg IMPALA-7406 saved 100 bytes per FileFbDesc and here we're 
only losing a little bit of that gain)


http://gerrit.cloudera.org:8080/#/c/13545/2/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/13545/2/be/src/scheduling/scheduler.cc@801
PS2, Line 801:   uint32_t hash = 
HashUtil::Hash(&hdfs_file_split->full_path_hash,
 :   sizeof(hdfs_file_split->full_path_hash), 0);
why hash the hash instead of just making the hash here be a uint32_t directly?


http://gerrit.cloudera.org:8080/#/c/13545/2/common/fbs/CatalogObjects.fbs
File common/fbs/CatalogObjects.fbs:

http://gerrit.cloudera.org:8080/#/c/13545/2/common/fbs/CatalogObjects.fbs@82
PS2, Line 82: int
should be a uint?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
Gerrit-Change-Number: 13545
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 23:20:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] fe: improve logging for metadata loading

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: fe: improve logging for metadata loading
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4481/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8
Gerrit-Change-Number: 13463
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 14 Jun 2019 23:14:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] fe: improve logging for metadata loading

Hello Vihang Karajgaonkar, Impala Public Jenkins,

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

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

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

Change subject: fe: improve logging for metadata loading
..

fe: improve logging for metadata loading

This annotates various catalogd-internal calls associated with
refreshing and loading metadata with a 'String why' parameter, useful
for logging. This can help understand why a particular piece of metadata
was loaded, and in the case of REFRESH calls, who issued the original
refresh.

Additionally, some of the log statements were improved to add a bit of
extra detail such as the list of partitions being refreshed
(abbreviated) and whether or not the table schema is being refreshed.

Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8
---
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
M fe/src/main/java/org/apache/impala/catalog/HBaseTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/IncompleteTable.java
M fe/src/main/java/org/apache/impala/catalog/KuduTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/TableLoader.java
M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
M fe/src/test/java/org/apache/impala/catalog/CatalogdTableInvalidatorTest.java
M 
fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
18 files changed, 207 insertions(+), 144 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8
Gerrit-Change-Number: 13463
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar

[Impala-ASF-CR] [WIP] IMPALA-8630: Hash the full path when calculating consistent remote placement

Hello Tim Armstrong, Impala Public Jenkins,

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

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

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

Change subject: [WIP] IMPALA-8630: Hash the full path when calculating 
consistent remote placement
..

[WIP] IMPALA-8630: Hash the full path when calculating consistent remote 
placement

Consistent remote placement currently uses the relative filename within
a partition for the consistent hash. If the relative filenames for
different partitions have a simple naming scheme, then multiple
partitions may have files of the same name. This is true for some
tables written by Hive (e.g. in our minicluster the tpcds.store_sales
has this problem). This can lead to unbalanced placement of remote
ranges.

This adds a full_path_hash to the FbFileDesc and computes it in the catalog.
The THdfsFileSplit is based on the FbFileDesc, and this gets passed through.
The scheduler hashes this rather than the relative path.

This is a proof of concept and is not ready to go.

Work remaining (if this is the approach we take):
 - Add some scheduler tests to simulate this approach
 - Look into an end-to-end test based on the Docker tests (which do remote IO)

The alternative is to construct the DescriptorTbl in the scheduler
and use that to reconstruct the full path for a file and hash it.

Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
---
M be/src/scheduling/scheduler-test-util.cc
M be/src/scheduling/scheduler-test-util.h
M be/src/scheduling/scheduler-test.cc
M be/src/scheduling/scheduler.cc
M common/fbs/CatalogObjects.fbs
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
8 files changed, 86 insertions(+), 24 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I46c739fc31af539af2b3509e2a161f4e29f44d7b
Gerrit-Change-Number: 13545
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8542. Add an access trace for the data cache
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3632/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 23:08:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8583: Add metrics for Basic auth
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f
Gerrit-Change-Number: 13640
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 23:06:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8583: Add metrics for Basic auth
..

IMPALA-8583: Add metrics for Basic auth

The patch adds two metrics:
impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-success
impala.thrift-server.hiveserver2-http-frontend.total-basic-auth-failure
which track the number of successful and failed attempts at using
Basic auth to authentication to the hiveserver2 http server.

A followup patch will add similar metrics for SASL attempts on the
beeswax and hiveserver2 binary servers.

Testing:
- Modified existing HTTP auth test to check that the produced metrics
  are correct.

Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f
Reviewed-on: http://gerrit.cloudera.org:8080/13640
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
8 files changed, 111 insertions(+), 22 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f
Gerrit-Change-Number: 13640
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Todd Lipcon

[Impala-ASF-CR] fe: improve logging for metadata loading

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13463 )

Change subject: fe: improve logging for metadata loading
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13463/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/13463/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@a948
PS3, Line 948:
> having this and the one after fetchAllPartitions call seems useful to quick
OK. Planning on working on a nicer patch to expose these timings as more 
structured information and even back to the profile of the user who submitted 
it, but will just add these back for now.


http://gerrit.cloudera.org:8080/#/c/13463/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@103
PS3, Line 103:
> nit, unnecessary new line
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d90a5f075b05d2dc96692b3349abe35ce24b8b8
Gerrit-Change-Number: 13463
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 14 Jun 2019 22:58:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8542. Add an access trace for the data cache
..


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4480/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 8
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 22:26:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache

Hello Michael Ho, Lars Volker, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8542. Add an access trace for the data cache
..

IMPALA-8542. Add an access trace for the data cache

This adds a relatively simple JSON-formatted access trace for the data
cache feature. Each partition stores a trace file named 'trace.txt',
with each line representing a hit, miss, or store into the cache.

The trace is collected using the kudu::AsyncLogger class which handles
buffering and deferring the actual IO to a background thread.

By default, the full cache key info is written to the trace (including
the file paths), but a flag can enable anonymization (128-bit
city-hashing) of the file names in case any user would like to capture a
trace to be shared publically without divulging their table names.

Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
---
M be/src/runtime/io/data-cache-test.cc
M be/src/runtime/io/data-cache.cc
M be/src/runtime/io/data-cache.h
3 files changed, 297 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/13425/7
--
To view, visit http://gerrit.cloudera.org:8080/13425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 7
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Todd Lipcon

[Impala-ASF-CR] IMPALA-8459. LocalCatalog: Allow dropping tables that fail to load

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13557 )

Change subject: IMPALA-8459. LocalCatalog: Allow dropping tables that fail to 
load
..


Patch Set 3:

Ran a build locally with LocalCatalog enabled. I got two failures but they were 
both "Exception: ("DB {0} didn't show up after {1}s", 'hms_sanity_db', 30)" 
which is tracked under another JIRA.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia85d6b10669866dcc9f2bf7385a4775e483dd6b0
Gerrit-Change-Number: 13557
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 14 Jun 2019 22:19:05 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8459. LocalCatalog: Allow dropping tables that fail to load

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8459. LocalCatalog: Allow dropping tables that fail to 
load
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia85d6b10669866dcc9f2bf7385a4775e483dd6b0
Gerrit-Change-Number: 13557
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 14 Jun 2019 22:19:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8459. LocalCatalog: Allow dropping tables that fail to load

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8459. LocalCatalog: Allow dropping tables that fail to 
load
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4479/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia85d6b10669866dcc9f2bf7385a4775e483dd6b0
Gerrit-Change-Number: 13557
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Fri, 14 Jun 2019 22:19:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8599: Create a Maven module for query event hook API

2019-06-14 Thread Bharath Vissapragada (Code Review)

Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13653 )

Change subject: IMPALA-8599: Create a Maven module for query event hook API
..


Patch Set 2: Code-Review+1

(1 comment)

I'm not a mvn ninja, so I prefer an additional set of eyes on the pom contents.

http://gerrit.cloudera.org:8080/#/c/13653/2/query-event-hook-api/CMakeLists.txt
File query-event-hook-api/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/13653/2/query-event-hook-api/CMakeLists.txt@18
PS2, Line 18: DEPENDS gen-deps impala-parent
qq: why is this needed? Looking at the hook, seems like a standalone thing.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84c422d83c19b75c3d1d7a772b971f4f7704d44c
Gerrit-Change-Number: 13653
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 21:57:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Remove "Could not transfer" exclusion in mvn-quiet.sh

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: Remove "Could not transfer" exclusion in mvn-quiet.sh
..

Remove "Could not transfer" exclusion in mvn-quiet.sh

"Could not transfer" warning messages are noisy. However, excluding
"Could not transfer" words can lead to actual error messages that
contain "Could not transfer" to not be shown in the stdout, which can
make debugging difficult. This patch updates mvn-quiet.sh to show
"Could not transfer" messages.

Testing:
- Ran FE build

Change-Id: Ide3367fd98abbbe11eec1fa86fbad8b32eeecb8d
Reviewed-on: http://gerrit.cloudera.org:8080/13647
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M bin/mvn-quiet.sh
1 file changed, 1 insertion(+), 2 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ide3367fd98abbbe11eec1fa86fbad8b32eeecb8d
Gerrit-Change-Number: 13647
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong

[Impala-ASF-CR] Remove "Could not transfer" exclusion in mvn-quiet.sh

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: Remove "Could not transfer" exclusion in mvn-quiet.sh
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3367fd98abbbe11eec1fa86fbad8b32eeecb8d
Gerrit-Change-Number: 13647
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Jun 2019 21:50:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 22: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 22
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Jun 2019 21:03:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8617: Add support for lz4 in parquet
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3631/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 20:42:45 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8617: Add support for lz4 in parquet

2019-06-14 Thread Abhishek Rawat (Code Review)

Abhishek Rawat has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/13582 )

Change subject: IMPALA-8617: Add support for lz4 in parquet
..

IMPALA-8617: Add support for lz4 in parquet

A new enum value LZ4_BLOCKED was added to the THdfsCompression enum, to
distinguish it from the existing LZ4 codec. LZ4_BLOCKED codec represents
the block compression scheme used by Hadoop. Its similar to
SNAPPY_BLOCKED as far as the block format is concerned, with the only
difference being the codec used for compression and decompression.

Added Lz4BlockCompressor and Lz4BlockDecompressor classes for
compressing and decompressing parquet data using Hadoop's
lz4 block compression scheme.

The Lz4BlockCompressor treats the input
as a single block and generates a compressed block with following layout
  <4 byte big endian uncompressed size>
  <4 byte big endian compressed size>
  
The hdfs parquet table writer should call the Lz4BlockCompressor
using the ideal input size (unit of compression in parquet is a page),
and so the Lz4BlockCompressor does not further break down the input
into smaller blocks.

The Lz4BlockDecompressor on the other hand should be compatible with
blocks written by Impala and other engines in Hadoop ecosystem. It can
decompress compressed data in following format
  <4 byte big endian uncompressed size>
  <4 byte big endian compressed size>
  
  ...
  <4 byte big endian compressed size>
  
  ...
  

Externally users can now set the lz4 codec for parquet using:
  set COMPRESSION_CODEC=lz4
This gets translated into LZ4_BLOCKED codec for the
HdfsParquetTableWriter. Similarly, when reading lz4 compressed parquet
data, the LZ4_BLOCKED codec is used.

Testing:
 - Added unit tests for LZ4_BLOCKED in decompress-test.cc
 - Added unit tests for Hadoop compatibility in decompress-test.cc,
   basically being able to decompress an outer block with multiple inner
   blocks (the Lz4BlockDecompressor description above)
 - Added interoperability tests for Hive and Impala for all parquet
   codecs. New test added to
   tests/custom_cluster/test_hive_parquet_codec_interop.py

Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
---
M be/src/exec/parquet/hdfs-parquet-table-writer.cc
M be/src/exec/parquet/parquet-column-readers.cc
M be/src/exec/parquet/parquet-common.cc
M be/src/exec/parquet/parquet-metadata-utils.cc
M be/src/service/query-options-test.cc
M be/src/util/codec.cc
M be/src/util/compress.cc
M be/src/util/compress.h
M be/src/util/decompress-test.cc
M be/src/util/decompress.cc
M be/src/util/decompress.h
M common/thrift/CatalogObjects.thrift
M common/thrift/generate_error_codes.py
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/common/test_dimensions.py
A tests/custom_cluster/test_hive_parquet_codec_interop.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_parquet.py
18 files changed, 439 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/13582/7
--
To view, visit http://gerrit.cloudera.org:8080/13582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia6850a39ef3f1e0e7ba48e08eef1d4f7cbb74d0c
Gerrit-Change-Number: 13582
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon

[Impala-ASF-CR] IMPALA-8599: Create a Maven module for query event hook API

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8599: Create a Maven module for query event hook API
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3630/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84c422d83c19b75c3d1d7a772b971f4f7704d44c
Gerrit-Change-Number: 13653
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 19:43:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8542. Add an access trace for the data cache
..


Patch Set 6: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 19:38:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8599: Create a Maven module for query event hook API

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8599: Create a Maven module for query event hook API
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4478/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84c422d83c19b75c3d1d7a772b971f4f7704d44c
Gerrit-Change-Number: 13653
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 19:02:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8599: Create a Maven module for query event hook API

2019-06-14 Thread Fredy Wijaya (Code Review)

Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13653


Change subject: IMPALA-8599: Create a Maven module for query event hook API
..

IMPALA-8599: Create a Maven module for query event hook API

This patch moves the query event event hook API into a separate Maven
module so that it can be easily consumed by other projects, such as
Atlas.

Testing:
- Ran make fe

Change-Id: I84c422d83c19b75c3d1d7a772b971f4f7704d44c
---
M CMakeLists.txt
M fe/CMakeLists.txt
M fe/pom.xml
M impala-parent/pom.xml
A query-event-hook-api/CMakeLists.txt
A query-event-hook-api/pom.xml
R 
query-event-hook-api/src/main/java/org/apache/impala/hooks/QueryCompleteContext.java
R query-event-hook-api/src/main/java/org/apache/impala/hooks/QueryEventHook.java
8 files changed, 92 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I84c422d83c19b75c3d1d7a772b971f4f7704d44c
Gerrit-Change-Number: 13653
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya

[Impala-ASF-CR] IMPALA-7802: Close connections of idle client sessions

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

Change subject: IMPALA-7802: Close connections of idle client sessions
..


Patch Set 1:

(5 comments)

Took another quick look in light of the clarifications

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

http://gerrit.cloudera.org:8080/#/c/13607/1//COMMIT_MSG@30
PS1, Line 30:
> It's the former for sure. Will update commit message to make it clearer.
I do wonder if we should try to handle the specific case where a connection is 
established and nothing is ever sent. I know we've seen some clients do things 
like this before, e.g. when load balancers are involved. But maybe I'm 
misremembering. I'm only thinking this because it seems like it might be 
straightforward. But yeah, I guess this opens a potential can of worms since it 
isn't just an extension of the old session timeout logic.


http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/service/impala-server.cc@2072
PS1, Line 2072: if (it == connection_to_sessions_map_.end()) return false;
What happens if the entry is present but the set is empty? Shouldn't that 
behave the same as the entry not being present? If it's an invariant that we 
don't have empty sets in the map, we should add a DCHECK.

Maybe l2020 also should be changed, although I think it doesn't make a 
behavioural difference tehre.


http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/service/impala-server.cc@2074
PS1, Line 2074: session_ids = connection_to_sessions_map_[connection_id];
Why not session_ids = it->second? To avoid the double-lookup.


http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/service/impala-server.cc@2086
PS1, Line 2086:   std::shared_ptr session_state = 
map_entry->second;
Minor thing, but this copy increments the refcount, and I don't think it's 
necessary, since we're holding the session_state_map_lock_ the whole time. OK 
to ignore.


http://gerrit.cloudera.org:8080/#/c/13607/1/tests/custom_cluster/test_session_expiration.py
File tests/custom_cluster/test_session_expiration.py:

http://gerrit.cloudera.org:8080/#/c/13607/1/tests/custom_cluster/test_session_expiration.py@122
PS1, Line 122:   def test_closing_idle_connection(self, vector):
> Should we also check that sockets that *don't* create a session get closed.
I guess it's still expected that these will stay open, so maybe we don't need 
to do this (or we could test explicitly that they don't get close).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I97c4fb8e1b741add273f8a913fb0967303683e38
Gerrit-Change-Number: 13607
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Anonymous Coward (443)
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 14 Jun 2019 18:59:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7802: Close connections of idle client sessions

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

Change subject: IMPALA-7802: Close connections of idle client sessions
..


Patch Set 1:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/13607/1//COMMIT_MSG@28
PS1, Line 28: If so, the connection will be closed. This allows the service 
threads
> If there are no open sessions associated with a connection, the connection_
Ah ok, I missed that detail when doing my initial pass over the code. The 
clarification helps.


http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:

http://gerrit.cloudera.org:8080/#/c/13607/1/be/src/rpc/TAcceptQueueServer.cpp@78
PS1, Line 78: if (!processor_->process(input_, output_, 
connectionContext) ||
> Yes but that's not the intention of this patch to address those cases. It's
Ok, makes sense. Maybe leave a comment here to explain that?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I97c4fb8e1b741add273f8a913fb0967303683e38
Gerrit-Change-Number: 13607
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Anonymous Coward (443)
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Fri, 14 Jun 2019 18:35:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-14 Thread Thomas Marshall (Code Review)

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

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 22: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12884/22/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/12884/22/shell/impala_shell.py@1178
PS22, Line 1178:   # self.close_connection()
?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 22
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Jun 2019 18:09:29 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8583: Add metrics for Basic auth
..


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4477/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f
Gerrit-Change-Number: 13640
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 17:35:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8583: Add metrics for Basic auth

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8583: Add metrics for Basic auth
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d1b1bcc093e4b00802bc108ff4d8d4e2cdfd88f
Gerrit-Change-Number: 13640
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 17:35:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8636: Implement INSERT for insert-only ACID tables

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8636: Implement INSERT for insert-only ACID tables
..


Patch Set 5:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3629/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c36fa6902676f06b4e38730f737becfc7c06ad
Gerrit-Change-Number: 13559
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 14 Jun 2019 17:19:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8636: Implement INSERT for insert-only ACID tables

2019-06-14 Thread Zoltan Borok-Nagy (Code Review)

Hello Impala Public Jenkins,

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

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

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

Change subject: IMPALA-8636: Implement INSERT for insert-only ACID tables
..

IMPALA-8636: Implement INSERT for insert-only ACID tables

This commit adds INSERT support for insert-only ACID tables.

The Frontend opens a transaction for queries that refer to
transactional tables. For INSERT statements that write insert-only
ACID tables it also allocates a write ID. The Frontend aborts the
transaction if an error occurs during analysis/planning.

The Backend gets the transaction id in TExecRequestState and the
write id is set for the HDFS table sinks. The sinks write the files
at their final destination which is an ACID base/delta directory.
There is no need for finalization of transactional INSERTS.

ClientRequestState commits the transaction in WaitInternal() if
everything went well. If the transaction is still open in Done(), it
means there was an error, therefore the transaction needs to be aborted.

The Backend commits/aborts the transaction by calling the Frontend via
JNI.

Testing:
* added new tables during dataload
* added acid-insert.test file with INSERT statements against the new
  tables
* added integration test with Hive to test_hms_integration.py. The test
  inserts data with Impala and reads with Hive. (These integration
  tests only run with exhaustive exploration strategy)

TODO in following commits:
* add locks and heartbeats
* implement TRUNCATE (maybe in another commit)
* CTAS creates files in the 'root' directory of the table/partition. It
  is handled correctly during SELECT, but would be better to create a
  base directory from the beginning.

Change-Id: Id6c36fa6902676f06b4e38730f737becfc7c06ad
---
M be/src/exec/hdfs-table-sink.cc
M be/src/exec/hdfs-table-sink.h
M be/src/runtime/coordinator.cc
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/util/jni-util.h
M common/thrift/DataSinks.thrift
M common/thrift/Frontend.thrift
M common/thrift/ImpalaInternalService.thrift
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
A fe/src/main/java/org/apache/impala/common/TransactionException.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
A testdata/workloads/functional-query/queries/QueryTest/acid-insert.test
M tests/metadata/test_hms_integration.py
M tests/query_test/test_insert.py
25 files changed, 662 insertions(+), 119 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id6c36fa6902676f06b4e38730f737becfc7c06ad
Gerrit-Change-Number: 13559
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] Remove "Could not transfer" exclusion in mvn-quiet.sh

2019-06-14 Thread Fredy Wijaya (Code Review)

Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13647 )

Change subject: Remove "Could not transfer" exclusion in mvn-quiet.sh
..


Patch Set 1:

> Patch Set 1:
>
> We could also consider removing the whole error suppression thing. I added it 
> a long time ago when I was trying to make Jenkins logs less noisy but I'm now 
> not sure that it was a good idea.

The current mvn-quiet.sh doesn't suppress error though. So, I think we're good 
here.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3367fd98abbbe11eec1fa86fbad8b32eeecb8d
Gerrit-Change-Number: 13647
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Jun 2019 16:25:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] Remove "Could not transfer" exclusion in mvn-quiet.sh

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: Remove "Could not transfer" exclusion in mvn-quiet.sh
..


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4476/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3367fd98abbbe11eec1fa86fbad8b32eeecb8d
Gerrit-Change-Number: 13647
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Jun 2019 16:25:52 +
Gerrit-HasComments: No

[Impala-ASF-CR] Remove "Could not transfer" exclusion in mvn-quiet.sh

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: Remove "Could not transfer" exclusion in mvn-quiet.sh
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide3367fd98abbbe11eec1fa86fbad8b32eeecb8d
Gerrit-Change-Number: 13647
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Jun 2019 16:25:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8542. Add an access trace for the data cache
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4475/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 16:16:49 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-8542. Add an access trace for the data cache
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 6
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 14 Jun 2019 16:16:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6590: Disable expr rewrites and codegen for VALUES() statements

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

Change subject: IMPALA-6590: Disable expr rewrites and codegen for VALUES()
statements
..

Patch Set 1:

(7 comments)

The approach makes sense.

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

http://gerrit.cloudera.org:8080/#/c/13645/1//COMMIT_MSG@38
PS1, Line 38: Results below.
I think we should have some frontend tests for the expression rewrite part of
this. I don't think anything too elaborate, but just queries with values
clauses in a couple of positions (e.g. a simple INSERT, and maybe a VALUE
clause as a subquery in a select), with expressions that would have otherwise
been rewritten.

I think that would fit in
./fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java

http://gerrit.cloudera.org:8080/#/c/13645/1/be/src/exec/union-node.h
File be/src/exec/union-node.h:

http://gerrit.cloudera.org:8080/#/c/13645/1/be/src/exec/union-node.h@70
PS1, Line 70: /// Number of const scalar expressions which will be codegened.
Can you mention that this is only used for observability? Or something along
those lines.

It gets confusing otherwise with all the state variables - it's hard to
understand what actually influences the control flow, etc.

http://gerrit.cloudera.org:8080/#/c/13645/1/be/src/exprs/scalar-expr.h
File be/src/exprs/scalar-expr.h:

http://gerrit.cloudera.org:8080/#/c/13645/1/be/src/exprs/scalar-expr.h@402
PS1, Line 402: bool is_codegen_disabled_;
Could be const since it always initialised in the constructor.

http://gerrit.cloudera.org:8080/#/c/13645/1/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

http://gerrit.cloudera.org:8080/#/c/13645/1/be/src/runtime/runtime-state.h@155
PS1, Line 155: uint32_t
Please use an int64_t - the style guide says to stick to signed integers (with
a few exceptions)
https://google.github.io/styleguide/cppguide.html#Integer_Types

http://gerrit.cloudera.org:8080/#/c/13645/1/common/thrift/Exprs.thrift
File common/thrift/Exprs.thrift:

http://gerrit.cloudera.org:8080/#/c/13645/1/common/thrift/Exprs.thrift@153
PS1, Line 153: // If codegen is disabled for this Expr
I was thinking about whether it would be better to invert the meaning of this
field, i.e. make it is_codegen_enabled, since it's often easier to follow the
code - you don't have to parse double negatives like !is_codegen_disabled.

I think this is OK, particularly since "codegen disabled" is used pretty
pervasively in Impala.

http://gerrit.cloudera.org:8080/#/c/13645/1/common/thrift/Exprs.thrift@154
PS1, Line 154: 5: required bool is_codegen_disabled
I would avoid renumbering the fields. you could just make this 22.

It doesn't *really* matter for this struct since it's only used internally
between daemons running the same version, but renumbering thrift fields does
break binary compatibility, with weird errors. So it's a bad habit to get into,
I think.

http://gerrit.cloudera.org:8080/#/c/13645/1/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/13645/1/fe/src/main/java/org/apache/impala/analysis/Expr.java@400
PS1, Line 400: for (Expr child: children_) {child.disableCodegen();}
Non-standard formatting. If we use braces, we generally just put the statement
on multiple lines.

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I229d67b821968321abd8f97f7c89cf2617000d8d
Gerrit-Change-Number: 13645
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Rawat
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Fri, 14 Jun 2019 16:05:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7369: part 2: Add INTERVAL expr support and built-in functions for DATE

2019-06-14 Thread Csaba Ringhofer (Code Review)

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

Change subject: IMPALA-7369: part 2: Add INTERVAL expr support and built-in 
functions for DATE
..


Patch Set 1:

(7 comments)

The change seems ok to me in general, congrats for the great test coverage.

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

http://gerrit.cloudera.org:8080/#/c/13648/1//COMMIT_MSG@105
PS1, Line 105: 1
This should be 0 according to the link.


http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/exprs/expr-test.cc@
PS1, Line :   // 2019-01-01 is Tuesday, it belongs to the first wek of the 
year.
typo: week


http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/exprs/expr-test.cc@7778
PS1, Line 7778:   TestValue("weekofyear(date '2018-12-31')", TYPE_INT, 1);
The comment for WeekOfYear() states that it can return values 1-53. Can you add 
a test where it returns 53? For example select weekofyear("2015-12-31"); 
returns 53 in Hive.


http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-test.cc
File be/src/runtime/date-test.cc:

http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-test.cc@845
PS1, Line 845:   // 2019-12-30 (Monday) and 2019-12-31 (Tuesday) belong to year 
2020.
Can you add a test with week number 53? See expr-test.cc for details.


http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-test.cc@910
PS1, Line 910:   EXPECT_TRUE(DateValue(2016, 2, 29).MonthsBetween(
 :   DateValue(2016, 1, 29), &months_between));
 :   EXPECT_EQ(1, months_between);
optional: a helper function like TestMonthsBetween(DateValue dt1, DateValue 
dt2, int min_expected, int max_expected) could make this function less verbose


http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-test.cc@936
PS1, Line 936: monrhs_between
typo: months_between


http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-value.h
File be/src/runtime/date-value.h:

http://gerrit.cloudera.org:8080/#/c/13648/1/be/src/runtime/date-value.h@121
PS1, Line 121: AddMonths
I would prefer to move this to .cc / inline.h
'keep_last_day' could be also an argument, I don't think that we really benefit 
from it being a template.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If404bffdaf055c769e79ffa8f193bac415cfdd1a
Gerrit-Change-Number: 13648
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Fri, 14 Jun 2019 15:50:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7290: part 2: Add HS2 support to Impala shell

2019-06-14 Thread Impala Public Jenkins (Code Review)

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

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
..


Patch Set 22:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4474/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 22
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 14 Jun 2019 15:34:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] Remove "Could not transfer" exclusion in mvn-quiet.sh