[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16881 )

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Patch Set 8: Code-Review+1

(2 comments)

Thanks for the explanation. LGTM.

http://gerrit.cloudera.org:8080/#/c/16881/7/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/16881/7/be/src/util/runtime-profile.cc@2723
PS7, Line 2723:   // Legacy profile did not show per-instance stats for 
averaged profile.
> Done
Resolved


http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py
File tests/observability/test_profile_tool.py:

http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py@36
PS6, Line 36: self._compare_profile_tool_output([],
> Yeah, producing identical output is not necessarily the goal of this tool,
Got it, thanks!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Dec 2020 06:58:34 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

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

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Patch Set 8:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Dec 2020 06:54:07 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16881 )

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16881/7/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/16881/7/be/src/util/runtime-profile.cc@2723
PS7, Line 2723:   if (verbosity <= Verbosity::LEGACY) continue;
> Looks like this condition can be flipped and merged with the branch bellow?
Done


http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py
File tests/observability/test_profile_tool.py:

http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py@36
PS6, Line 36: self._compare_profile_tool_output([],
> Ok, I understand the difference now. Setting --profile_verbosity=legacy mea
Yeah, producing identical output is not necessarily the goal of this tool, but 
we do want to be able to parse older profiles and we want to have some tests 
for pretty-printing so that it's rendered as expected.

One thing I'm hoping to achieve with the tool is to buy more flexibility around 
the text representation of the V2 profiles. We had a lot of dilemmas about how 
much information to include, but those are less important if we can print the 
profile with more/less information later on; or if we can improve the output in 
the tool independent of the version of Impala profiles being analyzed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Dec 2020 06:33:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Tim Armstrong (Code Review)
Hello Riza Suminto, Joe McDonnell, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..

IMPALA-9865: part 2/2: add verbosity to profile tool

Adds a --profile_verbosity option for impala-profile-tool with
the following levels:
* 0: minimal
* 1: legacy - matches old output, this is the default still
* 2: default - basic descriptive stats, used for V2 profile.
* 3: extended
* 4: full

This will help with transition to the V2 profile because we
can have a nice, high-level, readable text profile by default
with the option to produce more detailed profiles and alternate
views of the profile from the thrift profile.

Use the profile version in impala-profile-tool to dump the
more verbose output for the V2 profile while preserving the
same output for the legacy profile.

Reduce verbosity of v2 profile output - only include mean/min/max
by default. I intend to refine the output at the different
verbosity levels for the v2 profiles further as part of IMPALA-9382,
it is still fairly noisy.

Fix output with/without gen_experimental_profile - there
was a small difference in that the summary stats were not
output in the averaged profile.

Testing:
* Add an end-to-end test that generates output for a small
  profile log and compares against expected files.
* Tweak other profile tests to reflect changes to output.

Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
---
M be/src/util/impala-profile-tool.cc
M be/src/util/runtime-profile-counters.h
M be/src/util/runtime-profile-test.cc
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M bin/jenkins/dockerized-impala-run-tests.sh
M bin/rat_exclude_files.txt
A testdata/impala-profiles/README
A testdata/impala-profiles/impala_profile_log_tpcds_compute_stats
A testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.json
A 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.pretty.json
A 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.pretty_extended.json
A testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.txt
A 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats_default.expected.txt
A 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats_extended.expected.txt
M testdata/workloads/tpch/queries/runtime-profile-aggregated.test
A tests/observability/test_profile_tool.py
M tests/query_test/test_scanners.py
18 files changed, 95,019 insertions(+), 125 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-10317: Add query option that limits huge joins at runtime

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

Change subject: IMPALA-10317: Add query option that limits huge joins at runtime
..

IMPALA-10317: Add query option that limits huge joins at runtime

This patch adds support for limiting the rows produced by a join node
such that runaway join queries can be prevented.

The limit is specified by a query option. Queries exceeding that limit
get terminated. The checking runs periodically, so the actual rows
produced may go somewhat over the limit.

JOIN_ROWS_PRODUCED_LIMIT is exposed as an advanced query option.

Rows produced Query profile is updated to include query wide and per
backend metrics for RowsReturned. Example from "
set JOIN_ROWS_PRODUCED_LIMIT = 1000;
select count(*) from tpch_parquet.lineitem l1 cross join
(select * from tpch_parquet.lineitem l2 limit 5) l3;":

NESTED_LOOP_JOIN_NODE (id=2):
   - InactiveTotalTime: 107.534ms
   - PeakMemoryUsage: 16.00 KB (16384)
   - ProbeRows: 1.02K (1024)
   - ProbeTime: 0.000ns
   - RowsReturned: 10.00M (10002025)
   - RowsReturnedRate: 749.58 K/sec
   - TotalTime: 13s337ms

Testing:
 Added tests for JOIN_ROWS_PRODUCED_LIMIT

Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Reviewed-on: http://gerrit.cloudera.org:8080/16706
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/service/impala-server.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/protobuf/control_service.proto
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/generate_error_codes.py
M 
testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test
14 files changed, 167 insertions(+), 5 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Gerrit-Change-Number: 16706
Gerrit-PatchSet: 8
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-10317: Add query option that limits huge joins at runtime

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

Change subject: IMPALA-10317: Add query option that limits huge joins at runtime
..


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Gerrit-Change-Number: 16706
Gerrit-PatchSet: 7
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 22 Dec 2020 06:10:38 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

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

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 22 Dec 2020 04:58:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

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

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 22 Dec 2020 04:58:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

2020-12-21 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16884 )

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 22 Dec 2020 04:57:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9966: Add missing breaks in SetQueryOption

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

Change subject: IMPALA-9966: Add missing breaks in SetQueryOption
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I696204c7036c0059fe4eed2d23c8e947b6c53d8d
Gerrit-Change-Number: 16892
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 22 Dec 2020 04:31:55 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9966: Add missing breaks in SetQueryOption

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

Change subject: IMPALA-9966: Add missing breaks in SetQueryOption
..

IMPALA-9966: Add missing breaks in SetQueryOption

Change-Id: I696204c7036c0059fe4eed2d23c8e947b6c53d8d
Reviewed-on: http://gerrit.cloudera.org:8080/16892
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/service/query-options.cc
1 file changed, 7 insertions(+), 5 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I696204c7036c0059fe4eed2d23c8e947b6c53d8d
Gerrit-Change-Number: 16892
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

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

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 3:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 22 Dec 2020 03:41:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

2020-12-21 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16884 )

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16884/2/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:

http://gerrit.cloudera.org:8080/#/c/16884/2/be/src/runtime/buffered-tuple-stream.h@563
PS2, Line 563:   friend class SimpleTupleStreamTest_ShortDebugString_Test;
 :
 :   /// Runtime state instance used to check for cancellation. Not 
own
> Yeah, please delete them.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 22 Dec 2020 03:26:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

2020-12-21 Thread Riza Suminto (Code Review)
Hello Quanlong Huang, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..

IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

BufferedTupleStream::DebugString() iterate std::list that can
potentially grow very large. As consequent, the returned string can grow
large as well and cause a problem as previously happen in IMPALA-9851.
With this patch, BufferedTupleStream::DebugString() only include maximum
of 100 first pages of page list.

Testing:
- Add new be test SimpleTupleStreamTest.ShortDebugString in
  buffered-tuple-stream-test.cc
- Pass core tests

Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
---
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/buffered-tuple-stream.h
3 files changed, 73 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16881 )

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Patch Set 7:

(6 comments)

I have 1 follow up question and 1 new finding.

http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc
File be/src/util/impala-profile-tool.cc:

http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc@88
PS6, Line 88:   RuntimeProfileBase::Verbosity configured_verbosity =
> We shouldn't actually be using the default value, since we only use if it's
Done


http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc@136
PS6, Line 136: if (FLAGS_profile_verbosity == "") {
> Yes and no. We can set the verbosity level to any level for any profile, bu
Make sense. Thanks for clarifying this.


http://gerrit.cloudera.org:8080/#/c/16881/7/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/16881/7/be/src/util/runtime-profile.cc@2723
PS7, Line 2723:   if (verbosity <= Verbosity::LEGACY) continue;
Looks like this condition can be flipped and merged with the branch bellow?

if (verbosity >= Verbosity::DEFAULT && v.second.second.size() > 1) {


http://gerrit.cloudera.org:8080/#/c/16881/6/bin/rat_exclude_files.txt
File bin/rat_exclude_files.txt:

http://gerrit.cloudera.org:8080/#/c/16881/6/bin/rat_exclude_files.txt@157
PS6, Line 157: 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.txt
> Done
Resolved


http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py
File tests/observability/test_profile_tool.py:

http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py@36
PS6, Line 36: self._compare_profile_tool_output([],
> Leaving --profile_verbosity unset is different from setting it to --legacy
Ok, I understand the difference now. Setting --profile_verbosity=legacy means 
forcing legacy format, regardless of the profile_version.

Since we don't test with --profile_verbosity=legacy, am I correct to assume 
that accurate backward-compatible parsing of v2 profile to v1 representation is 
not the goal of impala-profile-tool?


http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py@38
PS6, Line 38: 
get_profile_path('impala_profile_log_tpcds_compute_stats.expected.txt'))
> See above comment - this is actually kinda different from legacy
Got it, thanks!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Dec 2020 03:12:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

2020-12-21 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16884 )

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 2: Code-Review+2

(1 comment)

LGTM. Thanks for handling this!

http://gerrit.cloudera.org:8080/#/c/16884/2/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:

http://gerrit.cloudera.org:8080/#/c/16884/2/be/src/runtime/buffered-tuple-stream.h@563
PS2, Line 563:   friend class ArrayTupleStreamTest_TestArrayDeepCopy_Test;
 :   friend class ArrayTupleStreamTest_TestComputeRowSize_Test;
 :   friend class 
MultiNullableTupleStreamTest_TestComputeRowSize_Test;
> I notice that since IMPALA-10337 changed BufferedTupleStream::ComputeRowSiz
Yeah, please delete them.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Tue, 22 Dec 2020 02:31:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

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

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Patch Set 7:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Dec 2020 01:38:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Tim Armstrong (Code Review)
Hello Riza Suminto, Joe McDonnell, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..

IMPALA-9865: part 2/2: add verbosity to profile tool

Adds a --profile_verbosity option for impala-profile-tool with
the following levels:
* 0: minimal
* 1: legacy - matches old output, this is the default still
* 2: default - basic descriptive stats, used for V2 profile.
* 3: extended
* 4: full

Use the profile version in impala-profile-tool to dump the
more verbose output for the V2 profile while preserving the
same output for the legacy profile.

Reduce verbosity of v2 profile output - only include mean/min/max
by default. I intend to refine the output at the different
verbosity levels for the v2 profiles further as part of IMPALA-9382,
it is still fairly noisy.

Fix output with/without gen_experimental_profile - there
was a small difference in that the summary stats were not
output in the averaged profile.

Testing:
* Add an end-to-end test that generates output for a small
  profile log and compares against expected files.
* Tweak other profile tests to reflect changes to output.

Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
---
M be/src/util/impala-profile-tool.cc
M be/src/util/runtime-profile-counters.h
M be/src/util/runtime-profile-test.cc
M be/src/util/runtime-profile.cc
M be/src/util/runtime-profile.h
M bin/jenkins/dockerized-impala-run-tests.sh
M bin/rat_exclude_files.txt
A testdata/impala-profiles/README
A testdata/impala-profiles/impala_profile_log_tpcds_compute_stats
A testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.json
A 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.pretty.json
A 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.pretty_extended.json
A testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected.txt
A 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats_default.expected.txt
A 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats_extended.expected.txt
M testdata/workloads/tpch/queries/runtime-profile-aggregated.test
A tests/observability/test_profile_tool.py
M tests/query_test/test_scanners.py
18 files changed, 95,019 insertions(+), 124 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16881 )

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc
File be/src/util/impala-profile-tool.cc:

http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc@88
PS6, Line 88:   RuntimeProfileBase::Verbosity configured_verbosity;
> Is this meant to be initialized as RuntimeProfileBase::Verbosity::LEGACY ?
We shouldn't actually be using the default value, since we only use if it's 
non-empty and parsing succeeds. But clang-tidy was complaining about this 
anyway so fixing it :)

Compiling
/home/ubuntu/Impala/be/src/util/impala-profile-tool.cc:134:47: warning: 
variable 'configured_verbosity' may be uninitialized when used here 
[clang-diagnostic-conditional-uninitialized]
RuntimeProfileBase::Verbosity verbosity = configured_verbosity;
  ^
/home/ubuntu/Impala/be/src/util/impala-profile-tool.cc:88:3: note: variable 
'configured_verbosity' is declared here
  RuntimeProfileBase::Verbosity configured_verbosity;
  ^


http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc@136
PS6, Line 136:   verbosity = profile_version < 2 ? 
RuntimeProfileBase::Verbosity::LEGACY :
> In case of v1 profile input and --profile_verbosity=default, this line does
Yes and no. We can set the verbosity level to any level for any profile, but it 
only emits what is present in the thrift profile. Older v1 profiles won't have 
the TAggregatedRuntimeProfileNode set for the averaged profile, so it will only 
instantiate a regular RuntimeProfile object when parsing the thrift, which 
means it will only print the legacy profile information.


http://gerrit.cloudera.org:8080/#/c/16881/6/bin/rat_exclude_files.txt
File bin/rat_exclude_files.txt:

http://gerrit.cloudera.org:8080/#/c/16881/6/bin/rat_exclude_files.txt@157
PS6, Line 157: 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected
> Should we add `.txt` suffix for expected output in text format?
Done


http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py
File tests/observability/test_profile_tool.py:

http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py@36
PS6, Line 36: self._compare_profile_tool_output([],
> Since default verbosity might change in the future, can we add a test using
Leaving --profile_verbosity unset is different from setting it to --legacy 
since it will auto-detect based on the file contents. I think it's a good thing 
if the test fails when we change the behaviour.

I'd like to add some V2 profile tests when I finish up IMPALA-9382 and the 
format is final.


http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py@38
PS6, Line 38: 
get_profile_path('impala_profile_log_tpcds_compute_stats.expected'))
> Rename expected file name to "impala_profile_log_tpcds_compute_stats_legacy
See above comment - this is actually kinda different from legacy



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 22 Dec 2020 01:15:54 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10336: Coordinator return incorrect error to client

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

Change subject: IMPALA-10336: Coordinator return incorrect error to client
..

IMPALA-10336: Coordinator return incorrect error to client

Due to race condition, coordinator could set execution status as RPC
aborted due to cancellation. This internal error should not be
returned to client.
This patch fixed the issue by setting the backend status as CANCELLED
instead of ABORTED if the exec RPC was aborted due to cancellation.

Testing:
 - Manual tests
   Since this is a racy bug, I could only reproduce the situation by
   adding some artificial delays in 3 places: QueryExecMgr.StartQuery(),
   Coordinator.UpdateBackendExecStatus(), and
   Coordinator::StartBackendExec() when running test case
   test_scanners.py::TestOrc::test_type_conversions_hive3.
   Verified that the issue did not happen after applying this patch
   by running test_scanners.py::TestOrc::test_type_conversions_hive3
   in a loop for hours.
 - Passed exhausive test.

Change-Id: I75f252e43006c6ff6980800e3254672de396b318
Reviewed-on: http://gerrit.cloudera.org:8080/16849
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
2 files changed, 15 insertions(+), 2 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I75f252e43006c6ff6980800e3254672de396b318
Gerrit-Change-Number: 16849
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Wenzhe Zhou 


[Impala-ASF-CR] IMPALA-10336: Coordinator return incorrect error to client

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

Change subject: IMPALA-10336: Coordinator return incorrect error to client
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I75f252e43006c6ff6980800e3254672de396b318
Gerrit-Change-Number: 16849
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Tue, 22 Dec 2020 01:02:46 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9975 (part 1): Various refactors for admission control daemon

2020-12-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16890 )

Change subject: IMPALA-9975 (part 1): Various refactors for admission control 
daemon
..


Patch Set 1: Code-Review+2

Thanks, this is an improvement, the duplication was unnecessary.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e097e20458354f78bfc3477cac6fb3a2835f094
Gerrit-Change-Number: 16890
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Tue, 22 Dec 2020 00:52:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10317: Add query option that limits huge joins at runtime

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

Change subject: IMPALA-10317: Add query option that limits huge joins at runtime
..


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Gerrit-Change-Number: 16706
Gerrit-PatchSet: 7
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 22 Dec 2020 00:31:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10317: Add query option that limits huge joins at runtime

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

Change subject: IMPALA-10317: Add query option that limits huge joins at runtime
..


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Gerrit-Change-Number: 16706
Gerrit-PatchSet: 7
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 22 Dec 2020 00:31:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10317: Add query option that limits huge joins at runtime

2020-12-21 Thread Aman Sinha (Code Review)
Aman Sinha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16706 )

Change subject: IMPALA-10317: Add query option that limits huge joins at runtime
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Gerrit-Change-Number: 16706
Gerrit-PatchSet: 6
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 22 Dec 2020 00:27:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

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

Change subject: IMPALA-6671: Skip locked tables from topic updates
..

IMPALA-6671: Skip locked tables from topic updates

This change adds a mechanism for topic-update thread
to skip a table which is locked for more than a
configurable interval from the topic updates. This is
especially useful in scenarios where long running operations on a
locked table (refresh, recover partitions, compute stats) block the
topic update thread. This causes unrelated queries which are
waiting on metadata via topic updates (catalog-v1 mode)
to unnecessarily block.

The ideal solution of this problem would be to make HdfsTable
immutable so that there is no need for table lock. But that
is large change and not easily portable to older releases
of Impala. It would be taken up as a separate patch.

This change introduces 2 new configurations for catalogd:

1. topic_update_tbl_max_wait_time_ms: This defines the
maximum time in msecs the topic update thread waits on a locked table
before skipping the table from that iteration of topic updates.
The default value is 500. If this configuration is set to 0
the lock with timeout for topic update thread is disabled.
2. catalog_max_lock_skipped_topic_updates: This defines
the maximum number of distinct lock operations which are skipped
by topic update thread due to lock contention. Once this limit
is reached, topic update thread will block until it acquires
the table lock and adds it to the updates.

Testing:
1. Added a test case which introduces a simulated delay
in a few potentially long running statements. This causes the table
to be locked for a long time. The topic update thread skips
that table from updates and unrelated queries are unblocked
since they receive the required metadata from updates.
2. Added a test where multiple threads run blocking statements
in a loop to stress the table lock. It makes sure that topic
update thread is not starved and eventually blocks
on table lock by hitting the limit defined by
catalog_max_lock_skipped_topic_updates.
3. Ran exhaustive tests with default configurations.

Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Reviewed-on: http://gerrit.cloudera.org:8080/16549
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
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/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/TopicUpdateLog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
A tests/custom_cluster/test_topic_update_frequency.py
13 files changed, 698 insertions(+), 149 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 18
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

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

Change subject: IMPALA-6671: Skip locked tables from topic updates
..


Patch Set 17: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 17
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 22 Dec 2020 00:19:31 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9966: Add missing breaks in SetQueryOption

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

Change subject: IMPALA-9966: Add missing breaks in SetQueryOption
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I696204c7036c0059fe4eed2d23c8e947b6c53d8d
Gerrit-Change-Number: 16892
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 Dec 2020 22:50:11 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9966: Add missing breaks in SetQueryOption

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

Change subject: IMPALA-9966: Add missing breaks in SetQueryOption
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I696204c7036c0059fe4eed2d23c8e947b6c53d8d
Gerrit-Change-Number: 16892
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 Dec 2020 22:50:12 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9966: Add missing breaks in SetQueryOption

2020-12-21 Thread Bikramjeet Vig (Code Review)
Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16892 )

Change subject: IMPALA-9966: Add missing breaks in SetQueryOption
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I696204c7036c0059fe4eed2d23c8e947b6c53d8d
Gerrit-Change-Number: 16892
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 Dec 2020 22:49:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate

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

Change subject: IMPALA-10325: Parquet scan should use min/max statistics to 
skip pages based on equi-join predicate
..


Patch Set 38:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691
Gerrit-Change-Number: 16720
Gerrit-PatchSet: 38
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 21 Dec 2020 22:31:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

2020-12-21 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16884 )

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:

http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream-test.cc@1472
PS1, Line 1472: SimpleTupleStreamTest
> nit: To be consistent with other tests, I think the label should be "Simple
Done


http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream-test.cc@1485
PS1, Line 1485: bool b = stream.AddRow(batch->GetRow(j), );
> I think we can remove this TODO.
Done


http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream-test.cc@1508
PS1, Line 1508:
> Can we calculate 148 out based on the vars? I'm concerning it will introduc
It is not obvious for me to calculate it based on the vars.
So instead, I change it to look up stream.num_pages_ directly and add 
verification that num_pages_ > MAX_PAGE_ITER_DEBUG, given the workload.


http://gerrit.cloudera.org:8080/#/c/16884/2/be/src/runtime/buffered-tuple-stream.h
File be/src/runtime/buffered-tuple-stream.h:

http://gerrit.cloudera.org:8080/#/c/16884/2/be/src/runtime/buffered-tuple-stream.h@563
PS2, Line 563:   friend class ArrayTupleStreamTest_TestArrayDeepCopy_Test;
 :   friend class ArrayTupleStreamTest_TestComputeRowSize_Test;
 :   friend class 
MultiNullableTupleStreamTest_TestComputeRowSize_Test;
I notice that since IMPALA-10337 changed BufferedTupleStream::ComputeRowSize() 
from private to public, these lines are not required anymore.
Can I delete them along with this patch?


http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream.cc
File be/src/runtime/buffered-tuple-stream.cc:

http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream.cc@189
PS1, Line 189: \
> Comparing to the original codes, we should have "\n" here.
Done. Thanks for catching this!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 21 Dec 2020 22:14:24 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

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

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 2:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 21 Dec 2020 22:12:19 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate

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

Change subject: IMPALA-10325: Parquet scan should use min/max statistics to 
skip pages based on equi-join predicate
..


Patch Set 38:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16720/38/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/16720/38/tests/run-tests.py@216
PS38, Line 216: #
flake8: E265 block comment should start with '# '


http://gerrit.cloudera.org:8080/#/c/16720/38/tests/run-tests.py@219
PS38, Line 219: %
flake8: E131 continuation line unaligned for hanging indent



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691
Gerrit-Change-Number: 16720
Gerrit-PatchSet: 38
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Mon, 21 Dec 2020 22:09:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate

2020-12-21 Thread Qifan Chen (Code Review)
Qifan Chen has uploaded a new patch set (#38). ( 
http://gerrit.cloudera.org:8080/16720 )

Change subject: IMPALA-10325: Parquet scan should use min/max statistics to 
skip pages based on equi-join predicate
..

IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on 
equi-join predicate

This patch adds a new class of predicates called overlap predicates
to aid in the determination of whether a Parquet row group or a page
overlap with a range computed from an equi hash join. If not, then
the entire row group or page are skipped. An overlap predicate exists
as a min/max filter.

For the following query, the min and max in such a min/max filter are
computed with the values from the join column from table 'b' and become
fully available when the entire hash table is built. To evaluate the
overlap predicate, these two values are compared against the min/max
of each row group or page at the scan node for 'a'.

  select straight_join count(*)
  from lineitem_sorted_l_shipdate a join [SHUFFLE]
   lineitem_sorted_l_shipdate b
  where a.l_shipdate = b.l_receiptdate
  and b.l_commitdate = "1992-01-31";

An overlap predicate associated with the column type J (in hash table)
and scan column type S will be formed when one of the following is true:
   Both J and S are booleans
   Both J and S are integers (tinyint, smallint, int, or bigint)
   Both J and S are approximate numeric (float or double)
   Both J and S are decimals with the same precision and scale
   Both J and S are strings (STRING, CHAR or VARCHAR)
   Both J and S are date
   Both J and S are timestamp

Like any existing min/max filters, MAX_NUM_RUNTIME_FILTERS query option
does not apply to min/max filters created for overlap predicates.
The overlap predicates will always be evaluated, after the min/max
conjuncts (if any).

Two new run-time profile counters are added to report the number of row
groups or pages filtered out via the overlap predicates respectively:
  1. NumRuntimeFilteredRowGroups
  2. NumRuntimeFilteredPages

Testing:
1. Unit tested on various column types with TPCH and TPCDS tables.
   Benefits were significant when the join column on the outer table
   is sorted, or when the min/max boundary values of the pages or row
   groups are monotonic;
2. Added new tests in min_max_filters.test to demonstrate the number
   of filtered out pages and row groups with the two new profile counters;
2. Added new tests in runtime-filter-propagation.test to demonstrate
   that the overlap predicates work with different column types;
4. Added data type specific overlap method tests in
   min-max-filter-test.cc;
5. Core testing.

TBD in this patch:
1. Performance measurement.

To do in follow-up JIRAs:
1. Apply the overlap predicate on partition columns;
2. IR code-gen for various MinMaxFilter::EvalOverlap methods.

Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691
---
M be/src/exec/exec-node.h
M be/src/exec/hdfs-scan-node-base.cc
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.h
M be/src/exec/parquet/parquet-column-stats.cc
M be/src/exec/parquet/parquet-column-stats.h
M be/src/exec/partitioned-hash-join-builder.cc
M be/src/exec/scan-node.cc
M be/src/runtime/coordinator.cc
M be/src/runtime/date-value.cc
M be/src/runtime/date-value.h
M be/src/runtime/runtime-filter-ir.cc
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/util/min-max-filter-test.cc
M be/src/util/min-max-filter.cc
M be/src/util/min-max-filter.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/PlanNodes.thrift
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A 
testdata/workloads/functional-planner/queries/PlannerTest/disable-runtime-overlap-filter.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/runtime-filter-propagation.test
M testdata/workloads/functional-query/queries/QueryTest/min_max_filters.test
M tests/run-tests.py
31 files changed, 1,828 insertions(+), 239 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/16720/38
--
To view, visit http://gerrit.cloudera.org:8080/16720
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691
Gerrit-Change-Number: 16720
Gerrit-PatchSet: 38
Gerrit-Owner: Qifan Chen 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan 

[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

2020-12-21 Thread Riza Suminto (Code Review)
Hello Quanlong Huang, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..

IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

BufferedTupleStream::DebugString() iterate std::list that can
potentially grow very large. As consequent, the returned string can grow
large as well and cause a problem as previously happen in IMPALA-9851.
With this patch, BufferedTupleStream::DebugString() only include maximum
of 100 first pages of page list.

Testing:
- Add new be test SimpleTupleStreamTest.ShortDebugString in
  buffered-tuple-stream-test.cc
- Pass core tests

Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
---
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/buffered-tuple-stream.cc
M be/src/runtime/buffered-tuple-stream.h
3 files changed, 73 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 


[Impala-ASF-CR] IMPALA-9966: Add missing breaks in SetQueryOption

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

Change subject: IMPALA-9966: Add missing breaks in SetQueryOption
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I696204c7036c0059fe4eed2d23c8e947b6c53d8d
Gerrit-Change-Number: 16892
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 Dec 2020 20:57:14 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9793: Impala quickstart cluster with docker-compose

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

Change subject: IMPALA-9793: Impala quickstart cluster with docker-compose
..


Patch Set 9:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc0b862af40a368381ada7ec2a355fe4b0aa778c
Gerrit-Change-Number: 15966
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Dec 2020 20:46:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9975 (part 1): Various refactors for admission control daemon

2020-12-21 Thread Wenzhe Zhou (Code Review)
Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16890 )

Change subject: IMPALA-9975 (part 1): Various refactors for admission control 
daemon
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e097e20458354f78bfc3477cac6fb3a2835f094
Gerrit-Change-Number: 16890
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Mon, 21 Dec 2020 20:37:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9966: Add missing breaks in SetQueryOption

2020-12-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/16892


Change subject: IMPALA-9966: Add missing breaks in SetQueryOption
..

IMPALA-9966: Add missing breaks in SetQueryOption

Change-Id: I696204c7036c0059fe4eed2d23c8e947b6c53d8d
---
M be/src/service/query-options.cc
1 file changed, 7 insertions(+), 5 deletions(-)



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

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


[Impala-ASF-CR] IMPALA-9793: Impala quickstart cluster with docker-compose

2020-12-21 Thread Tim Armstrong (Code Review)
Hello Quanlong Huang, Grant Henke, Joe McDonnell, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-9793: Impala quickstart cluster with docker-compose
..

IMPALA-9793: Impala quickstart cluster with docker-compose

What works:
* A single node cluster can be started up with docker-compose
* HMS data is stored in Derby database in a docker volume
* Filesystem data is stored in a shared docker volume, using the
  localfs support in the Hadoop client.
* A Kudu cluster with a single master can be optionally added on
  to the Impala cluster.
* TPC-DS data can be loaded automatically by a data loading container.

We need to set up a docker network called quickstart-network,
purely because docker-compose insists on generating network names
with underscores, which are part of the FQDN and end up causing
problems with Java's URL parsing, which rejects these technically
invalid domain names.

How to run:

Instructions for running the quickstart cluster are in
docker/quickstart.yml.

How to build containers:

  ./buildall.sh -release -noclean -notests -ninja
  ninja quickstart_hms_image quickstart_client_image docker_images

Misc other stuff:
* Added more metadata to all images.

TODO:
* Test and instructions to run against Kudu quickstart
* Upload latest version of containers before merging.

Change-Id: Ifc0b862af40a368381ada7ec2a355fe4b0aa778c
---
M docker/CMakeLists.txt
A docker/docker-build.sh
M docker/impala_base/Dockerfile
A docker/quickstart-kudu-minimal.yml
A docker/quickstart-load-data.yml
A docker/quickstart.yml
A docker/quickstart_client/Dockerfile
A docker/quickstart_client/data-load-entrypoint.sh
A docker/quickstart_client/load_tpcds_kudu.sql
A docker/quickstart_client/load_tpcds_parquet.sql
A docker/quickstart_conf/hive-site.xml
A docker/quickstart_hms/Dockerfile
A docker/quickstart_hms/hms-entrypoint.sh
13 files changed, 2,913 insertions(+), 2 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifc0b862af40a368381ada7ec2a355fe4b0aa778c
Gerrit-Change-Number: 15966
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9975 (part 2): Introduce new admission control daemon

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

Change subject: IMPALA-9975 (part 2): Introduce new admission control daemon
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id677814b31e9193035e8cf0d08aba0ce388a0ad9
Gerrit-Change-Number: 16891
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Dec 2020 19:52:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9975 (part 1): Various refactors for admission control daemon

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

Change subject: IMPALA-9975 (part 1): Various refactors for admission control 
daemon
..


Patch Set 1:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e097e20458354f78bfc3477cac6fb3a2835f094
Gerrit-Change-Number: 16890
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Dec 2020 19:52:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9975 (part 2): Introduce new admission control daemon

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

Change subject: IMPALA-9975 (part 2): Introduce new admission control daemon
..


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/16891/1/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/16891/1/bin/start-impala-cluster.py@74
PS1, Line 74: t
flake8: E501 line too long (98 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/16891/1/bin/start-impala-cluster.py@293
PS1, Line 293: def build_admissiond_arg_list():
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py
File tests/common/impala_cluster.py:

http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py@106
PS1, Line 106: s
flake8: E501 line too long (102 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py@236
PS1, Line 236: i
flake8: E501 line too long (101 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py@566
PS1, Line 566: class AdmissiondProcess(BaseImpalaProcess):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/16891/1/tests/common/impala_cluster.py@571
PS1, Line 571: #
flake8: E265 block comment should start with '# '



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id677814b31e9193035e8cf0d08aba0ce388a0ad9
Gerrit-Change-Number: 16891
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Dec 2020 19:32:21 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9975 (part 1): Various refactors for admission control daemon

2020-12-21 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16890 )

Change subject: IMPALA-9975 (part 1): Various refactors for admission control 
daemon
..


Patch Set 1:

It may be helpful to look at this patch first: 
https://gerrit.cloudera.org/#/c/16891/ to understand the context here


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e097e20458354f78bfc3477cac6fb3a2835f094
Gerrit-Change-Number: 16890
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Dec 2020 19:31:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9975 (part 2): Introduce new admission control daemon

2020-12-21 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/16891


Change subject: IMPALA-9975 (part 2): Introduce new admission control daemon
..

IMPALA-9975 (part 2): Introduce new admission control daemon

A recent patch (IMPALA-9930) introduces a new admission control rpc
service, which can be configured to perform admission control for
coordinators. In that patch, the admission service runs in an impalad.

This patch separates the service out to run in a new daemon, called
the admissiond.

Some notable changes:
- Adds a new class, AdmissiondEnv, which performs the same function
  for the admissiond as ExecEnv does for impalads.
- The '/admission' http endpoint is exposed on the admissiond's webui
  if the admission control service is in use, otherwise it is exposed
  on coordinator impalad's webuis.
- Executors now never create an AdmissionController, and coordinators
  do not create one if configured to use an admissiond. This requires
  removing admission related info on the '/backends' endpoint and
  completely removing the '/admission' endpoint for these daemons.
- start-impala-cluster.py takes a new flag --enable_admission_service
  which configures the minicluster to have an admissiond with all
  coordinators using it for admisison control.

TODO: integrate this with the Docker infrastructure

Testing:
- Existing tests for the admission control serivce are modified to run
  with an admissiond.

Change-Id: Id677814b31e9193035e8cf0d08aba0ce388a0ad9
---
M CMakeLists.txt
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/CMakeLists.txt
M be/src/scheduling/admission-control-client.cc
M be/src/scheduling/admission-control-service.cc
M be/src/scheduling/admission-control-service.h
A be/src/scheduling/admissiond-env.cc
A be/src/scheduling/admissiond-env.h
A be/src/scheduling/admissiond-main.cc
M be/src/service/CMakeLists.txt
M be/src/service/daemon-main.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-http-handler.h
M be/src/service/impala-server.cc
M be/src/util/mem-info.cc
M be/src/util/mem-info.h
M bin/start-impala-cluster.py
M common/thrift/metrics.json
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_cluster.py
M tests/common/impala_service.py
M tests/custom_cluster/test_admission_controller.py
M www/backends.tmpl
24 files changed, 574 insertions(+), 185 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id677814b31e9193035e8cf0d08aba0ce388a0ad9
Gerrit-Change-Number: 16891
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 


[Impala-ASF-CR] IMPALA-9975 (part 1): Various refactors for admission control daemon

2020-12-21 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/16890


Change subject: IMPALA-9975 (part 1): Various refactors for admission control 
daemon
..

IMPALA-9975 (part 1): Various refactors for admission control daemon

This patch contains a variety of small refactors needed to enable the
new admission control daemon, to separate them out from the main patch
for ease of reviewing.

The following changes are made:
- A new class is introduced, DaemonEnv, which contains singleton
  objects common to all Impala daemons, currently just a MetricGroup
  and Webservers. The purpose is to reduce code duplication when the
  new admissiond daemon is added. This is analogous to how ExecEnv is
  used for impalad-specific singletons already.

  This patch modifies the catalogd and statestored to use DaemonEnv.
  impalads could also use DaemonEnv, but its tricky due to
  dependencies in the order of creation and initialization for objects
  such as the ReservationTracker and BufferPool relative to the
  MetricGroup and Webserver, so this is left for followup work.

- Direct use of ExecEnv in ImpalaServicePool ahd AdmissionController
  is removed, as the admissiond will also need to use these classes
  and it will not have an ExecEnv.

Testing:
- Passed a run of existing core tests.

Change-Id: I2e097e20458354f78bfc3477cac6fb3a2835f094
---
M be/src/catalog/catalogd-main.cc
M be/src/common/CMakeLists.txt
A be/src/common/daemon-env.cc
A be/src/common/daemon-env.h
M be/src/rpc/impala-service-pool.cc
M be/src/rpc/impala-service-pool.h
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/rpc-mgr-test.h
M be/src/rpc/rpc-mgr.cc
M be/src/rpc/rpc-mgr.h
M be/src/runtime/data-stream-test.cc
M be/src/runtime/exec-env.cc
M be/src/scheduling/admission-control-service.cc
M be/src/scheduling/admission-controller-test.cc
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/service/control-service.cc
M be/src/service/data-stream-service.cc
M be/src/statestore/statestored-main.cc
20 files changed, 215 insertions(+), 112 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e097e20458354f78bfc3477cac6fb3a2835f094
Gerrit-Change-Number: 16890
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall 


[Impala-ASF-CR] IMPALA-10336: Coordinator return incorrect error to client

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

Change subject: IMPALA-10336: Coordinator return incorrect error to client
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I75f252e43006c6ff6980800e3254672de396b318
Gerrit-Change-Number: 16849
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Mon, 21 Dec 2020 19:25:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10336: Coordinator return incorrect error to client

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

Change subject: IMPALA-10336: Coordinator return incorrect error to client
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I75f252e43006c6ff6980800e3254672de396b318
Gerrit-Change-Number: 16849
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Mon, 21 Dec 2020 19:25:23 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10336: Coordinator return incorrect error to client

2020-12-21 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16849 )

Change subject: IMPALA-10336: Coordinator return incorrect error to client
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I75f252e43006c6ff6980800e3254672de396b318
Gerrit-Change-Number: 16849
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Mon, 21 Dec 2020 19:25:09 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Riza Suminto (Code Review)
Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16881 )

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc
File be/src/util/impala-profile-tool.cc:

http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc@88
PS6, Line 88:   RuntimeProfileBase::Verbosity configured_verbosity;
Is this meant to be initialized as RuntimeProfileBase::Verbosity::LEGACY ?


http://gerrit.cloudera.org:8080/#/c/16881/6/be/src/util/impala-profile-tool.cc@136
PS6, Line 136:   verbosity = profile_version < 2 ? 
RuntimeProfileBase::Verbosity::LEGACY :
In case of v1 profile input and --profile_verbosity=default, this line does not 
seem to override it back to Verbosity::LEGACY.
Does this implies that both v1 and v2 profile input can be parsed into any 
verbosity level?


http://gerrit.cloudera.org:8080/#/c/16881/6/bin/rat_exclude_files.txt
File bin/rat_exclude_files.txt:

http://gerrit.cloudera.org:8080/#/c/16881/6/bin/rat_exclude_files.txt@157
PS6, Line 157: 
testdata/impala-profiles/impala_profile_log_tpcds_compute_stats.expected
Should we add `.txt` suffix for expected output in text format?


http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py
File tests/observability/test_profile_tool.py:

http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py@36
PS6, Line 36: self._compare_profile_tool_output([],
Since default verbosity might change in the future, can we add a test using 
argument `--profile_verbosity=legacy` ?
Should we also test between v1 vs v2 profile input when --profile_verbosity is 
not specified?


http://gerrit.cloudera.org:8080/#/c/16881/6/tests/observability/test_profile_tool.py@38
PS6, Line 38: 
get_profile_path('impala_profile_log_tpcds_compute_stats.expected'))
Rename expected file name to 
"impala_profile_log_tpcds_compute_stats_legacy.expected" for clarity?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Dec 2020 19:18:57 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

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

Change subject: IMPALA-6671: Skip locked tables from topic updates
..


Patch Set 17:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 17
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Mon, 21 Dec 2020 18:44:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

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

Change subject: IMPALA-6671: Skip locked tables from topic updates
..


Patch Set 17: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 17
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Mon, 21 Dec 2020 18:44:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

2020-12-21 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16549 )

Change subject: IMPALA-6671: Skip locked tables from topic updates
..


Patch Set 16: Code-Review+2

There were minor updates since the last update. Carrying forward the votes from 
Tim and Quanlong.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 16
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Mon, 21 Dec 2020 18:36:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

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

Change subject: IMPALA-6671: Skip locked tables from topic updates
..


Patch Set 16:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 16
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Mon, 21 Dec 2020 18:27:33 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

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

Change subject: IMPALA-6671: Skip locked tables from topic updates
..


Patch Set 16:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16549/16/tests/custom_cluster/test_topic_update_frequency.py
File tests/custom_cluster/test_topic_update_frequency.py:

http://gerrit.cloudera.org:8080/#/c/16549/16/tests/custom_cluster/test_topic_update_frequency.py@54
PS16, Line 54: l
flake8: E501 line too long (146 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/16549/16/tests/custom_cluster/test_topic_update_frequency.py@120
PS16, Line 120: f
flake8: E126 continuation line over-indented for hanging indent


http://gerrit.cloudera.org:8080/#/c/16549/16/tests/custom_cluster/test_topic_update_frequency.py@152
PS16, Line 152: l
flake8: E501 line too long (146 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 16
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Mon, 21 Dec 2020 18:06:42 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

2020-12-21 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has uploaded a new patch set (#16). ( 
http://gerrit.cloudera.org:8080/16549 )

Change subject: IMPALA-6671: Skip locked tables from topic updates
..

IMPALA-6671: Skip locked tables from topic updates

This change adds a mechanism for topic-update thread
to skip a table which is locked for more than a
configurable interval from the topic updates. This is
especially useful in scenarios where long running operations on a
locked table (refresh, recover partitions, compute stats) block the
topic update thread. This causes unrelated queries which are
waiting on metadata via topic updates (catalog-v1 mode)
to unnecessarily block.

The ideal solution of this problem would be to make HdfsTable
immutable so that there is no need for table lock. But that
is large change and not easily portable to older releases
of Impala. It would be taken up as a separate patch.

This change introduces 2 new configurations for catalogd:

1. topic_update_tbl_max_wait_time_ms: This defines the
maximum time in msecs the topic update thread waits on a locked table
before skipping the table from that iteration of topic updates.
The default value is 500. If this configuration is set to 0
the lock with timeout for topic update thread is disabled.
2. catalog_max_lock_skipped_topic_updates: This defines
the maximum number of distinct lock operations which are skipped
by topic update thread due to lock contention. Once this limit
is reached, topic update thread will block until it acquires
the table lock and adds it to the updates.

Testing:
1. Added a test case which introduces a simulated delay
in a few potentially long running statements. This causes the table
to be locked for a long time. The topic update thread skips
that table from updates and unrelated queries are unblocked
since they receive the required metadata from updates.
2. Added a test where multiple threads run blocking statements
in a loop to stress the table lock. It makes sure that topic
update thread is not starved and eventually blocks
on table lock by hitting the limit defined by
catalog_max_lock_skipped_topic_updates.
3. Ran exhaustive tests with default configurations.

Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
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/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/TopicUpdateLog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
A tests/custom_cluster/test_topic_update_frequency.py
13 files changed, 698 insertions(+), 149 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 16
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

2020-12-21 Thread Vihang Karajgaonkar (Code Review)
Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16549 )

Change subject: IMPALA-6671: Skip locked tables from topic updates
..


Patch Set 15:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1311
PS15, Line 1311: to
> nit: to get
Done


http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1325
PS15, Line 1325: LOG.warn("Topic update thread blocking until lock is 
acquired for table {}",
> This is too verbose when topicUpdateTblLockMaxWaitTimeMs_ is 0. Almost all
makes sense. Done.


http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2235
PS15, Line 2235: the
> nit: remove "the"
Done


http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py
File tests/custom_cluster/test_topic_update_frequency.py:

http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py@20
PS15, Line 20: @SkipIfS3.variable_listing_times
> flake8: E302 expected 2 blank lines, found 1
Done


http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py@66
PS15, Line 66: is
> nit: without?
Thanks for pointing this out.


http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py@70
PS15, Line 70: is
> nit: with?
Done


http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py@106
PS15, Line 106: # there is no other good way other than to wait for some 
time to make sure that
  : # the slow query has been submitted and being compiled 
before we start the
  : # non_blocking queries
  : time.sleep(1)
> Just for comments, we can use "wait_for_state" to wait for the CREATE state
Actually, I had used this before in my earlier attempt but I found that to be 
flaky. My understanding is the CREATE state will be set when admission 
controller accepts the query? There is still a time delay between when the 
query is in create state and the query takes the table lock in catalogd.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 15
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Mon, 21 Dec 2020 17:50:11 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has removed a vote on this change.

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Removed Verified-1 by Impala Public Jenkins 
--
To view, visit http://gerrit.cloudera.org:8080/16881
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9865: part 2/2: add verbosity to profile tool

2020-12-21 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16881 )

Change subject: IMPALA-9865: part 2/2: add verbosity to profile tool
..


Patch Set 6:

The -1 is a minor clang-tidy issue.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82618a813e29af7996dfaed78873b2a73bc0231d
Gerrit-Change-Number: 16881
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Joe McDonnell 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 21 Dec 2020 17:30:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10317: Add query option that limits huge joins at runtime

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

Change subject: IMPALA-10317: Add query option that limits huge joins at runtime
..


Patch Set 6:

Build Successful

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Gerrit-Change-Number: 16706
Gerrit-PatchSet: 6
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 Dec 2020 15:06:06 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10317: Add query option that limits huge joins at runtime

2020-12-21 Thread Fucun Chu (Code Review)
Fucun Chu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16706 )

Change subject: IMPALA-10317: Add query option that limits huge joins at runtime
..


Patch Set 6:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/16706/5//COMMIT_MSG@9
PS5, Line 9:  a join no
> You may want to mention why it is useful even though it may be somewhat obv
Done


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

http://gerrit.cloudera.org:8080/#/c/16706/3/be/src/runtime/coordinator.h@230
PS3, Line 230: /// Total num rows produced by each join node. The key is 
join node id.
> Good !
Done


http://gerrit.cloudera.org:8080/#/c/16706/5/common/protobuf/control_service.proto
File common/protobuf/control_service.proto:

http://gerrit.cloudera.org:8080/#/c/16706/5/common/protobuf/control_service.proto@261
PS5, Line 261:   map per_join_rows_produced = 16;
> Mark this as 'optional' field ?
Maps cannot be repeated, optional, or required.
from: https://developers.google.com/protocol-buffers/docs/proto#maps



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Gerrit-Change-Number: 16706
Gerrit-PatchSet: 6
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Mon, 21 Dec 2020 14:44:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-10317: Add query option that limits huge joins at runtime

2020-12-21 Thread Fucun Chu (Code Review)
Fucun Chu has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/16706 )

Change subject: IMPALA-10317: Add query option that limits huge joins at runtime
..

IMPALA-10317: Add query option that limits huge joins at runtime

This patch adds support for limiting the rows produced by a join node
such that runaway join queries can be prevented.

The limit is specified by a query option. Queries exceeding that limit
get terminated. The checking runs periodically, so the actual rows
produced may go somewhat over the limit.

JOIN_ROWS_PRODUCED_LIMIT is exposed as an advanced query option.

Rows produced Query profile is updated to include query wide and per
backend metrics for RowsReturned. Example from "
set JOIN_ROWS_PRODUCED_LIMIT = 1000;
select count(*) from tpch_parquet.lineitem l1 cross join
(select * from tpch_parquet.lineitem l2 limit 5) l3;":

NESTED_LOOP_JOIN_NODE (id=2):
   - InactiveTotalTime: 107.534ms
   - PeakMemoryUsage: 16.00 KB (16384)
   - ProbeRows: 1.02K (1024)
   - ProbeTime: 0.000ns
   - RowsReturned: 10.00M (10002025)
   - RowsReturnedRate: 749.58 K/sec
   - TotalTime: 13s337ms

Testing:
 Added tests for JOIN_ROWS_PRODUCED_LIMIT

Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator.h
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/service/impala-server.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/protobuf/control_service.proto
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M common/thrift/generate_error_codes.py
M 
testdata/workloads/functional-query/queries/QueryTest/query-resource-limits.test
14 files changed, 167 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idbca7e053b61b4e31b066edcfb3b0398fa859d02
Gerrit-Change-Number: 16706
Gerrit-PatchSet: 6
Gerrit-Owner: Fucun Chu 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: Fucun Chu 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
..


Patch Set 13: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 13
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Mon, 21 Dec 2020 14:29:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
..

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE .
2. DROP ROLE .
3. GRANT ROLE  TO GROUP .
4. REVOKE ROLE  FROM GROUP .
5. GRANT  ON  TO ROLE .
6. REVOKE  ON  FROM ROLE .
7. SHOW GRANT ROLE  ON .
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP .

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE ,
GRANT/REVOKE ROLE  TO/FROM GROUP , and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE  ON , which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE  and
GRANT/REVOKE  TO/FROM GROUP . The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

We also note that in this patch we added the decorator of
@pytest.mark.execute_serially to each test in test_ranger.py since we
have observed that in some cases, e.g., if we are only running the E2E
tests in the Jenkins environment, some tests do not seem to be executed
sequentially.

Testing:
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.
 - Verified that test_ranger.py passes in a local development
   environment.
 - Verified that the patch passes the exhaustive tests in the DEBUG
   build.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Reviewed-on: http://gerrit.cloudera.org:8080/16837
Reviewed-by: Quanlong Huang 
Tested-by: Impala Public Jenkins 
---
M bin/create-test-configuration.sh
M 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A 

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
..


Patch Set 13:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 13
Gerrit-Owner: Fang-Yu Rao 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Comment-Date: Mon, 21 Dec 2020 08:54:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-10374: Limit iteration at BufferedTupleStream::DebugString

2020-12-21 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16884 )

Change subject: IMPALA-10374: Limit iteration at 
BufferedTupleStream::DebugString
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream-test.cc
File be/src/runtime/buffered-tuple-stream-test.cc:

http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream-test.cc@1472
PS1, Line 1472: SimpleTupleStreamTest
nit: To be consistent with other tests, I think the label should be 
"SimpleTupleStreamTest::ShortDebugString"


http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream-test.cc@1485
PS1, Line 1485: // TODO: test that AddRow succeeds after freeing memory.
I think we can remove this TODO.


http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream-test.cc@1508
PS1, Line 1508: 148
Can we calculate 148 out based on the vars? I'm concerning it will introduce 
flakiness.


http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream.cc
File be/src/runtime/buffered-tuple-stream.cc:

http://gerrit.cloudera.org:8080/#/c/16884/1/be/src/runtime/buffered-tuple-stream.cc@189
PS1, Line 189:  
Comparing to the original codes, we should have "\n" here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6626c8d54f35f303c01f85be1dd9aa54c8ad9a2d
Gerrit-Change-Number: 16884
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Riza Suminto 
Gerrit-Comment-Date: Mon, 21 Dec 2020 08:45:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6671: Skip locked tables from topic updates

2020-12-21 Thread Quanlong Huang (Code Review)
Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16549 )

Change subject: IMPALA-6671: Skip locked tables from topic updates
..


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1325
PS15, Line 1325: LOG.warn("Topic update thread blocking until lock is 
acquired for table {}",
This is too verbose when topicUpdateTblLockMaxWaitTimeMs_ is 0. Almost all 
tables will leave one log line here because their version <= ctx.toVersion. 
Could you change the condition to this?

 if (topicUpdateTblLockMaxWaitTimeMs_ > 0 && !lockWithTimeout)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655
Gerrit-Change-Number: 16549
Gerrit-PatchSet: 15
Gerrit-Owner: Vihang Karajgaonkar 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Shant Hovsepian 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Mon, 21 Dec 2020 08:12:25 +
Gerrit-HasComments: Yes