[Impala-ASF-CR] IMPALA-7039: Ignore the port in HBase planner tests

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10459 )

Change subject: IMPALA-7039: Ignore the port in HBase planner tests
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8eb7628061b2ebaf84323b37424925e9a64f70a0
Gerrit-Change-Number: 10459
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 May 2018 03:43:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-2751: Matching quotes are not requirerd in comments

2018-05-21 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10474


Change subject: IMPALA-2751: Matching quotes are not requirerd in comments
..

IMPALA-2751: Matching quotes are not requirerd in comments

This patch fixes the issue where non-matching quotes inside comments
will cause the shell to not terminate.

The fix is to strip any SQL comments before sending to shlex since shlex
does not understand SQL comments and will raise an exception when it sees
unmatched quotes regardless whether the quotes are in the comments or
not.

Testing:
- Added new shell tests
- Ran all end-to-end shell tests

Change-Id: Ic899fdddc182947f73101ddbc2e3c8caf97d9085
---
M shell/impala_shell.py
M tests/shell/test_shell_interactive.py
2 files changed, 25 insertions(+), 5 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic899fdddc182947f73101ddbc2e3c8caf97d9085
Gerrit-Change-Number: 10474
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 


[Impala-ASF-CR] IMPALA-7039: Ignore the port in HBase planner tests

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10459 )

Change subject: IMPALA-7039: Ignore the port in HBase planner tests
..

IMPALA-7039: Ignore the port in HBase planner tests

Before this patch, we used to check the HBase port in the HBase planner
tests. This caused a failure when HBase was running on a different port
than expected. We fix the problem in this patch by not checking the
HBase port.

Testing: ran the FE tests and they passed.

Change-Id: I8eb7628061b2ebaf84323b37424925e9a64f70a0
Reviewed-on: http://gerrit.cloudera.org:8080/10459
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M testdata/workloads/functional-planner/queries/PlannerTest/hbase.test
M testdata/workloads/functional-planner/queries/PlannerTest/joins.test
3 files changed, 20 insertions(+), 23 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8eb7628061b2ebaf84323b37424925e9a64f70a0
Gerrit-Change-Number: 10459
Gerrit-PatchSet: 2
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7051: Serialize Maven invocations.

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10460 )

Change subject: IMPALA-7051: Serialize Maven invocations.
..

IMPALA-7051: Serialize Maven invocations.

I've observed some rare cases where Impala fails to build. I believe
it's because two Maven targets (yarn-extras and ext-data-source) are
being executed simultaneously. Maven's handling of ~/.m2/repository,
for example, is known to be not safe.

This patch serializes the Maven builds with the following
dependency graph:
  fe -> yarn-extras -> ext-data-source -> impala-parent
The ordering of yarn-extras -> ext-data-source is arbitrary.

I decided that this artificial dependency was the clearest
way to prevent parallel executions. Having mvn-quiet.sh
take a lock seemed considerably more complex.

Change-Id: Ie24f34f421bc7dcf9140938464d43400da95275e
Reviewed-on: http://gerrit.cloudera.org:8080/10460
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M common/yarn-extras/CMakeLists.txt
1 file changed, 4 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie24f34f421bc7dcf9140938464d43400da95275e
Gerrit-Change-Number: 10460
Gerrit-PatchSet: 2
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7051: Serialize Maven invocations.

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10460 )

Change subject: IMPALA-7051: Serialize Maven invocations.
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24f34f421bc7dcf9140938464d43400da95275e
Gerrit-Change-Number: 10460
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 May 2018 02:23:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5737: Tighten minicluster memory limit

2018-05-21 Thread Tianyi Wang (Code Review)
Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10277 )

Change subject: IMPALA-5737: Tighten minicluster memory limit
..


Patch Set 7:

There is now a blue docker build: 
https://jenkins.impala.io/job/test-with-docker-parameterized/15/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If8240551e726c6da546a926a1ce3444f41ef87fe
Gerrit-Change-Number: 10277
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 22 May 2018 01:19:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10413 )

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..

IMPALA-7019: Schedule EC as remote & disable failed tests

This patch schedules HDFS EC files without considering locality. Failed
tests are disabled and a jenkins build should succeed with export
ERASURE_COINDG=true.

Testing: It passes core tests.

Cherry-picks: not for 2.x.

Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Reviewed-on: http://gerrit.cloudera.org:8080/10413
Reviewed-by: Taras Bobrovytsky 
Tested-by: Impala Public Jenkins 
---
M common/fbs/CatalogObjects.fbs
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M tests/common/skip.py
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_hdfs_fd_caching.py
M tests/metadata/test_explain.py
M tests/query_test/test_hdfs_caching.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_parquet.py
M tests/query_test/test_mt_dop.py
M tests/query_test/test_nested_types.py
M tests/query_test/test_queries.py
M tests/query_test/test_query_mem_limit.py
M tests/query_test/test_scanners.py
M tests/util/filesystem_utils.py
17 files changed, 75 insertions(+), 28 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 6
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 


[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10413 )

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 5
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Tue, 22 May 2018 01:10:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6953: clean up DiskIoMgr

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10245 )

Change subject: IMPALA-6953: clean up DiskIoMgr
..


Patch Set 7:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr-internal.h
File be/src/runtime/io/disk-io-mgr-internal.h:

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr-internal.h@73
PS7, Line 73:   void DiskThreadLoop(DiskIoMgr* io_mgr);
The main worker loop is now in DiskQueue rather than DiskIoMgr, but the logic 
didn't change.


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr-internal.h@89
PS7, Line 89:   void ShutDown();
The ShutDown() logic did change to use a local shut_down_ variable 
per-DiskQueue rather than a DiskIoMgr member variable.


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

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr.cc@493
PS7, Line 493: RequestRange* DiskQueue::GetNextRequestRange(RequestContext** 
request_context) {
This function was broken up into multiple functions, but the logic should be 
equivalent.


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.h
File be/src/runtime/io/request-context.h:

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.h@496
PS7, Line 496: };
> is there a way to coerce this diff to show more what has actually changed?
The non-whitespace diff for PerDiskState is below. The only substantive 
difference is that IncrementDiskThreadAfterDequeue() is now wrapped by 
RequestContext

  @@ -1,6 +1,6 @@
  -  /// Struct containing state per disk. See comments in the disk read loop 
on how
  -  /// they are used.
  -  class PerDiskState {
  +/// Struct containing state per disk. See comments in the disk read loop on 
how
  +/// they are used.
  +class RequestContext::PerDiskState {
  public:
   bool done() const { return done_; }
   void set_done(bool b) { done_ = b; }
  @@ -38,10 +38,7 @@
   void ScheduleContext(const boost::unique_lock& 
context_lock,
   RequestContext* context, int disk_id);

  -/// Called when dequeueing this RequestContext from the disk queue to 
increment the
  -/// count of disk threads with a reference to this context. These 
threads do not hold
  -/// any locks while reading from HDFS, so we need to prevent the 
RequestContext from
  -/// being destroyed underneath them.
  +  /// See RequestContext::IncrementDiskThreadAfterDequeue() comment for 
usage.
   ///
   /// The caller does not need to hold 'lock_', so this can execute 
concurrently with
   /// itself and DecrementDiskThread().
  @@ -152,4 +149,4 @@
   /// GetNextRequestRange() and GetNextUnstartedRange() may result in only 
reads being
   /// processed)
   InternalQueue unstarted_write_ranges_;
  -  };
  +};


  > };


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.h@498
PS7, Line 498: inline void RequestContext::IncrementDiskThreadAfterDequeue(int 
disk_id) {
These functions got pulled out of the class body, but the logic shouldn't have 
changed.


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.cc
File be/src/runtime/io/request-context.cc:

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.cc@a27
PS7, Line 27:
This code got moved to scan-range.cc but the logic didn't change.


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.cc@a57
PS7, Line 57:
Moved to BufferDescriptor::Free()


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.cc@216
PS7, Line 216: RequestRange* RequestContext::GetNextRequestRange(int disk_id) {
This was broken out of DiskIoMgr::GetNextRequestRange(). The logic should be 
preserved.


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-ranges.h
File be/src/runtime/io/request-ranges.h:

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-ranges.h@291
PS7, Line 291:   /// Reads from the DN cache. On success, sets cached_buffer_ 
to the DN buffer
These functions were moved from private: to protected: but not changed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a6e393f8c01d10143cbac91108af37f6498c1b1
Gerrit-Change-Number: 10245
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 May 2018 00:29:37 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7039: Ignore the port in HBase planner tests

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10459 )

Change subject: IMPALA-7039: Ignore the port in HBase planner tests
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8eb7628061b2ebaf84323b37424925e9a64f70a0
Gerrit-Change-Number: 10459
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 May 2018 00:19:01 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-05-21 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6023/15/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/6023/15/fe/src/main/java/org/apache/impala/analysis/Expr.java@490
PS15, Line 490:   result = ((ScalarType)result).getMinResolutionDecimal();
Why do we need this?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Tue, 22 May 2018 00:17:54 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6953: clean up DiskIoMgr

2018-05-21 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10245 )

Change subject: IMPALA-6953: clean up DiskIoMgr
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.h
File be/src/runtime/io/request-context.h:

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.h@496
PS7, Line 496: };
is there a way to coerce this diff to show more what has actually changed? if 
not, Tim, maybe you could comment on what if anything has changed in any of the 
large code movement locations?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a6e393f8c01d10143cbac91108af37f6498c1b1
Gerrit-Change-Number: 10245
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 May 2018 00:06:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS based table

2018-05-21 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9966 )

Change subject: IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS 
based table
..


Patch Set 2:

> Can we add a basic custom cluster test for --use_hdfs_pread flag?
 > It really should have been added as part of the IMPALA-4740 patch.
 >
 > That doesn't test the hedged read support, but at least exercises
 > the pread code path automatically.

Thanks for the suggestion, Tim. I added a test that does a simple scan while 
configured to use preads.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48fe80dfd9a1ed68a8f2b7038e5f42b5a3df3baa
Gerrit-Change-Number: 9966
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:54:58 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS based table

2018-05-21 Thread Sailesh Mukil (Code Review)
Hello Philip Zeyliger, Tim Armstrong,

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

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

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

Change subject: IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS 
based table
..

IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS based table

We realized that the libHDFS API call hdfsGetHedgedReadMetrics() crashes
when the 'fs' argument passed to it is not a HDFS filesystem.

There is an open bug for it on the HDFS side: HDFS-13417
However, it looks like we won't be getting a fix for it in the short term,
so our only option at this point is to skip it.

Testing: Made sure that enabling preads and scanning from S3 doesn't
cause a crash.
Also, added a custom cluster test to exercise the pread code path. We
are unable to verify hedged reads in a minicluster, but we can at least
exercise the code path to make sure that nothing breaks.

Change-Id: I48fe80dfd9a1ed68a8f2b7038e5f42b5a3df3baa
---
M be/src/runtime/io/scan-range.cc
A tests/custom_cluster/test_hedged_reads.py
2 files changed, 33 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I48fe80dfd9a1ed68a8f2b7038e5f42b5a3df3baa
Gerrit-Change-Number: 9966
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6131: Track time of last statistics update in metadata

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10116 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 16:

This change did not cherrypick successfully into branch 2.x. To resolve this, 
please do the cherry-pick manually and submit it to Gerrit at refs/for/2.x or 
add an exception to the branch 2.x copy of bin/ignored_commits.json. Thanks, 
your friendly bot at https://jenkins.impala.io/job/cherrypick-2.x-and-test/524/ 
.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59a671ac29d352bd92ce40d5cb6662bb23f146b5
Gerrit-Change-Number: 10116
Gerrit-PatchSet: 16
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 21 May 2018 23:52:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Added the hotspot analysis to Performance Best Practices doc

2018-05-21 Thread Alex Rodoni (Code Review)
Alex Rodoni has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10472


Change subject: [DOCS] Added the hotspot analysis to Performance Best Practices 
doc
..

[DOCS] Added the hotspot analysis to Performance Best Practices doc

Change-Id: Iffc96004aea121e73065466a39b012a0d6556095
---
M docs/topics/impala_perf_cookbook.xml
1 file changed, 59 insertions(+), 10 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iffc96004aea121e73065466a39b012a0d6556095
Gerrit-Change-Number: 10472
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 


[Impala-ASF-CR] IMPALA-7011: Simplify PlanRootSink control logic

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10449 )

Change subject: IMPALA-7011: Simplify PlanRootSink control logic
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Gerrit-Change-Number: 10449
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:50:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7011: Simplify PlanRootSink control logic

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10449 )

Change subject: IMPALA-7011: Simplify PlanRootSink control logic
..

IMPALA-7011: Simplify PlanRootSink control logic

1) The eos_ and sender_done_ bits really encode three possible states
   that the sender can be in. Make this explicit using an enum with
   three values.

2) The purpose of CloseConsumer() has changed over time and we can clean
   this up now:

 a) Originally, it looks like it was used to unblock the sender when the
   consumer finishes before eos, but also keep the sink alive long
   enough for the coordinator. This is no longer necessary now that
   control structures are owned by the QueryState whose lifetime is
   controlled by a reference count taken by the coordinator. So, we don't
   need the coordinator to tell the sink it's done calling it and we
   don't need the consumer_done_ state.

 b) Later on, CloseConsumer() was used as a cancellation mechinism.
   We need to keep this around (or use timeouts on the condvars) to kick
   both the consumer and producer on cancellation. But let's make the
   cancellation logic similar to the exec nodes and other sinks by
   driving the cancellation using the RuntimeState's cancellation
   flag. Now that CloseConsumer() is only about cancellation, rename it
   to Cancel() (later we may promote it to DataSink and implement in the
   data stream sender as well).

Testing:
- Exhaustive
- Minicluster concurrent_select.py stress

Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Reviewed-on: http://gerrit.cloudera.org:8080/10449
Reviewed-by: Dan Hecht 
Tested-by: Impala Public Jenkins 
---
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/runtime/coordinator.cc
M be/src/runtime/fragment-instance-state.cc
4 files changed, 60 insertions(+), 68 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Gerrit-Change-Number: 10449
Gerrit-PatchSet: 8
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6023 )

Change subject: IMPALA-4848: Add WIDTH_BUCKET() function
..


Patch Set 15:

We should find a reviewer for this, or maybe Taras can +2 if he's comfortable 
with it.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Mon, 21 May 2018 23:43:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7016: Implement ALTER DATABASE SET OWNER

2018-05-21 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/10471 )

Change subject: IMPALA-7016: Implement ALTER DATABASE SET OWNER
..

IMPALA-7016: Implement ALTER DATABASE SET OWNER

Alters the database owner to either user or role.

Syntax:
ALTER DATABASE db SET OWNER USER user
ALTER DATABASE db SET OWNER ROLE role

Testing:
- Added new front-end tests
- Added new end-to-end tests

Change-Id: Ie3b923021ebce5192d2d64784e7ddb952ba82bc3
---
M common/thrift/CatalogService.thrift
M common/thrift/JniCatalog.thrift
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
A fe/src/main/java/org/apache/impala/analysis/AlterDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
A fe/src/main/java/org/apache/impala/analysis/OwnerType.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M tests/metadata/test_ddl.py
M tests/metadata/test_ddl_base.py
15 files changed, 322 insertions(+), 17 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie3b923021ebce5192d2d64784e7ddb952ba82bc3
Gerrit-Change-Number: 10471
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] [DOCS] Fixed misleading documentation on Impala + HDFS caching

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10454 )

Change subject: [DOCS] Fixed misleading documentation on Impala + HDFS caching
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63cd1ff7b885a094a4a3e91c31101d25414b4db7
Gerrit-Change-Number: 10454
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:41:48 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Fixed misleading documentation on Impala + HDFS caching

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10454 )

Change subject: [DOCS] Fixed misleading documentation on Impala + HDFS caching
..

[DOCS] Fixed misleading documentation on Impala + HDFS caching

Change-Id: I63cd1ff7b885a094a4a3e91c31101d25414b4db7
Reviewed-on: http://gerrit.cloudera.org:8080/10454
Reviewed-by: Tim Armstrong 
Tested-by: Impala Public Jenkins 
---
M docs/topics/impala_perf_hdfs_caching.xml
1 file changed, 5 insertions(+), 8 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I63cd1ff7b885a094a4a3e91c31101d25414b4db7
Gerrit-Change-Number: 10454
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-7016: Implement ALTER DATABASE SET OWNER

2018-05-21 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10471


Change subject: IMPALA-7016: Implement ALTER DATABASE SET OWNER
..

IMPALA-7016: Implement ALTER DATABASE SET OWNER

Alters the database owner to either user or role.

Syntax:
ALTER DATABASE db SET OWNER USER user
ALTER DATABASE db SET OWNER ROLE role

Testing:
- Added new front-end tests
- Added new end-to-end tests

Change-Id: Ie3b923021ebce5192d2d64784e7ddb952ba82bc3
---
M common/thrift/CatalogService.thrift
M common/thrift/JniCatalog.thrift
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
A fe/src/main/java/org/apache/impala/analysis/AlterDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
A fe/src/main/java/org/apache/impala/analysis/OwnerType.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M tests/metadata/test_ddl.py
M tests/metadata/test_ddl_base.py
15 files changed, 322 insertions(+), 17 deletions(-)



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

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


[Impala-ASF-CR] IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS based table

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9966 )

Change subject: IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS 
based table
..


Patch Set 1:

Can we add a basic custom cluster test for --use_hdfs_pread flag? It really 
should have been added as part of the IMPALA-4740 patch.

That doesn't test the hedged read support, but at least exercises the pread 
code path automatically.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48fe80dfd9a1ed68a8f2b7038e5f42b5a3df3baa
Gerrit-Change-Number: 9966
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:40:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Fixed misleading documentation on Impala + HDFS caching

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10454 )

Change subject: [DOCS] Fixed misleading documentation on Impala + HDFS caching
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63cd1ff7b885a094a4a3e91c31101d25414b4db7
Gerrit-Change-Number: 10454
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:33:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] [DOCS] Fixed misleading documentation on Impala + HDFS caching

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10454 )

Change subject: [DOCS] Fixed misleading documentation on Impala + HDFS caching
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63cd1ff7b885a094a4a3e91c31101d25414b4db7
Gerrit-Change-Number: 10454
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:32:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6953: clean up DiskIoMgr

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10245 )

Change subject: IMPALA-6953: clean up DiskIoMgr
..


Patch Set 7:

(1 comment)

Rebase. Do you have time to take a look Joe? Otherwise I can find another 
reviewer.

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

http://gerrit.cloudera.org:8080/#/c/10245/4/be/src/runtime/io/disk-io-mgr.h@407
PS4, Line 407:   }
Still need to do these.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a6e393f8c01d10143cbac91108af37f6498c1b1
Gerrit-Change-Number: 10245
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:27:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6020: [DOCS] REFRESH statement cannot detect HDFS block movement

2018-05-21 Thread Alex Rodoni (Code Review)
Alex Rodoni has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10470


Change subject: IMPALA-6020: [DOCS] REFRESH statement cannot detect HDFS block 
movement
..

IMPALA-6020: [DOCS] REFRESH statement cannot detect HDFS block movement

Change-Id: Ib6de1469e0f375511fd415e79074f4ec72cf109b
---
M docs/topics/impala_new_features.xml
1 file changed, 0 insertions(+), 7 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib6de1469e0f375511fd415e79074f4ec72cf109b
Gerrit-Change-Number: 10470
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 


[Impala-ASF-CR] IMPALA-5392: Added all stack frames to ThreadInfo summary.

2018-05-21 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10145 )

Change subject: IMPALA-5392: Added all stack frames to ThreadInfo summary.
..


Patch Set 9:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10145/9/fe/src/main/java/org/apache/impala/common/JniUtil.java
File fe/src/main/java/org/apache/impala/common/JniUtil.java:

http://gerrit.cloudera.org:8080/#/c/10145/9/fe/src/main/java/org/apache/impala/common/JniUtil.java@263
PS9, Line 263:  " Id=" + 
threadInfo.getThreadId() + " " +
nit: we use 4 spaces for continued indentation.


http://gerrit.cloudera.org:8080/#/c/10145/9/fe/src/main/java/org/apache/impala/common/JniUtil.java@270
PS9, Line 270:   "\" Id=" + threadInfo.getLockOwnerId());
nit: we use 4 spaces for continued indentation.


http://gerrit.cloudera.org:8080/#/c/10145/9/fe/src/main/java/org/apache/impala/common/JniUtil.java@285
PS9, Line 285:   if(miList == null){
nit: space after if and space before {



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I80ab4aad03e0c1f01fecad6b87779531244c28b7
Gerrit-Change-Number: 10145
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Sharma 
Gerrit-Reviewer: Abhishek Sharma 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Charles Agnello 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:07:06 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6662: Make stress test resilient to hangs due to client crashes

2018-05-21 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9635 )

Change subject: IMPALA-6662: Make stress test resilient to hangs due to client 
crashes
..


Patch Set 6:

> What's the next step here?

This patch fixes the hang issue, but causes more timeouts for some reason. I'd 
dropped this intermittently, but I'll get back to investigating it soon.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I10c5dc9b8c2fffc471bac2279e348bc1d1fec3b7
Gerrit-Change-Number: 9635
Gerrit-PatchSet: 6
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 21 May 2018 23:03:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5392: Added all stack frames to ThreadInfo summary.

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10145 )

Change subject: IMPALA-5392: Added all stack frames to ThreadInfo summary.
..


Patch Set 9:

It looks like this got stalled. Jim, do you feel comfortable taking this to +2, 
since you've already looked at it?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I80ab4aad03e0c1f01fecad6b87779531244c28b7
Gerrit-Change-Number: 10145
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Sharma 
Gerrit-Reviewer: Abhishek Sharma 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Charles Agnello 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 23:01:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5737: Tighten minicluster memory limit

2018-05-21 Thread Tianyi Wang (Code Review)
Tianyi Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10277 )

Change subject: IMPALA-5737: Tighten minicluster memory limit
..


Patch Set 7:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10277/7/bin/impala-config.sh@644
PS7, Line 644:   MEM_AVAIL_KB=$(grep -oP "^MemAvailable:[[:space:]]*\K\d*" 
/proc/meminfo || : )
> In another review, Jim asked me to parse the output of "free -m"; see https
Here is the output of free -m on centos6. There is no 'available' column and we 
still need to do the same calculation.

 total   used   free sharedbuffers cached
Mem: 67814  67508306  0271  61151
-/+ buffers/cache:   6084  61729
Swap:0  0  0



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If8240551e726c6da546a926a1ce3444f41ef87fe
Gerrit-Change-Number: 10277
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Mon, 21 May 2018 23:01:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7051: Serialize Maven invocations.

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10460 )

Change subject: IMPALA-7051: Serialize Maven invocations.
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24f34f421bc7dcf9140938464d43400da95275e
Gerrit-Change-Number: 10460
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 22:58:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5164: Better benchmark heuristic

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has abandoned this change. ( http://gerrit.cloudera.org:8080/7389 
)

Change subject: IMPALA-5164: Better benchmark heuristic
..


Abandoned

Looks abandoned, can reopen if needed later
--
To view, visit http://gerrit.cloudera.org:8080/7389
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ie6fb1283716dbc591c10238d85b9ab436d02fb96
Gerrit-Change-Number: 7389
Gerrit-PatchSet: 2
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zach Amsden 


[Impala-ASF-CR] IMPALA-5266 Impala ABM / LZCNT support

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has abandoned this change. ( http://gerrit.cloudera.org:8080/5821 
)

Change subject: IMPALA-5266 Impala ABM / LZCNT support
..


Abandoned

Looks abandoned, can reopen if needed later
--
To view, visit http://gerrit.cloudera.org:8080/5821
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I9f6a465ab4a9ee4f582847f8e211a779bdede3d2
Gerrit-Change-Number: 5821
Gerrit-PatchSet: 8
Gerrit-Owner: Zach Amsden 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Marcel Kornacker 
Gerrit-Reviewer: Zach Amsden 


[Impala-ASF-CR] IMPALA-7051: Serialize Maven invocations.

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10460 )

Change subject: IMPALA-7051: Serialize Maven invocations.
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24f34f421bc7dcf9140938464d43400da95275e
Gerrit-Change-Number: 10460
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 22:56:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7039: Ignore the port in HBase planner tests

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10459 )

Change subject: IMPALA-7039: Ignore the port in HBase planner tests
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8eb7628061b2ebaf84323b37424925e9a64f70a0
Gerrit-Change-Number: 10459
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 22:56:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5706: Spilling sort optimisations

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9943 )

Change subject: IMPALA-5706: Spilling sort optimisations
..


Patch Set 14: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74857c1694802e81f1cfc765d2b4e8bc644387f9
Gerrit-Change-Number: 9943
Gerrit-PatchSet: 14
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 22:51:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5216: Make admission control queuing async

2018-05-21 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10060 )

Change subject: IMPALA-5216: Make admission control queuing async
..


Patch Set 11:

(29 comments)

That looks good. My review comments are mostly to clarify some of the code 
comments.

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

http://gerrit.cloudera.org:8080/#/c/10060/9/be/src/scheduling/admission-controller.h@44
PS9, Line 44: if
add "... or the caller wants to cancel" or something like that about 
cancellation.


http://gerrit.cloudera.org:8080/#/c/10060/9/be/src/scheduling/admission-controller.h@204
PS9, Line 204: Returns immediately if rejected
let's say what the return value and promise value are for each case, i.e. 
something like:
If rejected, returns immediately with an error status indicating the reason and 
sets 'admit_outcome' to REJECTED_OR_TIMED_OUT.  ... and same type of 
explanation for timed out, cancel and admitted case. That will also make it 
explicit that by the time this returns, the promise is always set.


http://gerrit.cloudera.org:8080/#/c/10060/9/be/src/scheduling/admission-controller.h@206
PS9, Line 206: ANCELLED
i.e. explain status in this case.


http://gerrit.cloudera.org:8080/#/c/10060/9/be/src/scheduling/admission-controller.h@212
PS9, Line 212: Promise
could create a typedef for that since it appears in several places and is kinda 
long, but okay to ignore.


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

http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/scheduling/admission-controller.cc@581
PS11, Line 581: result is set or the query is cancelled or it
  :   // times out.
the "result is set" also happens in the cancel case. To clarify, how about 
something like:
Block in Get() up to the time out waiting for the promise to be set when the 
query is admitted or cancelled.


http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/scheduling/admission-controller.cc@590
PS11, Line 590:   SleepIfSetInDebugOptions(schedule->query_options(), 
SLEEP_AFTER_ADMISSION_OUTCOME_MS);
what's interesting about this time interval? Is it to give the dequeue loop 
time to notice the cancellation?


http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/scheduling/admission-controller.cc@607
PS11, Line 607:  stats->Dequeue(*schedule, true);
nit: it was a bit hard to see that this block is really the same as the else 
case (which just different arguments) because things are done in a slightly 
different order. for consistency, how about moving this to after line 604?


http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/service/client-request-state.h@70
PS11, Line 70:   /// the query to the Admission controller for asynchronous 
admission control.
is there some post condition around the operation state we can document? like 
that on success, this sets operation state to RUNNING_STATE or PENDING_STATE.


http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/service/client-request-state.h@131
PS11, Line 131:  and sets the
  :   /// query status to CANCELLE
I thought you decided to hold off on that change.


http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/service/client-request-state.h@153
PS11, Line 153: T
missing space before


http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/service/client-request-state.h@286
PS11, Line 286: dequeuing thread
from this context, not clear what thread this is talking about. How about:
... set asynchronously by the admission controller (to admit).

Also the "admission control thread" is probably referring to what's now called 
the async exec thread?

This comment has a lot of admission controller details, rather than the high 
level overview of what this promise is. How about something like:

Promise used by the admission controller. AdmissionController:AdmitQuery() will 
block on this promise until the query is either rejected, admitted, times out, 
or is cancelled. Can be set to CANCELLED by the ClientRequestState in order to 
cancel, but otherwise is set by AdmissionController with the admission decision.


http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/service/client-request-state.h@442
PS11, Line 442: rocessing a query or dml execution request and submitting it to 
the
  :   /// admission controller
this is confusing since it doesn't actually do that work (it happens 
asynchronously.). I think this comment could use more of a rewrite given the 
new code structure.


http://gerrit.cloudera.org:8080/#/c/10060/11/be/src/service/client-request-state.h@508
PS11, Line 508:
  :   /// Submits the 

[Impala-ASF-CR] IMPALA-5737: Tighten minicluster memory limit

2018-05-21 Thread Philip Zeyliger (Code Review)
Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10277 )

Change subject: IMPALA-5737: Tighten minicluster memory limit
..


Patch Set 7:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10277/7/bin/impala-config.sh@644
PS7, Line 644:   MEM_AVAIL_KB=$(grep -oP "^MemAvailable:[[:space:]]*\K\d*" 
/proc/meminfo || : )
> I made a mistake here: MemAvailable is available in kernel >= 3.14, but on
In another review, Jim asked me to parse the output of "free -m"; see 
https://github.com/apache/impala/blob/master/docker/monitor.py#L60 .



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If8240551e726c6da546a926a1ce3444f41ef87fe
Gerrit-Change-Number: 10277
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Mon, 21 May 2018 22:21:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5737: Tighten minicluster memory limit

2018-05-21 Thread Tianyi Wang (Code Review)
Tianyi Wang has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/10277 )

Change subject: IMPALA-5737: Tighten minicluster memory limit
..

IMPALA-5737: Tighten minicluster memory limit

This patch limits minicluster memory based on the memory available.
Two memory profiles are introduced. If there is less than 32GB memory
available, memory limit of the minicluster will be set to ~27GB.
Otherwise it will be set to ~32GB. This will hopefully help with
minicluster with more than 3 nodes.

The 27GB profile works on m2.4xlarge which has 8 cores. It should be
more than enough for 4-core machines.

Change-Id: If8240551e726c6da546a926a1ce3444f41ef87fe
---
M bin/impala-config.sh
M bin/start-impala-cluster.py
M testdata/bin/run-hbase.sh
M testdata/bin/run-hive-server.sh
M testdata/bin/run-sentry-service.sh
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/kudu-common
8 files changed, 91 insertions(+), 22 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If8240551e726c6da546a926a1ce3444f41ef87fe
Gerrit-Change-Number: 10277
Gerrit-PatchSet: 7
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 


[Impala-ASF-CR] IMPALA-6034: Add Cpu and scanned bytes limits per query

2018-05-21 Thread Mostafa Mokhtar (Code Review)
Mostafa Mokhtar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10415 )

Change subject: IMPALA-6034: Add Cpu and scanned bytes limits per query
..


Patch Set 1:

(31 comments)

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

http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@115
PS1, Line 115: Cpu
> Maybe clarify:
These are methods returns total consumption per backend across fragment 
instances.


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@118
PS1, Line 118:   /// Return total sys Cpu.
> /// Return total system CPU consumption across all backends.
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@121
PS1, Line 121:   /// Return total scanned bytes.
> /// Return total scanned bytes (from HDFS or HDFS-compatible filesystem) ac
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@124
PS1, Line 124:   void GetBackendResourceUsage(int64_t& user_cpu, int64_t& 
sys_cpu, int64_t& scanned_bytes,
> It would be more readable if this returned a struct. 4 output arguments wit
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@125
PS1, Line 125: int64_t& peak_mem
> nit: use pointers for output parameters
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@206
PS1, Line 206: /// total scan ranges complete across all scan nodes
> maybe add "for this backend"
This is a per fragment instance counter.


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@209
PS1, Line 209: /// total user cpu consumed
> maybe add "for this backend"
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@213
PS1, Line 213: int64_t cpu_sys_ = 0;
> maybe add "for this backend"
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@281
PS1, Line 281:   /// total scan ranges complete across all scan nodes
> maybe add "across all backends. Updated by AggregateBackendStats()."
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@284
PS1, Line 284:   /// total user cpu consumed
> maybe add "across all backends. Updated by AggregateBackendStats()."
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator-backend-state.h@287
PS1, Line 287:   /// total system cpu consumed
> maybe add "across all backends. Updated by AggregateBackendStats()."
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:

http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.h@169
PS1, Line 169: void
> Maybe return a struct instead of multiple output arguments?
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.h@169
PS1, Line 169: int64_t&
> nit: use pointer for output parameters
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.cc@721
PS1, Line 721:
 :   int64_t total_cpu =0, total_scanned_bytes =0, backend_user_cpu 
= 0,
 :   backend_sys_cpu = 0,backend_bytes_read = 0, peak_memory = 
0;
> nit: various whitespace inconsistencies
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.cc@726
PS1, Line 726:
> nit: don't need some of this vertical whitespace
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.cc@751
PS1, Line 751:   query_profile_->AddInfoString("Total CPU time", 
cpu_total_info.str());
> nit: capitalisation is a bit inconsistent - I think everything should be ti
Done


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.cc@755
PS1, Line 755:   query_profile_->AddInfoString("Per Node User time", 
cpu_user_info.str());
> Any reason why we don't show the aggregate user and system CPU time across
Check "Total CPU time" which aggregates user and system CPU across all backends.


http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/runtime/coordinator.cc@802
PS1, Line 802: total_user_cpu += backend_state->GetUserCpu();
> Should we modify the BackendState functions so that we can get all three va
Done


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

http://gerrit.cloudera.org:8080/#/c/10415/1/be/src/service/impala-server.h@1041
PS1, Line 1041: int64_t max_cpu_time_ns;
> I think we can avoid including these new fields in ExpirationEvent - the ex
Done


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

[Impala-ASF-CR] IMPALA-6034: Add Cpu and scanned bytes limits per query

2018-05-21 Thread Mostafa Mokhtar (Code Review)
Hello Tim Armstrong, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6034: Add Cpu and scanned bytes limits per query
..

IMPALA-6034: Add Cpu and scanned bytes limits per query

To protect Impala from runaway queries add per query limits for Cpu and
scanned bytes, limits are enforced via query options and apply to the entire
query, not per backend like mem_limit.
If a query exceeds any of the limits it will get cancelled.

Query profile is upated to include query wide and per backend metrics for Cpu
and scanned bytes.

Testing:
Added tests for various permutations for  MAX_CPU_TIME_S and MAX_SCAN_BYTES

Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test
A tests/query_test/test_query_resource_limits.py
13 files changed, 372 insertions(+), 21 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
Gerrit-Change-Number: 10415
Gerrit-PatchSet: 3
Gerrit-Owner: Mostafa Mokhtar 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6034: Add Cpu and scanned bytes limits per query

2018-05-21 Thread Mostafa Mokhtar (Code Review)
Hello Tim Armstrong, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6034: Add Cpu and scanned bytes limits per query
..

IMPALA-6034: Add Cpu and scanned bytes limits per query

To protect Impala from runaway queries add per query limits for Cpu and
scanned bytes, limits are enforced via query options and apply to the entire
query, not per backend like mem_limit.
If a query exceeds any of the limits it will get cancelled.

Query profile is upated to include query wide and per backend metrics for Cpu
and scanned bytes.

Testing:
Added tests for various permutations for  MAX_CPU_TIME_S and MAX_SCAN_BYTES

Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
A 
testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test
A tests/query_test/test_query_resource_limits.py
13 files changed, 373 insertions(+), 21 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20
Gerrit-Change-Number: 10415
Gerrit-PatchSet: 2
Gerrit-Owner: Mostafa Mokhtar 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS based table

2018-05-21 Thread Sailesh Mukil (Code Review)
Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9966 )

Change subject: IMPALA-6813: Hedged reads metrics broken when scanning non-HDFS 
based table
..


Patch Set 1:

> > Patch Set 1:
 > >
 > > > Patch Set 1:
 > > >
 > > > I haven't added a custom cluster test for this since hedged
 > reads are not deterministic, and we'd be dealing with a bunch of
 > flakiness.
 > >
 > > I'm uncomfortable that we've got a flag with no testing of the
 > code path behind it.
 > >
 > > Can you force a hedged read by "kill -STOP"'ing a datanode before
 > the query and "kill -CONT" afterwards?
 >
 > Thanks for the suggestion, Phil. I will try this.

I picked this up again and spent some time trying to force hedged reads. 
However, I found that even if you SIGSTOP any combination of DataNodes while 
leaving only one running, and then run a query against HDFS, since all the 
DataNodes are on the same node in a minicluster, it doesn't count them as 
hedged reads.

This means that we can only have a hedged read test in a remote cluster setting.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48fe80dfd9a1ed68a8f2b7038e5f42b5a3df3baa
Gerrit-Change-Number: 9966
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Mon, 21 May 2018 21:56:39 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6317: Add -cmake only option to buildall.sh

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10455 )

Change subject: IMPALA-6317: Add -cmake_only option to buildall.sh
..

IMPALA-6317: Add -cmake_only option to buildall.sh

It's sometimes useful to be able to build a complete Impala dev
environment without necessarily building the Impala binary itself
-- e.g., when one wants to use the internal test framework to run
tests against an instance of Impala running on a remote cluster.

- This patch adds a -cmake_only flag to buildall.sh, which then
  gets propagated to the make_impala.sh.

- Added a missing line to the help text re: passing the -ninja
  command line option

Change-Id: If31a4e29425a6a20059cba2f43b72e4fb908018f
Reviewed-on: http://gerrit.cloudera.org:8080/10455
Reviewed-by: David Knupp 
Tested-by: Impala Public Jenkins 
---
M buildall.sh
1 file changed, 5 insertions(+), 0 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If31a4e29425a6a20059cba2f43b72e4fb908018f
Gerrit-Change-Number: 10455
Gerrit-PatchSet: 4
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6317: Add -cmake only option to buildall.sh

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10455 )

Change subject: IMPALA-6317: Add -cmake_only option to buildall.sh
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If31a4e29425a6a20059cba2f43b72e4fb908018f
Gerrit-Change-Number: 10455
Gerrit-PatchSet: 3
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 21:55:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10413 )

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 5
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Mon, 21 May 2018 21:43:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

2018-05-21 Thread Taras Bobrovytsky (Code Review)
Taras Bobrovytsky has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10413 )

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 5
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Mon, 21 May 2018 21:33:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7019: Schedule EC as remote & disable failed tests

2018-05-21 Thread Tianyi Wang (Code Review)
Tianyi Wang has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/10413 )

Change subject: IMPALA-7019: Schedule EC as remote & disable failed tests
..

IMPALA-7019: Schedule EC as remote & disable failed tests

This patch schedules HDFS EC files without considering locality. Failed
tests are disabled and a jenkins build should succeed with export
ERASURE_COINDG=true.

Testing: It passes core tests.

Cherry-picks: not for 2.x.

Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
---
M common/fbs/CatalogObjects.fbs
M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M tests/common/skip.py
M tests/custom_cluster/test_admission_controller.py
M tests/custom_cluster/test_hdfs_fd_caching.py
M tests/metadata/test_explain.py
M tests/query_test/test_hdfs_caching.py
M tests/query_test/test_insert.py
M tests/query_test/test_insert_parquet.py
M tests/query_test/test_mt_dop.py
M tests/query_test/test_nested_types.py
M tests/query_test/test_queries.py
M tests/query_test/test_query_mem_limit.py
M tests/query_test/test_scanners.py
M tests/util/filesystem_utils.py
17 files changed, 75 insertions(+), 28 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I138738d3e28e5daa1718c05c04cd9dd146c4ff84
Gerrit-Change-Number: 10413
Gerrit-PatchSet: 5
Gerrit-Owner: Tianyi Wang 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tianyi Wang 


[Impala-ASF-CR] IMPALA-6909: [DOCS] SET ROW FORMAT in ALTER TABLE

2018-05-21 Thread Alex Rodoni (Code Review)
Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10452 )

Change subject: IMPALA-6909: [DOCS] SET ROW FORMAT in ALTER TABLE
..


Patch Set 1:

Could you please review this doc updates? Thanks!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib680f25d8c929e1194a2274777f83851b04bcb93
Gerrit-Change-Number: 10452
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 May 2018 21:17:29 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7011: Simplify PlanRootSink control logic

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10449 )

Change subject: IMPALA-7011: Simplify PlanRootSink control logic
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Gerrit-Change-Number: 10449
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 20:29:05 +
Gerrit-HasComments: No


[Impala-ASF-CR](2.x) IMPALA-5384, part 2: Simplify Coordinator locking and clarify state

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10465 )

Change subject: IMPALA-5384, part 2: Simplify Coordinator locking and clarify 
state
..

IMPALA-5384, part 2: Simplify Coordinator locking and clarify state

The is the final change to clarify and break up the Coordinator's lock.
The state machine for the coordinator is made explicit, distinguishing
between executing state and multiple terminal states. Logic to
transition into a terminal state is centralized in one location and
executes exactly once for each coordinator object.

Derived from a patch for IMPALA_5384 by Marcel Kornacker.

Testing:
- exhaustive functional tests
- stress test on minicluster with memory overcommitment. Verified from
  the logs that this exercises all these paths:
  - successful queries
  - client requested cancellation
  - error from exec FInstances RPC
  - error reported asynchronously via report status RPC
  - eos before backend execution completed
- loop query_test & failure for 12 hours with no dchecks or crashes
  (This had previously reproduced IMPALA-7030 and IMPALA-7033 with
  the previous version of this change).

Change-Id: I6dc08da1295f1df3c9dce6d35d65d887b2c00a1c
Reviewed-on: http://gerrit.cloudera.org:8080/10440
Reviewed-by: Dan Hecht 
Tested-by: Impala Public Jenkins 
Reviewed-on: http://gerrit.cloudera.org:8080/10465
---
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/client-request-state.cc
M be/src/service/impala-server.h
M be/src/util/counting-barrier.h
6 files changed, 396 insertions(+), 391 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I6dc08da1295f1df3c9dce6d35d65d887b2c00a1c
Gerrit-Change-Number: 10465
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR](2.x) IMPALA-5384, part 2: Simplify Coordinator locking and clarify state

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10465 )

Change subject: IMPALA-5384, part 2: Simplify Coordinator locking and clarify 
state
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6dc08da1295f1df3c9dce6d35d65d887b2c00a1c
Gerrit-Change-Number: 10465
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 May 2018 19:57:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-2195: Improper handling of comments in queries

2018-05-21 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9933 )

Change subject: IMPALA-2195: Improper handling of comments in queries
..


Patch Set 12:

Rebased and carry +1. Can anyone review this PR?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ac7cb5a30e6dda73ebe761d9f0eb9ba038e14a7
Gerrit-Change-Number: 9933
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Comment-Date: Mon, 21 May 2018 18:41:53 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6317: Add -cmake only option to buildall.sh

2018-05-21 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10455 )

Change subject: IMPALA-6317: Add -cmake_only option to buildall.sh
..


Patch Set 3:

> Patch Set 3: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2511/

Filed https://issues.apache.org/jira/browse/IMPALA-7055 for build failure, 
which is not related to this patch. I'm going to try rerunning.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If31a4e29425a6a20059cba2f43b72e4fb908018f
Gerrit-Change-Number: 10455
Gerrit-PatchSet: 3
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 18:30:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6317: Add -cmake only option to buildall.sh

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10455 )

Change subject: IMPALA-6317: Add -cmake_only option to buildall.sh
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If31a4e29425a6a20059cba2f43b72e4fb908018f
Gerrit-Change-Number: 10455
Gerrit-PatchSet: 3
Gerrit-Owner: David Knupp 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 18:30:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7011: Simplify PlanRootSink control logic

2018-05-21 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10449 )

Change subject: IMPALA-7011: Simplify PlanRootSink control logic
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Gerrit-Change-Number: 10449
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 18:29:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6131: Track time of last statistics update in metadata

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10116 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 15: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59a671ac29d352bd92ce40d5cb6662bb23f146b5
Gerrit-Change-Number: 10116
Gerrit-PatchSet: 15
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 21 May 2018 18:25:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6131: Track time of last statistics update in metadata

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10116 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..

IMPALA-6131: Track time of last statistics update in metadata

The timestamp of the last COMPUTE STATS operation is saved to
table property "impala.lastComputeStatsTime". The format is
the same as in "transient_lastDdlTime", so the two can be
compared to check if the schema has changed since computing
statistics.

Other changes:
- Handling of "transient_lastDdlTime" is simplified - the old
  logic set it to current time + 1, if the old version was
  >= current time, to ensure that it is always increased by
  DDL operations. This was useful in the past, as IMPALA-387
  used lastDdlTime to check if partition data needs to be
  reloaded, but since IMPALA-1480, Impala does not rely on
  lastDdlTime at all.

- Computing / setting stats on HDFS tables no longer increases
  "transient_lastDdlTime".

- When Kudu tables are (re)loaded, it is checked if their
  HMS representation is up to date, and if it is, then
  IMetaStoreClient.alter_table() is not called. The old
  logic always called alter_table() after loading metadata
  from Kudu. This change was needed to ensure that
  "transient_lastDdlTime" works similarly in HDFS and Kudu
  tables, and should also make (re)loading Kudu tables faster.

Notes:
- Kudu will be able to sync its tables to HMS in the near
  future (see KUDU-2191), so the Kudu metadata handling in
  Impala may need to be redesigned.

Testing:
tests/metadata/test_last_ddl_time_update.py is extended by
- also checking "impala.lastComputeStatsTime"
- testing more SQL statements
- tests for Kudu tables

Note that test_last_ddl_time_update.py is ran only in
exhaustive testing.

Change-Id: I59a671ac29d352bd92ce40d5cb6662bb23f146b5
Reviewed-on: http://gerrit.cloudera.org:8080/10116
Reviewed-by: Lars Volker 
Tested-by: Impala Public Jenkins 
---
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.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/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M tests/metadata/test_last_ddl_time_update.py
8 files changed, 228 insertions(+), 173 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I59a671ac29d352bd92ce40d5cb6662bb23f146b5
Gerrit-Change-Number: 10116
Gerrit-PatchSet: 16
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 


[Impala-ASF-CR] IMPALA-7011: Simplify PlanRootSink control logic

2018-05-21 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10449 )

Change subject: IMPALA-7011: Simplify PlanRootSink control logic
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10449/5/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/10449/5/be/src/runtime/coordinator.cc@153
PS5, Line 153: are().ok());
 : coord_sink_ = coord_instance_->root_sink();
 : DCHECK(coord_sink_ != nullptr);
 :   }
 :   return Status::OK();
 : }
this comment is now obsolete, so deleted it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Gerrit-Change-Number: 10449
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 18:05:39 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7011: Simplify PlanRootSink control logic

2018-05-21 Thread Dan Hecht (Code Review)
Hello Michael Ho, Tim Armstrong, Bikramjeet Vig,

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

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

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

Change subject: IMPALA-7011: Simplify PlanRootSink control logic
..

IMPALA-7011: Simplify PlanRootSink control logic

1) The eos_ and sender_done_ bits really encode three possible states
   that the sender can be in. Make this explicit using an enum with
   three values.

2) The purpose of CloseConsumer() has changed over time and we can clean
   this up now:

 a) Originally, it looks like it was used to unblock the sender when the
   consumer finishes before eos, but also keep the sink alive long
   enough for the coordinator. This is no longer necessary now that
   control structures are owned by the QueryState whose lifetime is
   controlled by a reference count taken by the coordinator. So, we don't
   need the coordinator to tell the sink it's done calling it and we
   don't need the consumer_done_ state.

 b) Later on, CloseConsumer() was used as a cancellation mechinism.
   We need to keep this around (or use timeouts on the condvars) to kick
   both the consumer and producer on cancellation. But let's make the
   cancellation logic similar to the exec nodes and other sinks by
   driving the cancellation using the RuntimeState's cancellation
   flag. Now that CloseConsumer() is only about cancellation, rename it
   to Cancel() (later we may promote it to DataSink and implement in the
   data stream sender as well).

Testing:
- Exhaustive
- Minicluster concurrent_select.py stress

Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
---
M be/src/exec/plan-root-sink.cc
M be/src/exec/plan-root-sink.h
M be/src/runtime/coordinator.cc
M be/src/runtime/fragment-instance-state.cc
4 files changed, 60 insertions(+), 68 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Gerrit-Change-Number: 10449
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-5706: Spilling sort optimisations

2018-05-21 Thread Gabor Kaszab (Code Review)
Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9943 )

Change subject: IMPALA-5706: Spilling sort optimisations
..


Patch Set 14:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9943/8/be/src/runtime/sorter.cc
File be/src/runtime/sorter.cc:

http://gerrit.cloudera.org:8080/#/c/9943/8/be/src/runtime/sorter.cc@1676
PS8, Line 1676: num_required_buffers -= 
sorted_runs_[j]->HasVarLenPages() ? 2 : 1;
> Thanks for the heads-up! I count for 2 buffers for an intermediate result t
There is an issue with allocation 2 buffers unconditionally for the result run: 
In case there are no var len pages for any of the runs, then the # of available 
buffers will be 3 (as a result of ComputeMinReservation()) but if out of those 
two are taken by the merge result then won't be enough buffers to merge at 
least two runs.
So I have to take it into account that check in Run::Init() when I calculate 
the number of buffers for the merge result (if (has_var_len_slots_)).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74857c1694802e81f1cfc765d2b4e8bc644387f9
Gerrit-Change-Number: 9943
Gerrit-PatchSet: 14
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 16:56:37 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5706: Spilling sort optimisations

2018-05-21 Thread Gabor Kaszab (Code Review)
Hello Tim Armstrong, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-5706: Spilling sort optimisations
..

IMPALA-5706: Spilling sort optimisations

This patch covers multiple changes with the purpose of optimizing
spilling sort mechanism:
  - Remove the hard-coded maximum limit of buffers that can be used
for merging the sorted runs. Instead this number is calculated
based on the available memory through buffer pool.
  - The already sorted runs are distributed more optimally between
the last intermediate merge and the final merge to avoid that a
heavy intermediate merge is followed by a light final merge.
  - Right before starting the merging phase Sorter tries to allocate
additional memory through the buffer pool.
  - An output run is not allocated anymore for the final merge.

Note, double-buffering the runs during a merge was also planned with
this patch. However, performance testing showed that except some
exotic queries with unreasonably small amount of buffer pool memory
available double-buffering doesn't add to the overall performance.
It's basically because the half of the available buffers have to be
sacrificed to do double-buffering and as a result the merge tree can
get deeper. In addition the amount of I/O wait time is not reaching
the level where double-buffering could countervail the reduced number
of runs during a particular merge.

Performance measurements were made during manual testing to verify
that this is in fact an optimization:
  - In case doing a sort on top of a join when working with a
restricted amount of memory then the Sort node successfully
allocates additional memory right before the merging phase. This
is feasible because once Join finishes sending new input data and
calls InputDone() then it releases memory that can be picked up
by the Sorter. This results in shallower merging trees (more runs
grabbed for a merge).
  - On a multi-node cluster I verified that in cases when at least one
merging step is done then this change reduces the execution time
for sorts.
  - The more merging steps are done the bigger the performance gain is
compared to the baseline.

Change-Id: I74857c1694802e81f1cfc765d2b4e8bc644387f9
---
M be/src/runtime/sorter.cc
M be/src/runtime/sorter.h
M tests/query_test/test_sort.py
3 files changed, 152 insertions(+), 116 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/9943/14
--
To view, visit http://gerrit.cloudera.org:8080/9943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I74857c1694802e81f1cfc765d2b4e8bc644387f9
Gerrit-Change-Number: 9943
Gerrit-PatchSet: 14
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6947: Kudu tests flaky due to rpc timeout

2018-05-21 Thread Thomas Marshall (Code Review)
Thomas Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10466


Change subject: IMPALA-6947: Kudu tests flaky due to rpc timeout
..

IMPALA-6947: Kudu tests flaky due to rpc timeout

Some Kudu tests occasionally fail due to hitting an rpc timeout when
running on a heavily loaded machine. For normal operation, having a
low timeout is good for noticing issues quickly, but for tests there's
no real reason we can't set this higher to avoid flakiness.

This patch adds a flag, -kudu_rpc_timeout_ms, disabled by default. It
also sets -kudu_rpc_timeout_ms to 60s for cluster startup in
run-all-tests.sh, higher than Kudu's default value of 10s.

Testing:
- Passed a full run of core tests.

Change-Id: I8c4a2d87934cd4eb98509512f7060a596894ed53
---
M be/src/common/global-flags.cc
M be/src/exec/kudu-util.cc
M bin/run-all-tests.sh
3 files changed, 12 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8c4a2d87934cd4eb98509512f7060a596894ed53
Gerrit-Change-Number: 10466
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Marshall 


[Impala-ASF-CR] IMPALA-6035: Add query options to limit thread reservation

2018-05-21 Thread Mostafa Mokhtar (Code Review)
Mostafa Mokhtar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10365 )

Change subject: IMPALA-6035: Add query options to limit thread reservation
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10365/4/testdata/workloads/functional-query/queries/QueryTest/resource-limits.test
File testdata/workloads/functional-query/queries/QueryTest/resource-limits.test:

http://gerrit.cloudera.org:8080/#/c/10365/4/testdata/workloads/functional-query/queries/QueryTest/resource-limits.test@1
PS4, Line 1: 
Can you rename the file to "thread-limits.test"?
Same for added tests that test the thread limit?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b5bbbdad5cd6b24442eb6c99a4d38c2ad710007
Gerrit-Change-Number: 10365
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Mostafa Mokhtar 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 16:26:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7011: Simplify PlanRootSink control logic

2018-05-21 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10449 )

Change subject: IMPALA-7011: Simplify PlanRootSink control logic
..


Patch Set 5:

Michael, any more comments?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Gerrit-Change-Number: 10449
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 16:25:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-5168: Codegen HASH PARTITIONED KrpcDataStreamSender::Send()

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10421 )

Change subject: IMPALA-5168: Codegen HASH_PARTITIONED 
KrpcDataStreamSender::Send()
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10421/2/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/10421/2/be/src/runtime/krpc-data-stream-sender.cc@716
PS2, Line 716:   for (int i = 0; i < partition_exprs_.size(); ++i) {
> Have you done any experiments for wide rows to determine the codegen time?
A concrete scenario where N could be large here is something like "select 
distinct * from wide_table"



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c44cc9312c062cc7a5a4ac9156ceaa31fb887ff
Gerrit-Change-Number: 10421
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 16:15:37 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-5168: Codegen HASH PARTITIONED KrpcDataStreamSender::Send()

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10421 )

Change subject: IMPALA-5168: Codegen HASH_PARTITIONED 
KrpcDataStreamSender::Send()
..


Patch Set 2:

(1 comment)

Code looks good, just want to be sure we get ahead of any codegen time issues :)

http://gerrit.cloudera.org:8080/#/c/10421/2/be/src/runtime/krpc-data-stream-sender.cc
File be/src/runtime/krpc-data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/10421/2/be/src/runtime/krpc-data-stream-sender.cc@716
PS2, Line 716:   for (int i = 0; i < partition_exprs_.size(); ++i) {
Have you done any experiments for wide rows to determine the codegen time? 
Unrolling a loop N times is the classic pattern that we've seen cause codegen 
time to blow up before.

It would be good to get ahead of it in this instance and either do something to 
mitigate it (like http://gerrit.cloudera.org:8080/8211), or disable it above 
some number of expressions to avoid the regression



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c44cc9312c062cc7a5a4ac9156ceaa31fb887ff
Gerrit-Change-Number: 10421
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 16:14:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](2.x) IMPALA-5384, part 2: Simplify Coordinator locking and clarify state

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10465 )

Change subject: IMPALA-5384, part 2: Simplify Coordinator locking and clarify 
state
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6dc08da1295f1df3c9dce6d35d65d887b2c00a1c
Gerrit-Change-Number: 10465
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 May 2018 16:11:39 +
Gerrit-HasComments: No


[Impala-ASF-CR](2.x) IMPALA-5384, part 2: Simplify Coordinator locking and clarify state

2018-05-21 Thread Dan Hecht (Code Review)
Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10465 )

Change subject: IMPALA-5384, part 2: Simplify Coordinator locking and clarify 
state
..


Patch Set 1: Code-Review+2

(1 comment)

Resolved trivial merge conflict for 2.x

http://gerrit.cloudera.org:8080/#/c/10465/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/10465/1/be/src/runtime/coordinator.cc@766
PS1, Line 766:  if (admission_controller != nullptr) 
admission_controller->ReleaseQuery(schedule_);
 :   query_events_->MarkEvent("Released admission control 
resources");
merge conflict here



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6dc08da1295f1df3c9dce6d35d65d887b2c00a1c
Gerrit-Change-Number: 10465
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 May 2018 16:11:26 +
Gerrit-HasComments: Yes


[Impala-ASF-CR](2.x) IMPALA-5384, part 2: Simplify Coordinator locking and clarify state

2018-05-21 Thread Dan Hecht (Code Review)
Hello Impala Public Jenkins,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: IMPALA-5384, part 2: Simplify Coordinator locking and clarify 
state
..

IMPALA-5384, part 2: Simplify Coordinator locking and clarify state

The is the final change to clarify and break up the Coordinator's lock.
The state machine for the coordinator is made explicit, distinguishing
between executing state and multiple terminal states. Logic to
transition into a terminal state is centralized in one location and
executes exactly once for each coordinator object.

Derived from a patch for IMPALA_5384 by Marcel Kornacker.

Testing:
- exhaustive functional tests
- stress test on minicluster with memory overcommitment. Verified from
  the logs that this exercises all these paths:
  - successful queries
  - client requested cancellation
  - error from exec FInstances RPC
  - error reported asynchronously via report status RPC
  - eos before backend execution completed
- loop query_test & failure for 12 hours with no dchecks or crashes
  (This had previously reproduced IMPALA-7030 and IMPALA-7033 with
  the previous version of this change).

Change-Id: I6dc08da1295f1df3c9dce6d35d65d887b2c00a1c
Reviewed-on: http://gerrit.cloudera.org:8080/10440
Reviewed-by: Dan Hecht 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/service/client-request-state.cc
M be/src/service/impala-server.h
M be/src/util/counting-barrier.h
6 files changed, 396 insertions(+), 391 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6dc08da1295f1df3c9dce6d35d65d887b2c00a1c
Gerrit-Change-Number: 10465
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-7011: Simplify PlanRootSink control logic

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10449 )

Change subject: IMPALA-7011: Simplify PlanRootSink control logic
..


Patch Set 5: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc75617a253fd43a6122baa4b4dc7aeb1dbe633f
Gerrit-Change-Number: 10449
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 15:58:02 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6035: Add query options to limit thread reservation

2018-05-21 Thread Tim Armstrong (Code Review)
Hello Bikramjeet Vig,

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

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

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

Change subject: IMPALA-6035: Add query options to limit thread reservation
..

IMPALA-6035: Add query options to limit thread reservation

Adds two options: THREAD_RESERVATION_LIMIT and
THREAD_RESERVATION_AGGREGATE_LIMIT, which are both enforced by admission
control based on planner resource requirements and the schedule. The
mechanism used is the same as the minimum reservation checks.

THREAD_RESERVATION_LIMIT limits the total number of reserved threads in
fragments scheduled on a single backend.
THREAD_RESERVATION_AGGREGATE_LIMIT limits the sum of reserved threads
across all fragments.

This also slightly improves the minimum reservation error message to
include the host name.

Testing:
Added end-to-end tests that exercise the code paths.

Ran core tests.

Change-Id: I5b5bbbdad5cd6b24442eb6c99a4d38c2ad710007
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/query-schedule.h
M be/src/scheduling/scheduler.cc
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M 
testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test
A testdata/workloads/functional-query/queries/QueryTest/resource-limits.test
M testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
A tests/query_test/test_resource_limits.py
12 files changed, 248 insertions(+), 25 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5b5bbbdad5cd6b24442eb6c99a4d38c2ad710007
Gerrit-Change-Number: 10365
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-6035: Add query options to limit thread reservation

2018-05-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10365 )

Change subject: IMPALA-6035: Add query options to limit thread reservation
..


Patch Set 4:

(2 comments)

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

http://gerrit.cloudera.org:8080/#/c/10365/6/be/src/scheduling/admission-controller.cc@419
PS6, Line 419: t64_t cluster_thread_reservation = 0;
> after switch to using "e.first" instead of "e.second.instance_params[0]->ho
Done


http://gerrit.cloudera.org:8080/#/c/10365/4/be/src/service/query-options.cc
File be/src/service/query-options.cc:

http://gerrit.cloudera.org:8080/#/c/10365/4/be/src/service/query-options.cc@672
PS4, Line 672: Only -1
> makes sense. Lets document the -1 option in the thrift declaration too then
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b5bbbdad5cd6b24442eb6c99a4d38c2ad710007
Gerrit-Change-Number: 10365
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 May 2018 15:56:27 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6994: Avoid reloading a table's HMS data for file-only operations.

2018-05-21 Thread Pranay Singh (Code Review)
Pranay Singh has abandoned this change. ( http://gerrit.cloudera.org:8080/10450 
)

Change subject: IMPALA-6994: Avoid reloading a table's HMS data for file-only 
operations.
..


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Iaabdf38af3f30c65ada9734eb471dbfa6ecdd74a
Gerrit-Change-Number: 10450
Gerrit-PatchSet: 2
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Quanlong Huang 


[Impala-ASF-CR] IMPALA-6994: Avoid reloading a table's HMS data for file-only operations.

2018-05-21 Thread Pranay Singh (Code Review)
Pranay Singh has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10450 )

Change subject: IMPALA-6994: Avoid reloading a table's HMS data for file-only 
operations.
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10450/2/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/10450/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1410
PS2, Line 1410: partitionsToUpdateFileMdByPath = 
getPartitionsByPath(partitionsToUpdate);
  :   loadMetadataAndDiskIds(partitionsToUpdateFileMdByPath, 
true);
> I don't think so. In line 1404, the dirtyPartitions are all added to partit
Yes missed on that one, I'll rollback this change.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaabdf38af3f30c65ada9734eb471dbfa6ecdd74a
Gerrit-Change-Number: 10450
Gerrit-PatchSet: 2
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Mon, 21 May 2018 15:53:22 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6131: Track time of last statistics update in metadata

2018-05-21 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10116 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 15:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59a671ac29d352bd92ce40d5cb6662bb23f146b5
Gerrit-Change-Number: 10116
Gerrit-PatchSet: 15
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 21 May 2018 14:58:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6131: Track time of last statistics update in metadata

2018-05-21 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10116 )

Change subject: IMPALA-6131: Track time of last statistics update in metadata
..


Patch Set 15: Code-Review+2

Carrying Alex's +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I59a671ac29d352bd92ce40d5cb6662bb23f146b5
Gerrit-Change-Number: 10116
Gerrit-PatchSet: 15
Gerrit-Owner: Csaba Ringhofer 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 21 May 2018 14:58:17 +
Gerrit-HasComments: No