You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2020/03/14 21:50:08 UTC
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15438
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
[subprocess] copy subprocess JAR to be next to the master binary
This patch copies the subprocess JAR to be next to the master/tserver
binary. So that it can be found even with the default value of gflag
as 'ranger_jar_path'. The same for the hms JAR.
Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
---
M build-support/run_dist_test.py
M src/kudu/hms/CMakeLists.txt
M src/kudu/subprocess/CMakeLists.txt
3 files changed, 4 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15438/1
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: So that it can be found even with the default value of gflag
: as 'ranger_jar_path'. The same for the hms JAR.
> I suggested we change from a link to a copy. I thought this would be useful
Yeah, that's a nice property of having this patch. OTOH one of the nice things about linking is that in iterating and updating the Java subprocess, I've found it handy that I don't have to rerun any make commands if I want to run some C++ subprocess tests with the new Java binary. That said, I don't feel strongly about it.
If we do want to decouple the builds, would it make sense to have gradle output directly to the bin dir instead of copying? That seemed to be the original behavior of the HMS plugin since it used add_jar().
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 18:28:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15438
to look at the new patch set (#2).
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
[subprocess] copy subprocess JAR to be next to the master binary
This patch copies the subprocess JAR to be next to the master/tserver
binary. So that it can be found even with the default value of gflag
as 'ranger_jar_path'. The same for the hms JAR.
Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
---
M build-support/dist_test.py
M build-support/run_dist_test.py
M src/kudu/hms/CMakeLists.txt
M src/kudu/subprocess/CMakeLists.txt
4 files changed, 4 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15438/2
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: So that it can be found even with the default value of gflag
: as 'ranger_jar_path'. The same for the hms JAR.
> OK, this explanation makes sense to me. Hao, could you summarize it and add
Ack. For updating the gradle output directly to the bin dir, I keep the current behavior, as I don't see a strong reason to change it (which probably would require some custom changes in subprocess gradle build). Or do you have a strong opinion against it?
http://gerrit.cloudera.org:8080/#/c/15438/3/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/15438/3/src/kudu/hms/CMakeLists.txt@70
PS3, Line 70: COMMAND cp
> Should probably use 'cp -f' here. Same for the subprocess JAR.
Done
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 23:32:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: nstead of linking. It can reduce the coupling from the Java
: build and the C++ build, since the Java build c
> Ack. For updating the gradle output directly to the bin dir, I keep the cur
It's a similar point about decoupling: if we build C++, doesn't it write over whatever JAR exists in the Java build directory? Not sure that makes a difference with running tests though in the same way that linking affects the C++ tests.
I don't feel strongly about it.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 23:54:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 17 Mar 2020 00:54:10 +0000
Gerrit-HasComments: No
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: So that it can be found even with the default value of gflag
: as 'ranger_jar_path'. The same for the hms JAR.
> But we only use the JAR link once -- when we initialize the master -- so it
+1 to Andrew's comments. AFAICT this is only for testing. In production, the JAR's link would have been followed and the real file copied into a tarball (or system package, or whatever). So what's the concrete testing concern?
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 06:57:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: So that it can be found even with the default value of gflag
: as 'ranger_jar_path'. The same for the hms JAR.
> I think I'm missing something; why aren't the links sufficient? The default
I think the concern is the link can be broken when the master is running. The default value is changed in the previous related patch.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 15 Mar 2020 04:17:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 4:
Unrelated flaky tests.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 17 Mar 2020 00:54:23 +0000
Gerrit-HasComments: No
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15438
to look at the new patch set (#4).
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
[subprocess] copy subprocess JAR to be next to the master binary
This patch copies the subprocess JAR to be next to the master/tserver
binary instead of linking. It can reduce the coupling from the Java
build and the C++ build, since the Java build could remove the linked
jar if running C++ build at the same time. The same for the hms JAR as
we have similar expectation of the location of the JAR.
Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
---
M build-support/run_dist_test.py
M src/kudu/hms/CMakeLists.txt
M src/kudu/subprocess/CMakeLists.txt
3 files changed, 4 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15438/4
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: nstead of linking. It can reduce the coupling from the Java
: build and the C++ build, since the Java build c
> Ack
Chatted with Hao on Slack and she mentioned that a concurrent gradle build shouldn't affect any ongoing Java tests.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 17 Mar 2020 01:16:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 2:
> Patch Set 1:
>
> I think we can remove this too : https://github.com/apache/kudu/blob/master/build-support/dist_test.py#L113-L117
Hmm, it seems we have to keep this for the test to know where to find the jars.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 15 Mar 2020 01:35:02 +0000
Gerrit-HasComments: No
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
[subprocess] copy subprocess JAR to be next to the master binary
This patch copies the subprocess JAR to be next to the master/tserver
binary instead of linking. It can reduce the coupling from the Java
build and the C++ build, since the Java build could remove the linked
jar if running C++ build at the same time. The same for the hms JAR as
we have similar expectation of the location of the JAR.
Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Reviewed-on: http://gerrit.cloudera.org:8080/15438
Tested-by: Hao Hao <ha...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M build-support/run_dist_test.py
M src/kudu/hms/CMakeLists.txt
M src/kudu/subprocess/CMakeLists.txt
3 files changed, 4 insertions(+), 12 deletions(-)
Approvals:
Hao Hao: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: So that it can be found even with the default value of gflag
: as 'ranger_jar_path'. The same for the hms JAR.
> I suggested we change from a link to a copy. I thought this would be useful
OK, this explanation makes sense to me. Hao, could you summarize it and add it to the commit message?
http://gerrit.cloudera.org:8080/#/c/15438/3/src/kudu/hms/CMakeLists.txt
File src/kudu/hms/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/15438/3/src/kudu/hms/CMakeLists.txt@70
PS3, Line 70: COMMAND cp
Should probably use 'cp -f' here. Same for the subprocess JAR.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 18:30:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: So that it can be found even with the default value of gflag
: as 'ranger_jar_path'. The same for the hms JAR.
> +1 to Andrew's comments. AFAICT this is only for testing. In production, th
I suggested we change from a link to a copy. I thought this would be useful to reduce the coupling from the Java build and the C++ build. If I am running C++ tests and doing Java work at the same time, the Java build could remove the linked jar.
I also thought this would be better for production scenarios given we only deliver a source release and default with the expectation that the jar is next to the binary. We actually used to use a copy for the hms-plugin.jar and our documentation describes finding the jar next to the binaries. I assume we would do the same for this jar. https://github.com/apache/kudu/blame/master/docs/hive_metastore.adoc#L155
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 16 Mar 2020 13:19:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 1: Verified+1
Unrelated flaky failure.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 14 Mar 2020 22:36:37 +0000
Gerrit-HasComments: No
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 4:
(1 comment)
> Patch Set 4: Code-Review+1
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: nstead of linking. It can reduce the coupling from the Java
: build and the C++ build, since the Java build c
> It's a similar point about decoupling: if we build C++, doesn't it write ov
Ack
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 17 Mar 2020 00:56:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: So that it can be found even with the default value of gflag
: as 'ranger_jar_path'. The same for the hms JAR.
> I think the concern is the link can be broken when the master is running.
But we only use the JAR link once -- when we initialize the master -- so it shouldn't matter if the link gets broken while the master is running, right?
My point about the default changing is that this doesn't seem to "fix" some broken behavior, so I'm trying to better understand the intent behind this patch.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 15 Mar 2020 07:09:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: nstead of linking. It can reduce the coupling from the Java
: build and the C++ build, since the Java build c
> Chatted with Hao on Slack and she mentioned that a concurrent gradle build
Yeah, specifically I tried to run ./gradlew :kudu-subprocess:test and ./gradlew :kudu-subprocess:jar the same time without hitting any issues.
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 17 Mar 2020 01:27:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15438/3//COMMIT_MSG@10
PS3, Line 10: So that it can be found even with the default value of gflag
: as 'ranger_jar_path'. The same for the hms JAR.
I think I'm missing something; why aren't the links sufficient? The defaults values aren't changed in this patch, so why was this necessary?
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 15 Mar 2020 02:04:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15438 )
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
Patch Set 1:
I think we can remove this too : https://github.com/apache/kudu/blob/master/build-support/dist_test.py#L113-L117
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 14 Mar 2020 22:40:25 +0000
Gerrit-HasComments: No
[kudu-CR] [subprocess] copy subprocess JAR to be next to the master binary
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15438
to look at the new patch set (#3).
Change subject: [subprocess] copy subprocess JAR to be next to the master binary
......................................................................
[subprocess] copy subprocess JAR to be next to the master binary
This patch copies the subprocess JAR to be next to the master/tserver
binary. So that it can be found even with the default value of gflag
as 'ranger_jar_path'. The same for the hms JAR.
Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
---
M build-support/run_dist_test.py
M src/kudu/hms/CMakeLists.txt
M src/kudu/subprocess/CMakeLists.txt
3 files changed, 4 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15438/3
--
To view, visit http://gerrit.cloudera.org:8080/15438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0bab628fa2d1fbb8d9edee32b480c30da2756cdc
Gerrit-Change-Number: 15438
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)