[Impala-ASF-CR] IMPALA-7187: Fix test group impersonation running inside Docker

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/10772 )

Change subject: IMPALA-7187: Fix test_group_impersonation running inside Docker
..

IMPALA-7187: Fix test_group_impersonation running inside Docker

test_group_impersonation fails when running with test-with-docker.py
because os.getgroups() returns empty in Docker for the authorized
proxy groups which results in denying all access. The patch is to use
a group name from a current GID.

Testing:
- Ran all test_authorization with test-with-docker.py.

Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
---
M tests/authorization/test_authorization.py
1 file changed, 1 insertion(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Gerrit-Change-Number: 10772
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-7187: Fix test group impersonation running inside Docker

[Fredy Wijaya (Code Review)]

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

Change subject: IMPALA-7187: Fix test_group_impersonation running inside Docker
..


Patch Set 3:

I found the issue. So I decided to fix it instead of skipping it.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Gerrit-Change-Number: 10772
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 20 Jun 2018 07:02:36 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6119: Fix issue with multiple partitions sharing same location

[Gabor Kaszab (Code Review)]

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

Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same 
location
..


Patch Set 16:

Pls ignore the comments for Patch Set 12. They were some leftovers. Sorry for 
the noise.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a
Gerrit-Change-Number: 10543
Gerrit-PatchSet: 16
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 20 Jun 2018 07:40:02 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3307: Add support for IANA time-zone db

[Attila Jeges (Code Review)]

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

Change subject: IMPALA-3307: Add support for IANA time-zone db
..


Patch Set 19:

> Are we ready to go ahead and merge? Would be good to run exhaustive
 > tests + ASAN before merging just to be sure we aren't going to
 > break anything.

Exhaustive and ASAN failed because of a flaky test (IMPALA-7181).
Exhaustive: 
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/2336/
ASAN: 
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/2337/

Since IMPALA-7181 was resolved yesterday, I've rebased the patch-set and 
restarted the tests this morning.
Exhaustive: 
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/2345/
ASAN: 
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/2346/

Hopefully they will pass this time.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93c1fbffe81f067919706e30db0a34d0e58e7e77
Gerrit-Change-Number: 9986
Gerrit-PatchSet: 19
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 20 Jun 2018 14:00:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Tim Armstrong (Code Review)]

Hello Sailesh Mukil, anujphadke,

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

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

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..

IMPALA-7151: Rework ephemeral port assignment for be tests

Instead of using FindUnusedEphemeralPort() to choose
many ephemeral ports for in-process servers, we use
Thrift's builtin wildcard ports, where it picks an
ephemeral port when the configured port is 0. This
should be more robust because there's no window where
something else can steal our port. Only Thrift 0.9.2+
support the getPort() method required for this.

This does not necessarily make all ImpalaServer
functionality work with ports configured to 0,
just enough to get expr-test and session-expiry-test
to work. E.g. various strings and names get port 0 in
them and query execution may not work end-to-end.

Some tests still use FindUnusedEphemeralPort() to
pick a single port at a time. We haven't seen that usage pattern
to be flaky with any frequency and it appears to be more work
to fix those, so I left them for now.

Testing:
Ran core tests.

Cherry-picks: not for 2.x

Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/exprs/expr-test.cc
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/rpc/rpc-mgr-test-base.h
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M be/src/util/network-util.cc
M be/src/util/network-util.h
23 files changed, 175 insertions(+), 128 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 2:

(8 comments)

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

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/rpc/thrift-server.cc@366
PS2, Line 366: TServerSocket
> Are we sure that this change from TServerTransport to TServerSocket doesn't
TServerSocket is a subclass of TServerTransport - I needed to use the subclass 
here for getPort() - 
https://github.com/apache/thrift/blob/0.9.3/lib/cpp/src/thrift/transport/TServerSocket.h#L116

So yeah, shouldn't limit us at all.


http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/rpc/thrift-server.cc@452
PS2, Line 452: If port was 0
> nit: "If port_ was initially configured as 0..."
I think I meant the word port, but referring to the variable is probably 
clearer.


http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/runtime/exec-env.h
File be/src/runtime/exec-env.h:

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/runtime/exec-env.h@222
PS2, Line 222:   TNetworkAddress configured_backend_address_;
> Why make this public?
I'm not sure why I did this... reverted.


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

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/scheduling/scheduler.h@96
PS2, Line 96: UpdateLocalBackendForBeTest
> UpdateLocalBackendAddrForBeTest
Done


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

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.h@142
PS2, Line 142:   ///respective service will not be started.
> Update comment with new behavior about BE tests passing in port=0.
Done


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

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.cc@2070
PS2, Line 2070: >=
> > 0
Thanks for catching this.


http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/statestore/statestore-subscriber.h
File be/src/statestore/statestore-subscriber.h:

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/statestore/statestore-subscriber.h@190
PS2, Line 190:   /// Address that the heartbeat service should be started on. 
Initialised in constructor,
 :   /// updated in Start() with the actual port if the wildcard 
port 0 was specified.
 :   TNetworkAddress heartbeat_address_;
> Any reason this was moved down? Prefer leaving it where it was as surroundi
It's protected by the lock now, since it's modified in start rather than 
constant for the lifetime of the object. It was only ever referenced under the 
lock before anyway.


http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/util/network-util.cc
File be/src/util/network-util.cc:

http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/util/network-util.cc@196
PS2, Line 196: server_address.sin_port = htons(port);
> I think we can set this to 0 and bind to a random port instead of looping t
Nice catch! I hadn't really thought about it. We talked out of band and my 
concern was that doing this might increase the risk of collisions. But it 
sounds like it probably makes things better because linux apparently allocates 
them pseudo-sequentially: https://eklitzke.org/binding-on-port-zero . That 
whole blog post is really relevant.

I think you said you were going to look at doing it, but I can also do as a 
follow-on if needed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 16:10:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..


Patch Set 11: Code-Review+2

Carry +2, fix obvious missing reference found by clang-tidy


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 11
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 16:16:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Dan Hecht (Code Review)]

Hello Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..

IMPALA-7046: introduce "global" debug_actions

The motivation is to add jitter to backend startup in test_failpoints.
The race in IMPALA-7033 can be reproduced by adding jitter to the exec
rpcs when some backends fail. Let's add jitter to test_failpoints to get
better coverage of exec startup races.

This builds on top of the debug action extensions added in the async
admission control patch by allowing the new "global" debug actions
(i.e. actions that can be used in points outside of the ExecNodes).
See the code comments for details.

For now, we're only using the SLEEP and JITTER commands, but I've
included a FAIL command as well since I'll want to use that to write a
test for IMPALA-6788 to simulate exec rpc failure.

Note that I don't bother resolving the actions ahead of time (like we do
for ExecNode actions). It doesn't seem worth it since the resolution
only needs to occur after we've matched the label and I don't expect the
same label to be hit many times within a single thread. We can always
optimize this later if needed.

Testing:
- Verified that test_failpoints can reproduce the race in
  IMPALA-7033 by reverting that fix and testing.
- Ran the modified tests and grepped the impalad log to see
  that the sleeps are still occuring.
- Manually verify global FAIL command (in a build with another patch).
- Manually verified invalid debug_actions (both ExecNode and global)

Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
---
M be/src/runtime/coordinator.cc
M be/src/runtime/debug-options.cc
M be/src/scheduling/admission-controller.cc
M be/src/service/client-request-state.cc
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M common/thrift/ImpalaService.thrift
M tests/custom_cluster/test_admission_controller.py
M tests/failure/test_failpoints.py
M tests/query_test/test_observability.py
10 files changed, 201 insertions(+), 70 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 11
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..


Patch Set 12: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 12
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 16:16:38 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..


Patch Set 12:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 12
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 16:16:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7150: fixes jni frame scope

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10746 )

Change subject: IMPALA-7150: fixes jni frame scope
..


Patch Set 1:

(1 comment)

I have not been able to repro-- this particular issue has been rare as far as 
I've seen. Instead, given the cores and prev. change, I've treated this more as 
a revert to prev. memory behavior.

http://gerrit.cloudera.org:8080/#/c/10746/1/be/src/exprs/hive-udf-call.cc
File be/src/exprs/hive-udf-call.cc:

http://gerrit.cloudera.org:8080/#/c/10746/1/be/src/exprs/hive-udf-call.cc@202
PS1, Line 202:
> trailing whitespace.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Gerrit-Change-Number: 10746
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:01:26 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7150: fixes jni frame scope

[Vuk Ercegovac (Code Review)]

Hello Tim Armstrong,

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

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

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

Change subject: IMPALA-7150: fixes jni frame scope
..

IMPALA-7150: fixes jni frame scope

A previous change (https://gerrit.cloudera.org/#/c/9968/)
reduced the time that a lib-cache entry was held when initializing
a udf. It did so by introducing a new scope. However, that scope
captured a jni frame, which resulted in an earlier frame pop
than before. This change decouples the frame scope from the lib-cache
scope so that the frame is popped at the end of the method (as before)
instread of with the lib-cache scope.

Testing:
- core, asan tests pass

Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
---
M be/src/exprs/hive-udf-call.cc
1 file changed, 7 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Gerrit-Change-Number: 10746
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Sailesh Mukil (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc@223
PS4, Line 223: heartbeat_address_.port = heartbeat_server_->port();
Are we sure that setting this outside the scope of 'lock_' won't cause any kind 
of race?

I don't exactly understand the comment on L193-195 since Register() happens 
only after dropping the lock_, so I'm not able to make the call.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:05:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7121: Clean up partitionIds from HdfsTable

[Gabor Kaszab (Code Review)]

Hello Bharath Vissapragada, Zoltan Borok-Nagy, Sailesh Mukil, Todd Lipcon, Tim 
Armstrong,

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

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

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

Change subject: IMPALA-7121: Clean up partitionIds_ from HdfsTable
..

IMPALA-7121: Clean up partitionIds_ from HdfsTable

The purpose of introducing partitionIds_ member to HdfsTable was to be
able to return the IDs of all the current partitions in constant time.
Apparently, partitionMap_ also contains these IDs as the key of the
map and this is accessible via keySet() also in constant time. It
seems reasonable then to remove partitionIds_ and use
partitionMap_.keySet() in getPartitionIds() to save some memory.

One thing needs extra attention here is that modifying the result of
keySet() would also modify partitionMap_ and we should avoid doing
this. On every callsites of getPartitionIds() the first step the
caller does is to copy the received items to a separate set. So as a
solution getPartitionIds() internally creates a copy of the keySet(),
removes the default partition and returns this copy to be sure that
partitionMap_ can't be altered. The caller sites are also changed not
to copy the items but to simpy use the set they received. This will
guarantee that we don't regress the computing complexity of getting
the partition IDs.

Change-Id: I8b5a480e570aeae565fafd4f3e2b279e7a98c7da
---
M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java
M fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
4 files changed, 17 insertions(+), 19 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b5a480e570aeae565fafd4f3e2b279e7a98c7da
Gerrit-Change-Number: 10654
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc@223
PS4, Line 223: heartbeat_address_.port = heartbeat_server_->port();
> Are we sure that setting this outside the scope of 'lock_' won't cause any
Am I misreading something - isn't this within the same scoped block as 
exclusive_lock?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:40:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7150: fixes jni frame scope

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7150: fixes jni frame scope
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Gerrit-Change-Number: 10746
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:41:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7150: fixes jni frame scope

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7150: fixes jni frame scope
..


Patch Set 2:

No worries, we can continue to monitor builds since we do reproduce at some 
rate there.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Gerrit-Change-Number: 10746
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:41:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Sailesh Mukil (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 4: Code-Review+2

(1 comment)

LGTM. Please also check with Anuj if he has any further comments.

http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc@223
PS4, Line 223: heartbeat_address_.port = heartbeat_server_->port();
> Am I misreading something - isn't this within the same scoped block as excl
Oh my bad. I completely misinterpreted the scope of the lock by looking at the 
wrong curly braces.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:43:17 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc
File be/src/statestore/statestore-subscriber.cc:

http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc@223
PS4, Line 223: heartbeat_address_.port = heartbeat_server_->port();
> Oh my bad. I completely misinterpreted the scope of the lock by looking at
Np, it's better than you don't assume that I got it right.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:46:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3307: Add support for IANA time-zone db

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3307: Add support for IANA time-zone db
..


Patch Set 19: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93c1fbffe81f067919706e30db0a34d0e58e7e77
Gerrit-Change-Number: 9986
Gerrit-PatchSet: 19
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:46:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3307: Add support for IANA time-zone db

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-3307: Add support for IANA time-zone db
..


Patch Set 19:

Thanks for running those tests!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93c1fbffe81f067919706e30db0a34d0e58e7e77
Gerrit-Change-Number: 9986
Gerrit-PatchSet: 19
Gerrit-Owner: Attila Jeges 
Gerrit-Reviewer: Attila Jeges 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:47:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7121: Clean up partitionIds from HdfsTable

[Todd Lipcon (Code Review)]

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

Change subject: IMPALA-7121: Clean up partitionIds_ from HdfsTable
..


Patch Set 4:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10654/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@593
PS4, Line 593: return Sets.newHashSet(partitionMap_.keySet());
Now that we don't need to modify the key set, could we just use 
Collections.unmodifiableSet() here, and then keep the call-sites as-is using 
addAll where they might try to modify it? Then we can leave the interface type 
as Set<> and this patch will only be a few lines?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b5a480e570aeae565fafd4f3e2b279e7a98c7da
Gerrit-Change-Number: 10654
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:52:21 +
Gerrit-HasComments: Yes

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

[Tim Armstrong (Code Review)]

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

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


Patch Set 15:

Maybe Taras can comment on whether that precondition is valid in this case.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 17:53:16 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Added another scenario when REFRESH is required

[Alex Rodoni (Code Review)]

Alex Rodoni has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10773


Change subject: [DOCS] Added another scenario when REFRESH is required
..

[DOCS] Added another scenario when REFRESH is required

Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
---
M docs/topics/impala_refresh.xml
1 file changed, 5 insertions(+), 4 deletions(-)



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

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

[Impala-ASF-CR] IMPALA-7150: fixes jni frame scope

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7150: fixes jni frame scope
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Gerrit-Change-Number: 10746
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 18:08:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7150: fixes jni frame scope

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7150: fixes jni frame scope
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Gerrit-Change-Number: 10746
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 18:08:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 19:06:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 4:

Checked with Anuj directly...


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 19:06:28 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 19:06:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..


Patch Set 12: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 12
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 19:37:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..

IMPALA-7046: introduce "global" debug_actions

The motivation is to add jitter to backend startup in test_failpoints.
The race in IMPALA-7033 can be reproduced by adding jitter to the exec
rpcs when some backends fail. Let's add jitter to test_failpoints to get
better coverage of exec startup races.

This builds on top of the debug action extensions added in the async
admission control patch by allowing the new "global" debug actions
(i.e. actions that can be used in points outside of the ExecNodes).
See the code comments for details.

For now, we're only using the SLEEP and JITTER commands, but I've
included a FAIL command as well since I'll want to use that to write a
test for IMPALA-6788 to simulate exec rpc failure.

Note that I don't bother resolving the actions ahead of time (like we do
for ExecNode actions). It doesn't seem worth it since the resolution
only needs to occur after we've matched the label and I don't expect the
same label to be hit many times within a single thread. We can always
optimize this later if needed.

Testing:
- Verified that test_failpoints can reproduce the race in
  IMPALA-7033 by reverting that fix and testing.
- Ran the modified tests and grepped the impalad log to see
  that the sleeps are still occuring.
- Manually verify global FAIL command (in a build with another patch).
- Manually verified invalid debug_actions (both ExecNode and global)

Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Reviewed-on: http://gerrit.cloudera.org:8080/10690
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/runtime/coordinator.cc
M be/src/runtime/debug-options.cc
M be/src/scheduling/admission-controller.cc
M be/src/service/client-request-state.cc
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M common/thrift/ImpalaService.thrift
M tests/custom_cluster/test_admission_controller.py
M tests/failure/test_failpoints.py
M tests/query_test/test_observability.py
10 files changed, 201 insertions(+), 70 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 13
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] [DOCS] Added another scenario when REFRESH is required

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10773 )

Change subject: [DOCS] Added another scenario when REFRESH is required
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10773/1/docs/topics/impala_refresh.xml
File docs/topics/impala_refresh.xml:

http://gerrit.cloudera.org:8080/#/c/10773/1/docs/topics/impala_refresh.xml@90
PS1, Line 90: appending, or altering
why not say "modifying" or "altering"? Is there a distinction between appending 
and altering that the user should be aware of?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
Gerrit-Change-Number: 10773
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 20:20:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6119: Fix issue with multiple partitions sharing same location

[Bharath Vissapragada (Code Review)]

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

Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same 
location
..


Patch Set 17:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10543/15/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:

http://gerrit.cloudera.org:8080/#/c/10543/15/tests/metadata/test_partition_metadata.py@177
PS15, Line 177: pytest.skip()
> I preferred to disable this test for two reasons:
The problem with skipping though is that no one will remember to un-skip this 
test again once HIVE-19830 is fixed, since it does not show up anywhere. If an 
assert fails causing a test failure, someone will revisit this and fix the 
assert/behavior of the test.


http://gerrit.cloudera.org:8080/#/c/10543/17/tests/metadata/test_partition_metadata.py
File tests/metadata/test_partition_metadata.py:

http://gerrit.cloudera.org:8080/#/c/10543/17/tests/metadata/test_partition_metadata.py@168
PS17, Line 168: false
False.

This was caught by impala-flake8. Can you run it on this file and fix the 
errors? (not everything is related to this patch though)

$ impala-flake8 tests/metadata/test_partition_metadata.py 

tests/metadata/test_partition_metadata.py:22:5: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:23:1: F401 
'tests.util.filesystem_utils.get_fs_path' imported but unused
tests/metadata/test_partition_metadata.py:29:19: E201 whitespace after '{'
tests/metadata/test_partition_metadata.py:30:5: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:30:26: E202 whitespace before '}'
tests/metadata/test_partition_metadata.py:35:1: E302 expected 2 blank lines, 
found 1
tests/metadata/test_partition_metadata.py:49:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:55:27: E261 at least two spaces 
before inline comment
tests/metadata/test_partition_metadata.py:73:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:75:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:128:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:130:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:132:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:149:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:149:39: E703 statement ends with a 
semicolon
tests/metadata/test_partition_metadata.py:168:12: F821 undefined name 'false'
tests/metadata/test_partition_metadata.py:194:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:196:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:224:76: E502 the backslash is 
redundant between brackets
tests/metadata/test_partition_metadata.py:225:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:226:75: E502 the backslash is 
redundant between brackets
tests/metadata/test_partition_metadata.py:227:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:238:74: E502 the backslash is 
redundant between brackets
tests/metadata/test_partition_metadata.py:239:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:271:5: F841 local variable 
'file_format' is assigned to but never used
tests/metadata/test_partition_metadata.py:277:9: E122 continuation line missing 
indentation or outdented
tests/metadata/test_partition_metadata.py:287:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:290:9: E122 continuation line missing 
indentation or outdented
tests/metadata/test_partition_metadata.py:301:9: E122 continuation line missing 
indentation or outdented
tests/metadata/test_partition_metadata.py:307:9: E128 continuation line 
under-indented for visual indent
tests/metadata/test_partition_metadata.py:315:9: E122 continuation line missing 
indentation or outdented



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a
Gerrit-Change-Number: 10543
Gerrit-PatchSet: 17
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoltan Borok-Nagy 
G

[Impala-ASF-CR] IMPALA-7150: fixes jni frame scope

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7150: fixes jni frame scope
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Gerrit-Change-Number: 10746
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 21:33:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7150: fixes jni frame scope

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7150: fixes jni frame scope
..

IMPALA-7150: fixes jni frame scope

A previous change (https://gerrit.cloudera.org/#/c/9968/)
reduced the time that a lib-cache entry was held when initializing
a udf. It did so by introducing a new scope. However, that scope
captured a jni frame, which resulted in an earlier frame pop
than before. This change decouples the frame scope from the lib-cache
scope so that the frame is popped at the end of the method (as before)
instread of with the lib-cache scope.

Testing:
- core, asan tests pass

Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Reviewed-on: http://gerrit.cloudera.org:8080/10746
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/exprs/hive-udf-call.cc
1 file changed, 7 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I38d482dde6ed7f8857ae565bf62a8e923ffb0ae9
Gerrit-Change-Number: 10746
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-7180: Pin Impala CDH dependencies

[Lars Volker (Code Review)]

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

Change subject: IMPALA-7180: Pin Impala CDH dependencies
..


Patch Set 2:

(14 comments)

I left some comments but I'm don't know how to validate the changes in the .pom 
files.

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

http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@9
PS2, Line 9: For profile=3
Without context it's hard to understand what that means. Can you add a sentence 
to introduce the meaning?


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@11
PS2, Line 11: between
extra word


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@10
PS2, Line 10: Maven
: reposito
Maybe reword to "Maven artifacts" or "Maven dependencies"? Or are we really 
storing the repository in S3? If so, how does it get there?


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@16
PS2, Line 16: https://repositories.cloudera.com
Isn't this repository.cloudera.com ?


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@19
PS2, Line 19: global
global to what? Maybe "unique build number"?


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@28
PS2, Line 28: This patch also fixes dependency issue in Hadoop that transitively
"a dependency issue" or "dependency issues"?


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@49
PS2, Line 49: HOST = "https://native-toolchain.s3.amazonaws.com/build";
rename to distinguish from CDH_HOST.


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@361
PS2, Line 361: def download_cdh_components(toolchain_root, cdh_components, 
download_path_prefix):
This should have a better name, e.g. URL prefix. Otherwise it's not clear 
whether it's a local path or a URL. It also should be included in the method's 
comment.


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@395
PS2, Line 395:   the CDH components (i.e. hadoop, hbase, hive, llama, 
llama-minikidc and sentry) into the
Update the comment to include which one gets downloaded from where.


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@414
PS2, Line 414:
The checks below fail, even when DOWNLOAD_CDH_COMPONENTS is false. Should they 
be moved into the block in L440?


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@415
PS2, Line 415:   if not os.environ.get("CDH_HOST"):
if not "CDH_HOST" in os.environ:

Seems slightly more idiomatic


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@420
PS2, Line 420:   if not os.environ.get("CDH_BUILD_NUMBER"):
Both checks have the same effect, make them one check:

  if not ... or not ...:


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@449
PS2, Line 449: cdh_components = map(Package, ["llama-minikdc"])
Simplify to [Package(...)] if it's only one.

Also add a comment why this one is treated differently.


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

http://gerrit.cloudera.org:8080/#/c/10748/2/bin/impala-config.sh@191
PS2, Line 191: : ${CDH_HOST:=native-toolchain.s3.amazonaws.com}
CDH_DEPENDENCY_HOST? CDH_DOWNLOAD_HOST?

Could this conceivably by anything but a CDH host? If so, maybe rename it to 
HADOOP_DEPENDENCY_HOST?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I66c0dcb8abdd0d187490a761f129cda3b3500990
Gerrit-Change-Number: 10748
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 20 Jun 2018 21:34:35 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5202: Disallow PREPARE:WAIT debug action

[Dan Hecht (Code Review)]

Dan Hecht has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10776


Change subject: IMPALA-5202: Disallow PREPARE:WAIT debug action
..

IMPALA-5202: Disallow PREPARE:WAIT debug action

In order to simplify FIS startup, we don't allow cancellation until all
FIS have finished Prepare(), so we shouldn't allow PREPARE:WAIT since
there will be no way to cancel out of the loop.  Make this explicit.

Change-Id: I1caa4f8e6ce7f32a8a3722648e08e24f34dba35d
---
M be/src/exec/exec-node.cc
M be/src/runtime/debug-options.cc
M tests/failure/test_failpoints.py
3 files changed, 15 insertions(+), 6 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1caa4f8e6ce7f32a8a3722648e08e24f34dba35d
Gerrit-Change-Number: 10776
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 

[Impala-ASF-CR] [WIP] Skip remaining exec rpcs if a backend has failed

[Dan Hecht (Code Review)]

Dan Hecht has abandoned this change. ( http://gerrit.cloudera.org:8080/10691 )

Change subject: [WIP] Skip remaining exec rpcs if a backend has failed
..


Abandoned

Oops, didn't mean to publish this.
--
To view, visit http://gerrit.cloudera.org:8080/10691
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Idf80c1dd7c651901bae0ba6ecb555d7a95bcbd10
Gerrit-Change-Number: 10691
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Hecht 

[Impala-ASF-CR] [WIP] Skip remaining exec rpcs if a backend has failed

[Dan Hecht (Code Review)]

Dan Hecht has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/10691 )

Change subject: [WIP] Skip remaining exec rpcs if a backend has failed
..

[WIP] Skip remaining exec rpcs if a backend has failed

Change-Id: Idf80c1dd7c651901bae0ba6ecb555d7a95bcbd10
---
M be/src/common/atomic.h
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/fragment-instance-state.h
6 files changed, 68 insertions(+), 45 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf80c1dd7c651901bae0ba6ecb555d7a95bcbd10
Gerrit-Change-Number: 10691
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Hecht 

[Impala-ASF-CR] IMPALA-5202: Disallow PREPARE:WAIT debug action

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-5202: Disallow PREPARE:WAIT debug action
..


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1caa4f8e6ce7f32a8a3722648e08e24f34dba35d
Gerrit-Change-Number: 10776
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 21:56:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7187: Fix test group impersonation running inside Docker

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7187: Fix test_group_impersonation running inside Docker
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Gerrit-Change-Number: 10772
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:08:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7187: Fix test group impersonation running inside Docker

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10772 )

Change subject: IMPALA-7187: Fix test_group_impersonation running inside Docker
..


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10772/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10772/3//COMMIT_MSG@15
PS3, Line 15: - Ran all test_authorization with test-with-docker.py.
I assume you ran them the "normal" way too and they have the same testing power?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Gerrit-Change-Number: 10772
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:08:36 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7187: Fix test group impersonation running inside Docker

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7187: Fix test_group_impersonation running inside Docker
..


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Gerrit-Change-Number: 10772
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:08:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7187: Fix test group impersonation running inside Docker

[Fredy Wijaya (Code Review)]

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

Change subject: IMPALA-7187: Fix test_group_impersonation running inside Docker
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10772/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10772/3//COMMIT_MSG@15
PS3, Line 15: - Ran all test_authorization with test-with-docker.py.
> I assume you ran them the "normal" way too and they have the same testing p
Yup. I did that, too.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Gerrit-Change-Number: 10772
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:09:55 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..

IMPALA-7151: Rework ephemeral port assignment for be tests

Instead of using FindUnusedEphemeralPort() to choose
many ephemeral ports for in-process servers, we use
Thrift's builtin wildcard ports, where it picks an
ephemeral port when the configured port is 0. This
should be more robust because there's no window where
something else can steal our port. Only Thrift 0.9.2+
support the getPort() method required for this.

This does not necessarily make all ImpalaServer
functionality work with ports configured to 0,
just enough to get expr-test and session-expiry-test
to work. E.g. various strings and names get port 0 in
them and query execution may not work end-to-end.

Some tests still use FindUnusedEphemeralPort() to
pick a single port at a time. We haven't seen that usage pattern
to be flaky with any frequency and it appears to be more work
to fix those, so I left them for now.

Testing:
Ran core tests.

Cherry-picks: not for 2.x

Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Reviewed-on: http://gerrit.cloudera.org:8080/10740
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/benchmarks/expr-benchmark.cc
M be/src/exprs/expr-test.cc
M be/src/rpc/rpc-mgr-kerberized-test.cc
M be/src/rpc/rpc-mgr-test-base.h
M be/src/rpc/rpc-mgr-test.cc
M be/src/rpc/thrift-server-test.cc
M be/src/rpc/thrift-server.cc
M be/src/rpc/thrift-server.h
M be/src/runtime/exec-env.cc
M be/src/runtime/exec-env.h
M be/src/scheduling/scheduler.cc
M be/src/scheduling/scheduler.h
M be/src/service/client-request-state.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-subscriber.cc
M be/src/statestore/statestore-subscriber.h
M be/src/statestore/statestore-test.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
M be/src/util/network-util.cc
M be/src/util/network-util.h
23 files changed, 175 insertions(+), 128 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 

[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5
Gerrit-Change-Number: 10740
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:22:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5202: Disallow PREPARE:WAIT debug action

[Sailesh Mukil (Code Review)]

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

Change subject: IMPALA-5202: Disallow PREPARE:WAIT debug action
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1caa4f8e6ce7f32a8a3722648e08e24f34dba35d
Gerrit-Change-Number: 10776
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:35:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Dan Hecht (Code Review)]

Dan Hecht has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10778


Change subject: IMPALA-7191: don't call srand() at random times
..

IMPALA-7191: don't call srand() at random times

Instead, call it from the daemons' main(), and remove the
calls that happen at random time during the lifetime of the
daemons. Doesn't attempt to cleanup the backend unit tests
which can happen separately or not at all.

Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
---
M be/src/common/init.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/testutil/fault-injection-util.cc
M be/src/util/network-util.cc
6 files changed, 2 insertions(+), 13 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-7191: don't call srand() at random times
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/util/network-util.cc
File be/src/util/network-util.cc:

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/util/network-util.cc@a186
PS1, Line 186:
oops, I meant to revert that since not trying to deal with testing code.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:36:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-7191: don't call srand() at random times
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/util/network-util.cc
File be/src/util/network-util.cc:

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/util/network-util.cc@a186
PS1, Line 186:
> oops, I meant to revert that since not trying to deal with testing code.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:38:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Dan Hecht (Code Review)]

Hello Michael Ho, Tim Armstrong,

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

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

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

Change subject: IMPALA-7191: don't call srand() at random times
..

IMPALA-7191: don't call srand() at random times

Instead, call it from the daemons' main(), and remove the
calls that happen at random time during the lifetime of the
daemons. Doesn't attempt to cleanup the backend unit tests
which can happen separately or not at all.

Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
---
M be/src/common/init.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/runtime/data-stream-sender.cc
M be/src/runtime/krpc-data-stream-sender.cc
M be/src/testutil/fault-injection-util.cc
5 files changed, 2 insertions(+), 11 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7191: don't call srand() at random times
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/common/init.cc
File be/src/common/init.cc:

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/common/init.cc@176
PS1, Line 176:   srand(time(NULL));
Would it be better to use random_device here?

Maybe it doesn't matter that much since this doesn't need to be (and can't be) 
cryptographically secure or anything.


http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/runtime/data-stream-sender.cc
File be/src/runtime/data-stream-sender.cc:

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/runtime/data-stream-sender.cc@a364
PS1, Line 364:
... what?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:39:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7191: don't call srand() at random times
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:39:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-7191: don't call srand() at random times
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/common/init.cc
File be/src/common/init.cc:

http://gerrit.cloudera.org:8080/#/c/10778/1/be/src/common/init.cc@176
PS1, Line 176:   srand(time(NULL));
> Would it be better to use random_device here?
I don't think it matters. If we require more randomness than this then we 
probably shouldn't be using rand() in those places to begin with.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:48:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Lars Volker (Code Review)]

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10690/13/be/src/util/debug-util.cc
File be/src/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/10690/13/be/src/util/debug-util.cc@296
PS13, Line 296:   split(actions, debug_actions, is_any_of("|"), 
token_compress_on);
Apologies for being late. We recently had a couple of ASAN builds fail when 
using split and ended up re-implementing the logic (IMPALA-7111). You might 
want to have a look if you can see what went wrong there, and consider 
re-implementing this, possibly using strings::Split().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 13
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:48:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10690/13/be/src/util/debug-util.cc
File be/src/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/10690/13/be/src/util/debug-util.cc@296
PS13, Line 296:   split(actions, debug_actions, is_any_of("|"), 
token_compress_on);
> Apologies for being late. We recently had a couple of ASAN builds fail when
I have have doubts that split() was the reason problem there, and especially 
that it could be a problem here. So I think this is fine to keep.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 13
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:50:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5202: Disallow PREPARE:WAIT debug action

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5202: Disallow PREPARE:WAIT debug action
..


Patch Set 1:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1caa4f8e6ce7f32a8a3722648e08e24f34dba35d
Gerrit-Change-Number: 10776
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:53:23 +
Gerrit-HasComments: No

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

[Taras Bobrovytsky (Code Review)]

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

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


Patch Set 15:

I already let Anuj know offline that it's ok to remove the precondition.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:55:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Michael Ho (Code Review)]

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

Change subject: IMPALA-7191: don't call srand() at random times
..


Patch Set 1:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10778/2/be/src/runtime/krpc-data-stream-sender.cc@a604
PS2, Line 604:
Not sure if it warrants a comment that the random number generator is seeded 
during initialization of the demon.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:56:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-7191: don't call srand() at random times
..


Patch Set 2:

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10778/2/be/src/runtime/krpc-data-stream-sender.cc@a604
PS2, Line 604:
> Not sure if it warrants a comment that the random number generator is seede
I don't think so but I can add it if you prefer.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:57:04 +
Gerrit-HasComments: Yes

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

[Taras Bobrovytsky (Code Review)]

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

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


Patch Set 15:

Also, I agree with Tim that it's a good idea to add this to the fuzz test. I 
think that it is ok to do that in a separate commit.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f
Gerrit-Change-Number: 6023
Gerrit-PatchSet: 15
Gerrit-Owner: anujphadke 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Taras Bobrovytsky 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: anujphadke 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:57:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7191: don't call srand() at random times

[Michael Ho (Code Review)]

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

Change subject: IMPALA-7191: don't call srand() at random times
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf0ef5a0e842ebdcdb6d7355302e68fb0bc7ef5f
Gerrit-Change-Number: 10778
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 22:58:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7046: introduce "global" debug actions

[Tim Armstrong (Code Review)]

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

Change subject: IMPALA-7046: introduce "global" debug_actions
..


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10690/13/be/src/util/debug-util.cc
File be/src/util/debug-util.cc:

http://gerrit.cloudera.org:8080/#/c/10690/13/be/src/util/debug-util.cc@296
PS13, Line 296:   split(actions, debug_actions, is_any_of("|"), 
token_compress_on);
> I have have doubts that split() was the reason problem there, and especiall
Yeah, my theory for why split() was a problem doesn't apply here since 
debug_actions is a const&. My theory was that somehow in the template expansion 
it dispatched to some code that assumed that modified the string because it was 
a non-constant ref.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77663a539be18711a4f12c470ffd7474e3d69388
Gerrit-Change-Number: 10690
Gerrit-PatchSet: 13
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Bikramjeet Vig 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 20 Jun 2018 23:00:50 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Added another scenario when REFRESH is required

[Alex Rodoni (Code Review)]

Hello Impala Public Jenkins, Vuk Ercegovac,

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

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

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

Change subject: [DOCS] Added another scenario when REFRESH is required
..

[DOCS] Added another scenario when REFRESH is required

Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
---
M docs/topics/impala_refresh.xml
1 file changed, 4 insertions(+), 7 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
Gerrit-Change-Number: 10773
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] [DOCS] Added another scenario when REFRESH is required

[Alex Rodoni (Code Review)]

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

Change subject: [DOCS] Added another scenario when REFRESH is required
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10773/1/docs/topics/impala_refresh.xml
File docs/topics/impala_refresh.xml:

http://gerrit.cloudera.org:8080/#/c/10773/1/docs/topics/impala_refresh.xml@90
PS1, Line 90: appending, or altering
> why not say "modifying" or "altering"? Is there a distinction between appen
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
Gerrit-Change-Number: 10773
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 23:11:01 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6119: Fix issue with multiple partitions sharing same location

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10543 )

Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same 
location
..


Patch Set 17:

(9 comments)

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

http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@187
PS17, Line 187: The second parameter is a set of
  :   // the partitions pointing to this location.
we're paying memory for speed. what operations will be slowed down to address 
this "synonym" capability if this map is not used?


http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@189
PS17, Line 189: locationToPartMap_
can you quantify the additional memory needed per partition? perhaps measure 
the difference for a table with a large number of partitions (~100k).


http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1235
PS17, Line 1235: PartMap
use consistent naming. so far, there's: LocationMapping and PartMap


http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1492
PS17, Line 1492: received
what does "received" mean?

"grouped by their base dir..." implies same location, so I don't think the 
change here improves the comment.


http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1494
PS17, Line 1494:   private HashMap> 
getPartitionsByPath(
Is the modification to this method the one that fixes the issue with refreshes 
not applying to all synonyms of a location?
If so, I see that its used right above on L1484. That method includes a loop 
over all partitions, so it seems that if we make a small map given the 
partitions to update, we can collect those additional partitions that share the 
same location as the ones that we're updating. That would seem to be a small, 
additional cost in both time and memory for this operation.


http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2255
PS17, Line 2255: (HdfsTable)
lift the cast. perhaps put it right after the precondition on L2239


http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2258
PS17, Line 2258:   // Display only a subset of the partition names not 
to flood the logs in
at this point, size of partitionNameList must be > 1. perhaps add a 
precondition for this?


http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2262
PS17, Line 2262: ImpalaRuntimeException
is there a solution in mind? for example, should we suggest that duplicate 
references be updated to unique locations (do the locations need to be valid?), 
then the operation can proceed?


http://gerrit.cloudera.org:8080/#/c/10543/17/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2464
PS17, Line 2464: HdfsTable
make this a precondition for this method?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a
Gerrit-Change-Number: 10543
Gerrit-PatchSet: 17
Gerrit-Owner: Gabor Kaszab 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Gabor Kaszab 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Reviewer: Zoltan Borok-Nagy 
Gerrit-Comment-Date: Wed, 20 Jun 2018 23:43:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [DOCS] Added another scenario when REFRESH is required

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10773 )

Change subject: [DOCS] Added another scenario when REFRESH is required
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
Gerrit-Change-Number: 10773
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 20 Jun 2018 23:46:07 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-110 (part 2): Refactor PartitionedAggregationNode

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/aggregator.h
File be/src/exec/aggregator.h:

http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/aggregator.h@54
PS3, Line 54: AggregationNode
Used for other exec nodes, no? (e.g. streaming)


http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/aggregator.h@63
PS3, Line 63:   virtual Status Init(const TPlanNode& tnode, RuntimeState* 
state) WARN_UNUSED_RESULT;
:   virtual Status Prepare(RuntimeState* state) WARN_UNUSED_RESULT;
:   virtual void Codegen(RuntimeState* state) = 0;
:   virtual Status Open(RuntimeState* state) WARN_UNUSED_RESULT;
:   virtual Status GetNext(
:   RuntimeState* state, RowBatch* row_batch, bool* eos) 
WARN_UNUSED_RESULT = 0;
:   virtual Status Reset(RuntimeState* state) WARN_UNUSED_RESULT = 
0;
:   virtual void Close(RuntimeState* state);
I guess these are used one-to-one to implement the ExecNode interface? Or are 
there any subtle differences in lifecycle? let's briefly comment either way.


http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/aggregator.h@80
PS3, Line 80: SetDebugOptions
why does the aggregator need to copy the debug_options? where does it get used 
at the aggregator level?


http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/streaming-aggregation-node.h
File be/src/exec/streaming-aggregation-node.h:

http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/streaming-aggregation-node.h@31
PS3, Line 31: Node for doing partitioned hash aggregation.
: /// This node consumes the input from child(0) during GetNext() 
and then passes it to the
: /// Aggregator, which does the actual work of aggregating.
: /// This node only supports grouping aggregations.
somewhere in here we should explain how this differs from AggregationNode, i.e. 
what does streaming mean.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e
Gerrit-Change-Number: 10394
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Marshall 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 21 Jun 2018 00:21:21 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7187: Fix test group impersonation running inside Docker

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7187: Fix test_group_impersonation running inside Docker
..

IMPALA-7187: Fix test_group_impersonation running inside Docker

test_group_impersonation fails when running with test-with-docker.py
because os.getgroups() returns empty in Docker for the authorized
proxy groups which results in denying all access. The patch is to use
a group name from a current GID.

Testing:
- Ran all test_authorization with test-with-docker.py.

Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Reviewed-on: http://gerrit.cloudera.org:8080/10772
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M tests/authorization/test_authorization.py
1 file changed, 1 insertion(+), 4 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Gerrit-Change-Number: 10772
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-7187: Fix test group impersonation running inside Docker

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-7187: Fix test_group_impersonation running inside Docker
..


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb6089273fd9bc3763cb45f0591610b04a6b232e
Gerrit-Change-Number: 10772
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 21 Jun 2018 01:30:39 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Added another scenario when REFRESH is required

[Impala Public Jenkins (Code Review)]

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

Change subject: [DOCS] Added another scenario when REFRESH is required
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
Gerrit-Change-Number: 10773
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 21 Jun 2018 01:31:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] [DOCS] Added another scenario when REFRESH is required

[Impala Public Jenkins (Code Review)]

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

Change subject: [DOCS] Added another scenario when REFRESH is required
..

[DOCS] Added another scenario when REFRESH is required

Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
Reviewed-on: http://gerrit.cloudera.org:8080/10773
Reviewed-by: Vuk Ercegovac 
Tested-by: Impala Public Jenkins 
---
M docs/topics/impala_refresh.xml
1 file changed, 4 insertions(+), 7 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
Gerrit-Change-Number: 10773
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] [DOCS] Added another scenario when REFRESH is required

[Impala Public Jenkins (Code Review)]

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

Change subject: [DOCS] Added another scenario when REFRESH is required
..


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifff8f6b6f834402a158312d57076a84ad1eeab45
Gerrit-Change-Number: 10773
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 21 Jun 2018 01:36:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py

[Taras Bobrovytsky (Code Review)]

Taras Bobrovytsky has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10780


Change subject: IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py
..

IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py

We add some timestamps to the output of start-impala-cluster.py in
order to make it easier to debug failures in custom cluster tests that
happen when starting the cluster.

Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
---
M bin/start-impala-cluster.py
M tests/common/impala_service.py
2 files changed, 22 insertions(+), 15 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
Gerrit-Change-Number: 10780
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 

[Impala-ASF-CR] IMPALA-7175: deflake check for failed impalad

[Vuk Ercegovac (Code Review)]

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10745 )

Change subject: IMPALA-7175: deflake check for failed impalad
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10745/1/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/10745/1/tests/query_test/test_udfs.py@301
PS1, Line 301:   def __num_impalads(self):
> Other tests may also run to the same problem, so I think that it would be u
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I97c6b398e43c6abb1df2b1783c26159137f14fa4
Gerrit-Change-Number: 10745
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 21 Jun 2018 02:07:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7175: deflake check for failed impalad

[Vuk Ercegovac (Code Review)]

Hello Csaba Ringhofer,

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

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

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

Change subject: IMPALA-7175: deflake check for failed impalad
..

IMPALA-7175: deflake check for failed impalad

test_native_functions_race checks for an impalad crash
by testing whether the number of impalads at the start and
end of the test is the same. A recent run was flaky since
the number of impalads at the start was incorrectly found
to be 2. This fix tries to make the test most robust by
determining the number of impalads based on how many
can evaluate a trivial test query. For these tests, its
assumed that the number of coordinators is the same as
the number of impalads in the cluster.

Change-Id: I97c6b398e43c6abb1df2b1783c26159137f14fa4
---
M tests/common/impala_cluster.py
M tests/query_test/test_udfs.py
2 files changed, 17 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I97c6b398e43c6abb1df2b1783c26159137f14fa4
Gerrit-Change-Number: 10745
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Vuk Ercegovac 

[Impala-ASF-CR] IMPALA-5202: Disallow PREPARE:WAIT debug action

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5202: Disallow PREPARE:WAIT debug action
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1caa4f8e6ce7f32a8a3722648e08e24f34dba35d
Gerrit-Change-Number: 10776
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Jun 2018 02:20:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7180: Pin Impala CDH dependencies

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/10748 )

Change subject: IMPALA-7180: Pin Impala CDH dependencies
..

IMPALA-7180: Pin Impala CDH dependencies

For IMPALA_MINICLUSTER_PROFILE=3 (Hadoop 3.x components)
Pin the CDH dependencies by storing the CDH tarballs and Maven
repository in S3. This solves the issue of build coherency between
the the CDH tarballs and Maven dependencies.

For IMPALA_MINICLUSTER_PROFILE=2 (Hadoop 2.x components)
Pin the CDH dependencies by storing only the CDH tarballs in S3.
The Maven repository will still use https://repository.cloudera.com.
So there is still a possibility of a build coherency issue.

For each CDH dependency, there is a unique build number in each repository
URL to indicate the build number that created those CDH dependencies.
This informaton can be useful for debugging issues related to CDH
dependencies.

This patch introduces CDH_DOWNLOAD_HOST and CDH_BUILD_NUMBER environment
variables that can be overriden, which can be useful for running an
integration job.

This patch also fixes dependency issues in Hadoop that transitively
depends on snapshot versions of dependencies that no longer exist.
See HADOOP-14903.

Testing:
- Ran all core tests on IMPALA_MINICLUSTER_PROFILE=2 and
  IMPALA_MINICLUSTER_PROFILE=3

Cherry-picks: not for 2.x

Change-Id: I66c0dcb8abdd0d187490a761f129cda3b3500990
---
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
M common/yarn-extras/pom.xml
M fe/pom.xml
M impala-parent/pom.xml
M testdata/pom.xml
M tests/test-hive-udfs/pom.xml
7 files changed, 295 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I66c0dcb8abdd0d187490a761f129cda3b3500990
Gerrit-Change-Number: 10748
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-7180: Pin Impala CDH dependencies

[Fredy Wijaya (Code Review)]

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

Change subject: IMPALA-7180: Pin Impala CDH dependencies
..


Patch Set 5:

(14 comments)

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

http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@9
PS2, Line 9: For IMPALA_MI
> Without context it's hard to understand what that means. Can you add a sent
It's an environment variable used in Impala to switch between Hadoop 2.x and 
3.x components. I'll change it to IMPALA_MINICLUSTER_PROFILE instead.


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@10
PS2, Line 10: Maven
: reposito
> Maybe reword to "Maven artifacts" or "Maven dependencies"? Or are we really
Yes, we are storing the whole Maven repository.  Cloudera has access to the S3 
native-toolchain and they will upload the CDH dependencies when we need it.


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@11
PS2, Line 11: between
> extra word
Done


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@16
PS2, Line 16: https://repository.cloudera.com.
> Isn't this repository.cloudera.com ?
Typo. Done.


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@19
PS2, Line 19: unique
> global to what? Maybe "unique build number"?
Done


http://gerrit.cloudera.org:8080/#/c/10748/2//COMMIT_MSG@28
PS2, Line 28: This patch also fixes dependency issues in Hadoop that 
transitively
> "a dependency issue" or "dependency issues"?
Should be issues. Done.


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@49
PS2, Line 49: TOOLCHAIN_HOST = "https://native-toolchain.s3.amazonaws.com/build";
> rename to distinguish from CDH_HOST.
Done


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@361
PS2, Line 361: def download_cdh_components(toolchain_root, cdh_components, 
url_prefix):
> This should have a better name, e.g. URL prefix. Otherwise it's not clear w
Done


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@395
PS2, Line 395:   $IMPALA_TOOLCHAIN. If $DOWNLOAD_CDH_COMPONENTS is true, the 
presence of
> Update the comment to include which one gets downloaded from where.
Done


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@414
PS2, Line 414:
> The checks below fail, even when DOWNLOAD_CDH_COMPONENTS is false. Should t
Make sense. Done.


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@415
PS2, Line 415:   # Create the destination directory if necessary
> if not "CDH_HOST" in os.environ:
Done


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@420
PS2, Line 420: sys.exit(1)
> Both checks have the same effect, make them one check:
Done


http://gerrit.cloudera.org:8080/#/c/10748/2/bin/bootstrap_toolchain.py@449
PS2, Line 449: download_cdh_components(toolchain_root, cdh_components, 
download_path_prefix)
> Simplify to [Package(...)] if it's only one.
Done


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

http://gerrit.cloudera.org:8080/#/c/10748/2/bin/impala-config.sh@191
PS2, Line 191: : ${CDH_DOWNLOAD_HOST:=native-toolchain.s3.amazonaws.com}
> CDH_DEPENDENCY_HOST? CDH_DOWNLOAD_HOST?
CDH_DOWNLOAD_HOST makes more sense. The host should only contain anything 
related to CDH.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I66c0dcb8abdd0d187490a761f129cda3b3500990
Gerrit-Change-Number: 10748
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Comment-Date: Thu, 21 Jun 2018 02:30:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7180: Pin Impala CDH dependencies

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10748 )

Change subject: IMPALA-7180: Pin Impala CDH dependencies
..


Patch Set 5: Code-Review+1

(7 comments)

I think this is fine. I have some nits, but nothing earth-shattering.

When the CDH_BUILD_NUMBER variable changes upstream, will people need to 
re-build their miniclusters? I don't know if we're going to like have the build 
numbers in impala-config.sh. On one hand, it's reproducible, but on the other, 
it may have annoying noisy commits and so on.

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

http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@15
PS5, Line 15: Pin the CDH dependencies by storing only the CDH tarballs in S3.
nit: lowercase pin


http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@16
PS5, Line 16: The Maven repository will still use 
https://repository.cloudera.com.
nit:", so" across this and the next line.


http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@28
PS5, Line 28: This patch also fixes dependency issues in Hadoop that 
transitively
Let's be explicit about json-smart in here. Also reminds me of IMPALA-7120.

If this is a bug in the CDH version of Hadoop we're picking up, can you see if 
it's easily fixable or fileable?


http://gerrit.cloudera.org:8080/#/c/10748/5/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

http://gerrit.cloudera.org:8080/#/c/10748/5/bin/bootstrap_toolchain.py@403
PS5, Line 403:   into the directory specified by $CDH_COMPONENTS_HOME.
nit: move line 403 into line 397 "following CDH components into the 
directory:" so that it's clearer?


http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml
File impala-parent/pom.xml:

http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml@148
PS5, Line 148:   cdh.snapshots.repo
We should probably rename this id to something else. Perhaps "impala.cdh.repo". 
Sometimes these names show up in errors and logs, and it's useful to be able to 
know which one it is.


http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml@150
PS5, Line 150:   CDH Snapshots Repository
rename this too


http://gerrit.cloudera.org:8080/#/c/10748/5/testdata/pom.xml
File testdata/pom.xml:

http://gerrit.cloudera.org:8080/#/c/10748/5/testdata/pom.xml@39
PS5, Line 39: 

[Impala-ASF-CR] IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10780 )

Change subject: IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py
..


Patch Set 1:

This seems fine, but using Python logging would make this more straightforward.

>>> if True:
...   import logging
...   logging.basicConfig(level=logging.INFO,
...   format='%(asctime)s %(threadName)s %(levelname)s: %(message)s')
...   logging.info("hey")
...
2018-06-20 20:35:43,799 MainThread INFO: hey
>>>


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
Gerrit-Change-Number: 10780
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Jun 2018 03:36:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py

[Philip Zeyliger (Code Review)]

Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10780 )

Change subject: IMPALA-6642 (Part 2): Add timestamps to start-impala-cluster.py
..


Patch Set 1: Code-Review+1

(1 comment)

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

http://gerrit.cloudera.org:8080/#/c/10780/1/bin/start-impala-cluster.py@460
PS1, Line 460: print 'Error starting cluster: {0}'.format(e)
If you switch to logging, this becomes just "logging.exception("Error starting 
cluster")". Python does the stack trace printing magically.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I60169203c61ae6bc0a3ccd3dea355799b603efe5
Gerrit-Change-Number: 10780
Gerrit-PatchSet: 1
Gerrit-Owner: Taras Bobrovytsky 
Gerrit-Reviewer: Philip Zeyliger 
Gerrit-Reviewer: Tianyi Wang 
Gerrit-Comment-Date: Thu, 21 Jun 2018 03:36:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5202: Disallow PREPARE:WAIT debug action

[Impala Public Jenkins (Code Review)]

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

Change subject: IMPALA-5202: Disallow PREPARE:WAIT debug action
..


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1caa4f8e6ce7f32a8a3722648e08e24f34dba35d
Gerrit-Change-Number: 10776
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Jun 2018 04:03:31 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5202: Disallow PREPARE:WAIT debug action

[Dan Hecht (Code Review)]

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

Change subject: IMPALA-5202: Disallow PREPARE:WAIT debug action
..


Patch Set 2: Code-Review+2

Rebased onto master


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1caa4f8e6ce7f32a8a3722648e08e24f34dba35d
Gerrit-Change-Number: 10776
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht 
Gerrit-Reviewer: Dan Hecht 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 21 Jun 2018 04:03:20 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7180: Pin Impala CDH dependencies

[Fredy Wijaya (Code Review)]

Fredy Wijaya has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/10748 )

Change subject: IMPALA-7180: Pin Impala CDH dependencies
..

IMPALA-7180: Pin Impala CDH dependencies

For IMPALA_MINICLUSTER_PROFILE=3 (Hadoop 3.x components), pin the
CDH dependencies by storing the CDH tarballs and Maven repository
in S3. This solves the issue of build coherency between the the CDH
tarballs and Maven dependencies.

For IMPALA_MINICLUSTER_PROFILE=2 (Hadoop 2.x components), pin the
CDH dependencies by storing only the CDH tarballs in S3. The Maven
repository will still use https://repository.cloudera.com, so there
is still a possibility of a build coherency issue.

For each CDH dependency, there is a unique build number in each repository
URL to indicate the build number that created those CDH dependencies.
This informaton can be useful for debugging issues related to CDH
dependencies.

This patch introduces CDH_DOWNLOAD_HOST and CDH_BUILD_NUMBER environment
variables that can be overriden, which can be useful for running an
integration job.

This patch also fixes dependency issues in Hadoop that transitively
depend on snapshot versions of dependencies that no longer exist, i.e.
- net.minidev:json-smart:2.3-SNAPSHOT (HADOOP-14903)
- org.glassfish:javax.el:3.0.1-b06-SNAPSHOT
The fix is to force the dependencies by using the released versions of
those dependencies.

Testing:
- Ran all core tests on IMPALA_MINICLUSTER_PROFILE=2 and
  IMPALA_MINICLUSTER_PROFILE=3

Cherry-picks: not for 2.x

Change-Id: I66c0dcb8abdd0d187490a761f129cda3b3500990
---
M bin/bootstrap_toolchain.py
M bin/impala-config.sh
M common/yarn-extras/pom.xml
M fe/pom.xml
M impala-parent/pom.xml
M testdata/pom.xml
M tests/test-hive-udfs/pom.xml
7 files changed, 295 insertions(+), 32 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I66c0dcb8abdd0d187490a761f129cda3b3500990
Gerrit-Change-Number: 10748
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Philip Zeyliger 

[Impala-ASF-CR] IMPALA-7180: Pin Impala CDH dependencies

[Fredy Wijaya (Code Review)]

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

Change subject: IMPALA-7180: Pin Impala CDH dependencies
..


Patch Set 7: Code-Review+1

(7 comments)

> Patch Set 5: Code-Review+1
>
> (7 comments)
>
> I think this is fine. I have some nits, but nothing earth-shattering.
>
> When the CDH_BUILD_NUMBER variable changes upstream, will people need to 
> re-build their miniclusters? I don't know if we're going to like have the 
> build numbers in impala-config.sh. On one hand, it's reproducible, but on the 
> other, it may have annoying noisy commits and so on.

They may need to run a build but reformatting the test data is not necessary 
(although recommended to make sure the clients and servers are in coherent) 
unless there is something major in the CDH components that require re-creating 
the test data. It should work similarly to the way it works right now.

Carrying Phil's +1

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

http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@15
PS5, Line 15: CDH dependencies by storing only the CDH tarballs in S3. The Maven
> nit: lowercase pin
Done


http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@16
PS5, Line 16: repository will still use https://repository.cloudera.com, so 
there
> nit:", so" across this and the next line.
Done


http://gerrit.cloudera.org:8080/#/c/10748/5//COMMIT_MSG@28
PS5, Line 28: This patch also fixes dependency issues in Hadoop that 
transitively
> Let's be explicit about json-smart in here. Also reminds me of IMPALA-7120.
Done


http://gerrit.cloudera.org:8080/#/c/10748/5/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

http://gerrit.cloudera.org:8080/#/c/10748/5/bin/bootstrap_toolchain.py@403
PS5, Line 403:   - llama-minikdc (downloaded from $TOOLCHAIN_HOST)
> nit: move line 403 into line 397 "following CDH components into the directo
Done


http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml
File impala-parent/pom.xml:

http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml@148
PS5, Line 148:   impala.cdh.repo
> We should probably rename this id to something else. Perhaps "impala.cdh.re
Done


http://gerrit.cloudera.org:8080/#/c/10748/5/impala-parent/pom.xml@150
PS5, Line 150:   Impala CDH Repository
> rename this too
Done


http://gerrit.cloudera.org:8080/#/c/10748/5/testdata/pom.xml
File testdata/pom.xml:

http://gerrit.cloudera.org:8080/#/c/10748/5/testdata/pom.xml@39
PS5, Line 39: 

[Impala-ASF-CR] [DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls

[Bharath Vissapragada (Code Review)]

Bharath Vissapragada has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10696


Change subject: [DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls
..

[DRAFT] IMPALA-6189: Add thread watchdogs for HDFS IO calls

This commit adds scoped thread watchdogs around blocking HDFS
IO calls. These dump a user (native + JVM) and kernel stack trace
to the logs if the blocking function call runs for greater than
'threshold' milliseconds. Having such traces will help debug issues
with query hangs due to hangs in external components.

The threshold can be configured using --thread_watchdog_threshold_ms

If stack unwinding tends to be CPU heavy and causing performance
problems, the watchdogs can be disabled by setting

--thread_watchdog_threshold_ms=0 (default = 5000ms)

Usage:
-
This patch adds a macro called SCOPED_WATCHDOG(). Following
is a sample usage:

{
  SCOPED_WATCHDOG();
  hdfsOpenFile(fs, fname, O_RDONLY, 0, 0, 0)
}

The above code logs kernel and user level stacks for the current thread
if hdfsOpenFile() takes more than 5000 milliseconds.

Sample output:
-

Native Stacks:
==

Injected a manual sleep using SleepForMs() before hdfsOpenFile() to
simulate a hang and the following is logged in the impalad logs.

W0611 16:21:01.602687   347 kernel_stack_watchdog.cc:146] Thread 383
stuck at /home/bharath/Impala/be/src/runtime/io/handle-cache.inline.h:34
for 5036ms:
Kernel stack:
[<>] hrtimer_nanosleep+0xbb/0x180
[<>] SyS_nanosleep+0x66/0x80
[<>] system_call_fastpath+0x1a/0x1f
[<>] 0x

User stack:
@ 0x7f808e7fed40  (unknown)
@ 0x7f808eb9db9d  __libc_nanosleep
@  0x1f460e6  std::this_thread::sleep_for<>()
@  0x1f44edd  impala::SleepForMs()
@  0x2fef2ca  impala::io::HdfsFileHandle::HdfsFileHandle()
@  0x2fef5f7  
impala::io::CachedHdfsFileHandle::CachedHdfsFileHandle()
@  0x2fefe78  impala::io::FileHandleCache::GetFileHandle()
@  0x2ff3e3f  impala::io::DiskIoMgr::GetCachedHdfsFileHandle()
@  0x300fad7  impala::io::ScanRange::Read()
@  0x300bca4  impala::io::ScanRange::DoRead()
@  0x2ff3207  impala::io::DiskQueue::DiskThreadLoop()
@  0x2ffac32  boost::_mfi::mf1<>::operator()()
@  0x2ffa79f  boost::_bi::list2<>::operator()<>()
@  0x2ff9fbd  boost::_bi::bind_t<>::operator()()
@  0x2ff970a  
boost::detail::function::void_function_obj_invoker0<>::invoke()
@  0x1b4e388  boost::function0<>::operator()()

JVM Stack:
Thread not attached to the JVM.

JVM Stacks:
===

If a JVM Thread is attached for a given hung native thread, the the
stack trace output looks like follows.

W0618 20:55:45.93  1220 kernel_stack_watchdog.cc:147] Thread 1381
stuck at be/src/service/impala-server.cc:882 for 5083ms:
Kernel stack:
[<>] futex_wait_queue_me+0xde/0x140
[<>] futex_wait+0x182/0x290
[<>] do_futex+0xde/0x6a0
[<>] SyS_futex+0x71/0x150
[<>] system_call_fastpath+0x1a/0x1f
[<>] 0x

User stack:
@ 0x7fee18315d40  (unknown)
@ 0x7fee186b17ce  __pthread_cond_timedwait
@ 0x7fee1b1d6a9f  os::PlatformEvent::park()
@ 0x7fee1b1c2c3f  ObjectMonitor::wait()
@ 0x7fee1afd5ed2  JVM_MonitorWait
@ 0x7fee03cb6ca4  (unknown)

JVM Stack:
Ljava/lang/Object;wait()
Lorg/apache/impala/catalog/ImpaladCatalog;waitForCatalogUpdate(Line:239)
Lorg/apache/impala/analysis/StmtMetadataLoader;loadTables(Line:136)
Lorg/apache/impala/analysis/StmtMetadataLoader;loadTables(Line:115)
Lorg/apache/impala/service/Frontend;createExecRequest(Line:996)
Lorg/apache/impala/service/JniFrontend;createExecRequest(Line:152)

Change-Id: I28e918761c120043d332b034450208eaf34e3e2b
---
M be/src/common/global-flags.cc
M be/src/exec/hdfs-table-sink.cc
M be/src/kudu/util/debug-util.cc
M be/src/kudu/util/kernel_stack_watchdog.cc
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/scan-range.cc
M be/src/service/CMakeLists.txt
A be/src/service/jvmti-agent.cc
A be/src/service/jvmti-agent.h
M be/src/util/CMakeLists.txt
A be/src/util/jvm-stack-trace.cc
A be/src/util/jvm-stack-trace.h
A be/src/util/scoped-watchdog.h
M bin/impala-config.sh
14 files changed, 498 insertions(+), 27 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I28e918761c120043d332b034450208eaf34e3e2b
Gerrit-Change-Number: 10696
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada 
Gerrit-Reviewer: Ba