[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. KUDU-428: add Sentry to thirdparty, mini-sentry This commit adds Sentry to thirdparty, and fills out the MiniSentry class with an initial implementation. Notable features that aren't implemented: - Stripped Sentry packaging. I've put an unmodified version of Sentry 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about 5s to startup on my laptop. We will probably want to add a stripped version later to reduce both of these. - Kerberos support for mini-sentry. Right now Kerberos is disabled, which is an atypical configuration. A follow-up commit will add a Kerberos support configuration taking advantage of the mini KDC. - The mini Sentry is not yet configured with the location of the HMS, which will be necessary to do anything non-trivial with it. Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Reviewed-on: http://gerrit.cloudera.org:8080/11347 Reviewed-by: Adar Dembo Reviewed-by: Hao Hao Tested-by: Kudu Jenkins --- M build-support/dist_test.py M build-support/run_dist_test.py M src/kudu/hms/mini_hms.cc M src/kudu/sentry/CMakeLists.txt M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry_client-test.cc M src/kudu/util/test_util.cc M src/kudu/util/test_util.h M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 12 files changed, 281 insertions(+), 43 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Hao Hao: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 7 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 6: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh File thirdparty/vars.sh: http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh@230 PS5, Line 230: apache-sentry-$SENTRY_VERSION-bin > I'm planning to do the repackaging in a follow up commit, Adar and I talked Ah sorry, I missed that. -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 26 Sep 2018 22:38:05 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 26 Sep 2018 22:30:24 + Gerrit-HasComments: No
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/11347/5/src/kudu/sentry/mini_sentry.cc File src/kudu/sentry/mini_sentry.cc: http://gerrit.cloudera.org:8080/#/c/11347/5/src/kudu/sentry/mini_sentry.cc@86 PS5, Line 86: Su > nit: 4 space indent? Done http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh File thirdparty/vars.sh: http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh@230 PS5, Line 230: apache-sentry-$SENTRY_VERSION-bin > Can we use the same naming rule as for Hive and Hadoop? I'm planning to do the repackaging in a follow up commit, Adar and I talked about it on a different thread in this review. -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 26 Sep 2018 22:25:37 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Hello Tidy Bot, Andrew Wong, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11347 to look at the new patch set (#6). Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. KUDU-428: add Sentry to thirdparty, mini-sentry This commit adds Sentry to thirdparty, and fills out the MiniSentry class with an initial implementation. Notable features that aren't implemented: - Stripped Sentry packaging. I've put an unmodified version of Sentry 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about 5s to startup on my laptop. We will probably want to add a stripped version later to reduce both of these. - Kerberos support for mini-sentry. Right now Kerberos is disabled, which is an atypical configuration. A follow-up commit will add a Kerberos support configuration taking advantage of the mini KDC. - The mini Sentry is not yet configured with the location of the HMS, which will be necessary to do anything non-trivial with it. Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 --- M build-support/dist_test.py M build-support/run_dist_test.py M src/kudu/hms/mini_hms.cc M src/kudu/sentry/CMakeLists.txt M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry_client-test.cc M src/kudu/util/test_util.cc M src/kudu/util/test_util.h M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 12 files changed, 281 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/11347/6 -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 6 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/11347/5/src/kudu/sentry/mini_sentry.cc File src/kudu/sentry/mini_sentry.cc: http://gerrit.cloudera.org:8080/#/c/11347/5/src/kudu/sentry/mini_sentry.cc@86 PS5, Line 86: nit: 4 space indent? http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh File thirdparty/vars.sh: http://gerrit.cloudera.org:8080/#/c/11347/5/thirdparty/vars.sh@230 PS5, Line 230: apache-sentry-$SENTRY_VERSION-bin Can we use the same naming rule as for Hive and Hadoop? -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 5 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 26 Sep 2018 21:29:15 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 26 Sep 2018 20:38:19 + Gerrit-HasComments: No
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11347/3/src/kudu/hms/mini_hms.cc File src/kudu/hms/mini_hms.cc: http://gerrit.cloudera.org:8080/#/c/11347/3/src/kudu/hms/mini_hms.cc@77 PS3, Line 77: data_root_ = std::move(data_root); > warning: parameter 'data_root' is passed by value and only copied once; con Done -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 26 Sep 2018 20:25:38 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Hello Tidy Bot, Andrew Wong, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11347 to look at the new patch set (#4). Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. KUDU-428: add Sentry to thirdparty, mini-sentry This commit adds Sentry to thirdparty, and fills out the MiniSentry class with an initial implementation. Notable features that aren't implemented: - Stripped Sentry packaging. I've put an unmodified version of Sentry 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about 5s to startup on my laptop. We will probably want to add a stripped version later to reduce both of these. - Kerberos support for mini-sentry. Right now Kerberos is disabled, which is an atypical configuration. A follow-up commit will add a Kerberos support configuration taking advantage of the mini KDC. - The mini Sentry is not yet configured with the location of the HMS, which will be necessary to do anything non-trivial with it. Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 --- M build-support/dist_test.py M build-support/run_dist_test.py M src/kudu/hms/mini_hms.cc M src/kudu/sentry/CMakeLists.txt M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry_client-test.cc M src/kudu/util/test_util.cc M src/kudu/util/test_util.h M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 12 files changed, 281 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/11347/4 -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@13 PS2, Line 13: - Stripped Sentry packaging. I've put an unmodified version of Sentry : 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about : 5s to startup on my laptop. We will probably want to add a stripped : version later to reduce both of these. > Also I should note that it appears that Sentry's startup times are much bet 5s startup is still pretty bad, though I suppose that's on par with the (stripped) HMS. -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Wed, 26 Sep 2018 20:03:53 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@13 PS2, Line 13: - Stripped Sentry packaging. I've put an unmodified version of Sentry : 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about : 5s to startup on my laptop. We will probably want to add a stripped : version later to reduce both of these. > This is difficult to frontload, because we can't know all the necessary lib Also I should note that it appears that Sentry's startup times are much better than the HMS's, which was the original motivation for repackaging. -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 26 Sep 2018 19:07:50 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Dan Burkert has posted comments on this change. (
http://gerrit.cloudera.org:8080/11347 )
Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..
Patch Set 3:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@13
PS2, Line 13: - Stripped Sentry packaging. I've put an unmodified version of
Sentry
: 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes
about
: 5s to startup on my laptop. We will probably want to add a
stripped
: version later to reduce both of these.
> The Sentry integration isn't in any particular hurry, right? Maybe we can f
This is difficult to frontload, because we can't know all the necessary
libraries in the tarball until we are exercising all of the sentry
functionality we need. Unfortunately it doesn't look like there will be 'easy'
wins with Sentry like there were for Hadoop where the share/ (docs) directory
was huge. This needs to be revisited after we hook up more of the
functionality.
http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@22
PS2, Line 22: The mini Sentry is not yet configured with the location of the
HMS,
: which will be necessary to do anything non-trivial with it.
> So it's impossible to use ephemeral ports for both? Do either allow already
I agree with all of this. I'm going to leave it to a follow-up commit to
implement it, since it's not strictly necessary to do in this commit.
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.h
File src/kudu/sentry/mini_sentry.h:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.h@47
PS2, Line 47: // Stops the mini Sentry service.
: Status Stop() WARN_UNUSED_RESULT;
:
: // Pause the Sentry service.
: Status Pause() WARN_UNUSED_RESULT;
:
: // Unpause the Sentry service.
: Status Resume() WARN_UNUSED_RESULT;
> At some point we should consider inserting MiniSentry, MiniHMS, and MiniKDC
I'd prefer to wait for a situation in which we want to work with these
components in a generic way, otherwise the hierarchy will just be boilerplate.
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@88
PS2, Line 88: JoinPathSegments(tmp_dir, "sentry-site.xm
> nit: use JoinPathSegments?
Done
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@142
PS2, Line 142:
: sentry.service.security.mode
: none
:
> Right, I think the purpose if the flag is to allow Sentry to start up in a
Yep. A follow-up commit will flip this between the none and kerberos value
depending on whether kerberos is enabled.
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@169
PS2, Line 169: Substitut
> nit: not needed
Done
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@173
PS2, Line 173: JoinPathSegments(tmp_dir, "sentry-site.xml")
> Do you foresee CreateSentrySite() being used with any directory besides `Ge
No, however there are multiple things which need the raw tmp_dir (line 169
here, as well as an additional file in a follow-up commit).
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/sentry_client-test.cc@46
PS2, Line 46:
:
:
:
> As useful as this was, you can remove it now.
Done
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/util/test_util.cc@453
PS2, Line 453: string dir = env == nullptr ? JoinPathSegments(bin_dir,
Substitute("$0-home", name)) : env
> nit: Maybe don't set this if we're returning an error? Or document that it
Done
--
To view, visit http://gerrit.cloudera.org:8080/11347
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 26 Sep 2018 19:01:09 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Hello Andrew Wong, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11347 to look at the new patch set (#3). Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. KUDU-428: add Sentry to thirdparty, mini-sentry This commit adds Sentry to thirdparty, and fills out the MiniSentry class with an initial implementation. Notable features that aren't implemented: - Stripped Sentry packaging. I've put an unmodified version of Sentry 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about 5s to startup on my laptop. We will probably want to add a stripped version later to reduce both of these. - Kerberos support for mini-sentry. Right now Kerberos is disabled, which is an atypical configuration. A follow-up commit will add a Kerberos support configuration taking advantage of the mini KDC. - The mini Sentry is not yet configured with the location of the HMS, which will be necessary to do anything non-trivial with it. Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 --- M build-support/dist_test.py M build-support/run_dist_test.py M src/kudu/hms/mini_hms.cc M src/kudu/sentry/CMakeLists.txt M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry_client-test.cc M src/kudu/util/test_util.cc M src/kudu/util/test_util.h M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 12 files changed, 280 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/11347/3 -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc File src/kudu/sentry/mini_sentry.cc: http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@142 PS2, Line 142: : sentry.service.security.mode : none : > Yeah, eventually this will be a configurable parameter. I'm not sure at th Right, I think the purpose if the flag is to allow Sentry to start up in a non kerberized cluster. -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 05 Sep 2018 01:15:26 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/11347 )
Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@88
PS2, Line 88: Substitute("$0/sentry-site.xml", tmp_dir)
nit: use JoinPathSegments?
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@169
PS2, Line 169: strings::
nit: not needed
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@173
PS2, Line 173: JoinPathSegments(tmp_dir, "sentry-site.xml")
Do you foresee CreateSentrySite() being used with any directory besides
`GetTestDataDirectory()`? If not, maybe this can be a private member/function?
And use it here and L88
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/util/test_util.cc@453
PS2, Line 453: *home_dir = env == nullptr ? JoinPathSegments(bin_dir,
Substitute("$0-home", name)) : env;
nit: Maybe don't set this if we're returning an error? Or document that it
always sets `home_dir`
--
To view, visit http://gerrit.cloudera.org:8080/11347
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 30 Aug 2018 18:02:25 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@22 PS2, Line 22: The mini Sentry is not yet configured with the location of the HMS, : which will be necessary to do anything non-trivial with it. > One thing I realized after putting this patch up is that there's a circular So it's impossible to use ephemeral ports for both? Do either allow already bound ports to be transferred to the new process and reused? If not, can they bind to alternative localhost addresses in the 127.x.y.z space? Those are all the "easy" options. A crappy option is to pick one of them and restart it once as part of startup. Something like: - Start HMS. Find out what port it's on. - Start Sentry, configured to talk to HMS's port. Find out what port it's on. - Restart HMS on the same port, reconfigured to talk to Sentry's port. Besides inflating the amount of time it takes to start the cluster, HMS's ephemeral port may have been snatched up in the restart, so there's potential flakiness (or retries, I guess) there. -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 29 Aug 2018 21:06:01 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11347 ) Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@22 PS2, Line 22: The mini Sentry is not yet configured with the location of the HMS, : which will be necessary to do anything non-trivial with it. One thing I realized after putting this patch up is that there's a circular dependency between the HMS and Sentry in terms of configuring each with the other's address, including port. The way our mini servers have worked up till this point is that the process is started with configuration to bind to port 0, then the actual port is discovered by polling lsof. This isn't going to work when we have such a circular dependency, so we need to figure out what to do about that. http://gerrit.cloudera.org:8080/#/c/11347/2/build-support/run_dist_test.py File build-support/run_dist_test.py: http://gerrit.cloudera.org:8080/#/c/11347/2/build-support/run_dist_test.py@153 PS2, Line 153: apache-sentry-* > This will change when you repackage the Sentry package, right? Correct. http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc File src/kudu/sentry/mini_sentry.cc: http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@142 PS2, Line 142: : sentry.service.security.mode : none : > This one isn't explained above, but that's because it's temporary until Ker Yeah, eventually this will be a configurable parameter. I'm not sure at this point the extent of what this config flag does, other than Sentry doesn't complain about missing kerberos configuration at startup when it's set to 'none'. -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Wed, 29 Aug 2018 20:49:03 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11347 )
Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry
..
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11347/2//COMMIT_MSG@13
PS2, Line 13: - Stripped Sentry packaging. I've put an unmodified version of
Sentry
: 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes
about
: 5s to startup on my laptop. We will probably want to add a
stripped
: version later to reduce both of these.
The Sentry integration isn't in any particular hurry, right? Maybe we can
frontload this work, to save download time and space for Kudu developers who
don't otherwise care about Sentry?
http://gerrit.cloudera.org:8080/#/c/11347/2/build-support/run_dist_test.py
File build-support/run_dist_test.py:
http://gerrit.cloudera.org:8080/#/c/11347/2/build-support/run_dist_test.py@153
PS2, Line 153: apache-sentry-*
This will change when you repackage the Sentry package, right?
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.h
File src/kudu/sentry/mini_sentry.h:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.h@47
PS2, Line 47: // Stops the mini Sentry service.
: Status Stop() WARN_UNUSED_RESULT;
:
: // Pause the Sentry service.
: Status Pause() WARN_UNUSED_RESULT;
:
: // Unpause the Sentry service.
: Status Resume() WARN_UNUSED_RESULT;
At some point we should consider inserting MiniSentry, MiniHMS, and MiniKDC
into the ExternalDaemon class hierarchy. At they very least they could share
common Stop/Pause/Resume implementations.
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/mini_sentry.cc@142
PS2, Line 142:
: sentry.service.security.mode
: none
:
This one isn't explained above, but that's because it's temporary until
Kerberos support lands?
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/11347/2/src/kudu/sentry/sentry_client-test.cc@46
PS2, Line 46: TEST_F(SentryClientTest, ItWorks) {
: SentryClient client;
: std::move(client);
: }
As useful as this was, you can remove it now.
--
To view, visit http://gerrit.cloudera.org:8080/11347
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Gerrit-Change-Number: 11347
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 29 Aug 2018 20:42:11 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Hello Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11347 to look at the new patch set (#2). Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. KUDU-428: add Sentry to thirdparty, mini-sentry This commit adds Sentry to thirdparty, and fills out the MiniSentry class with an initial implementation. Notable features that aren't implemented: - Stripped Sentry packaging. I've put an unmodified version of Sentry 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about 5s to startup on my laptop. We will probably want to add a stripped version later to reduce both of these. - Kerberos support for mini-sentry. Right now Kerberos is disabled, which is an atypical configuration. A follow-up commit will add a Kerberos support configuration taking advantage of the mini KDC. - The mini Sentry is not yet configured with the location of the HMS, which will be necessary to do anything non-trivial with it. Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 --- M build-support/dist_test.py M build-support/run_dist_test.py M src/kudu/hms/mini_hms.cc M src/kudu/sentry/CMakeLists.txt M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry_client-test.cc M src/kudu/util/test_util.cc M src/kudu/util/test_util.h M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 12 files changed, 273 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/11347/2 -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-428: add Sentry to thirdparty, mini-sentry
Dan Burkert has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11347 Change subject: KUDU-428: add Sentry to thirdparty, mini-sentry .. KUDU-428: add Sentry to thirdparty, mini-sentry This commit adds Sentry to thirdparty, and fills out the MiniSentry class with an initial implementation. Notable features that aren't implemented: - Stripped Sentry packaging. I've put an unmodified version of Sentry 2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about 5s to startup on my laptop. We will probably want to add a stripped version later to reduce both of these. - Kerberos support for mini-sentry. Right now Kerberos is disabled, which is an atypical configuration. A follow-up commit will add a Kerberos support configuration taking advantage of the mini KDC. - The mini Sentry is not yet configured with the location of the HMS, which will be necessary to do anything non-trivial with it. Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 --- M build-support/dist_test.py M build-support/run_dist_test.py M src/kudu/hms/mini_hms.cc M src/kudu/sentry/CMakeLists.txt M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry_client-test.cc M src/kudu/util/test_util.cc M src/kudu/util/test_util.h M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/vars.sh 12 files changed, 272 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/11347/1 -- To view, visit http://gerrit.cloudera.org:8080/11347 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5 Gerrit-Change-Number: 11347 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert
