[kudu-CR] [build] Move default sanitizer options into build from shell scripts
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11176 ) Change subject: [build] Move default sanitizer options into build from shell scripts .. Patch Set 7: (3 comments) The past few runs have been failing in TSAN in the same way. I suspect the suppressions (or maybe all of the options) aren't getting through. http://gerrit.cloudera.org:8080/#/c/11176/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11176/7//COMMIT_MSG@17 PS7, Line 17: The only option that was not provided at build time is : the external_symbolizer_path. Instead the : llvm_symbolizer from thirdparty is added to the PATH : so it can be found if no custom : external_symbolizer_path is provided. Please explain why this deviation was necessary. http://gerrit.cloudera.org:8080/#/c/11176/7/build-support/jenkins/build-and-test.sh File build-support/jenkins/build-and-test.sh: http://gerrit.cloudera.org:8080/#/c/11176/7/build-support/jenkins/build-and-test.sh@176 PS7, Line 176: # Sanitizer suppressions require symbolization. Does this work? Don't PATH entries need to be directories? $ PATH=~/Source/kudu/thirdparty/installed/uninstrumented/bin/llvm-symbolizer:$PATH; llvm-symbolizer --version Command 'llvm-symbolizer' not found, but can be installed with: sudo apt install llvm $ PATH=~/Source/kudu/thirdparty/installed/uninstrumented/bin:$PATH; llvm-symbolizer --version LLVM (http://llvm.org/): LLVM version 6.0.0 Optimized build. Default target: x86_64-unknown-linux-gnu Host CPU: skylake http://gerrit.cloudera.org:8080/#/c/11176/7/build-support/jenkins/build-and-test.sh@180 PS7, Line 180: export PATH=$(pwd)/thirdparty/installed/uninstrumented/bin/llvm-symbolizer:$PATH So the downside of placing this here is that we don't get it if we run Java tests locally, or via an IDE, or basically in any other way that doesn't go through build-and-test.sh and doesn't use dist-test. -- To view, visit http://gerrit.cloudera.org:8080/11176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9eab2d3ef0b107ae9a4e971bf8a6514bf425f645 Gerrit-Change-Number: 11176 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Aug 2018 04:28:29 + Gerrit-HasComments: Yes
[kudu-CR] Bump Hive version, normalize hive package name
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11277 ) Change subject: Bump Hive version, normalize hive package name .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11277/3/thirdparty/package-hive.sh File thirdparty/package-hive.sh: http://gerrit.cloudera.org:8080/#/c/11277/3/thirdparty/package-hive.sh@37 PS3, Line 37: wget https://archive.apache.org/dist/hive/hive-$VERSION/$ARTIFACT.tar.gz I left this feedback in Hao's patch too, so it may not apply here, but could we use curl here instead, to be more consistent with the rest of the thirdparty scripts? -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 21 Aug 2018 04:16:28 + Gerrit-HasComments: Yes
[kudu-CR] [thirdparty] allow different URL prefix
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11275 )
Change subject: [thirdparty] allow different URL prefix
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh@129
PS1, Line 129: fetch_with_url_and_patch() {
> The problem here is fetch_and_patch already has an optional argument for 'C
Ah good point. OK, so let's just focus on making fetch_and_patch simpler.
Perhaps like this?
// Call fetch_with_url_and_patch with the default dependency URL source.
fetch_and_patch() {
fetch_with_url_and_patch $1 $2 $3 $DEP_URL "$@"
}
(I still think CLOUDFRONT_URL_PREFIX should be read-only and there should be a
new variable to represent the URL that defaults to CLOUDFRONT_URL_PREFIX but
may be overridden/patched).
--
To view, visit http://gerrit.cloudera.org:8080/11275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004828785d9b0d484806d743d7a1084a3ffd576b
Gerrit-Change-Number: 11275
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 21 Aug 2018 04:14:24 +
Gerrit-HasComments: Yes
[kudu-CR] [hms] Add Hcatalog jars to the HMS classpath
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11273 )
Change subject: [hms] Add Hcatalog jars to the HMS classpath
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11273/2/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11273/2/src/kudu/hms/mini_hms.cc@119
PS2, Line 119: string aux_jars =
Substitute("$0/hms-plugin.jar,$1/hcatalog/share/hcatalog/*", bin_dir,
hive_home);
Looks like you need to wrap this to make lint happy.
--
To view, visit http://gerrit.cloudera.org:8080/11273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifed85fffdabb5af19f22e343843e012b009888a0
Gerrit-Change-Number: 11273
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 21 Aug 2018 04:08:28 +
Gerrit-HasComments: Yes
[kudu-CR] [hms] use IPv4 for mini HMS
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11276 ) Change subject: [hms] use IPv4 for mini HMS .. Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/11276/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11276/4//COMMIT_MSG@9 PS4, Line 9: protocal protocol http://gerrit.cloudera.org:8080/#/c/11276/4//COMMIT_MSG@10 PS4, Line 10: pass are passed http://gerrit.cloudera.org:8080/#/c/11276/4//COMMIT_MSG@10 PS4, Line 10: . Since ", because" -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 21 Aug 2018 04:07:34 + Gerrit-HasComments: Yes
[kudu-CR] [hms] update the HIVE/HADOOP package scripts
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11274 ) Change subject: [hms] update the HIVE/HADOOP package scripts .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/11274/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11274/2//COMMIT_MSG@10 PS2, Line 10: do Nit: to do http://gerrit.cloudera.org:8080/#/c/11274/2/thirdparty/package-hadoop.sh File thirdparty/package-hadoop.sh: http://gerrit.cloudera.org:8080/#/c/11274/2/thirdparty/package-hadoop.sh@49 PS2, Line 49: ; Nit: eliminate extra space here. http://gerrit.cloudera.org:8080/#/c/11274/2/thirdparty/package-hadoop.sh@72 PS2, Line 72: wget https://archive.apache.org/dist/hadoop/common/$ARTIFACT/$ARTIFACT.tar.gz Should have noticed this earlier, but let's use curl here instead of wget; the rest of the thirdparty scripts use curl. http://gerrit.cloudera.org:8080/#/c/11274/2/thirdparty/package-hive.sh File thirdparty/package-hive.sh: http://gerrit.cloudera.org:8080/#/c/11274/2/thirdparty/package-hive.sh@51 PS2, Line 51: ; Nit: extra space. http://gerrit.cloudera.org:8080/#/c/11274/2/thirdparty/package-hive.sh@75 PS2, Line 75: wget https://archive.apache.org/dist/hive/hive-$VERSION/$ARTIFACT.tar.gz Use curl. -- To view, visit http://gerrit.cloudera.org:8080/11274 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie66c4e132800fbf5ea76ad4b1fec8212757c84c2 Gerrit-Change-Number: 11274 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 21 Aug 2018 04:06:36 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11259 ) Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. KUDU-2531: (part 1) Ignore invalid tablet metadata files Changes the behavior of FsManager::IsValidTabletId to return false for all files that are named with invalid tabet ids. This results in all files that are not valid table ids being ignored, instead of just tmp files. Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Reviewed-on: http://gerrit.cloudera.org:8080/11259 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h 3 files changed, 37 insertions(+), 16 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11260 ) Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool Adds a -nobackup flag to the pbc edit tool to simplify usage when the backup file will not be used and the user doesn’t want to worry about cleanup. Change-Id: If544f7682a30933077db0824452639a68507ba40 Reviewed-on: http://gerrit.cloudera.org:8080/11260 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc 2 files changed, 50 insertions(+), 12 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 8 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11259 ) Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. Patch Set 6: Code-Review+2 Carrying the +2 through the rebase and lint fix. -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 6 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 21 Aug 2018 03:31:25 + Gerrit-HasComments: No
[kudu-CR] [build] Move default sanitizer options into build from shell scripts
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11176 ) Change subject: [build] Move default sanitizer options into build from shell scripts .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/11176/6/src/kudu/util/CMakeLists.txt File src/kudu/util/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/11176/6/src/kudu/util/CMakeLists.txt@327 PS6, Line 327: kudu_test_util) > Nit: should precede kudu_util. Done http://gerrit.cloudera.org:8080/#/c/11176/6/src/kudu/util/sanitizer_options.cc File src/kudu/util/sanitizer_options.cc: http://gerrit.cloudera.org:8080/#/c/11176/6/src/kudu/util/sanitizer_options.cc@35 PS6, Line 35: #endif // ADDRESS_SANITIZER > Nit: could we inline kAsanDefaultOptions directly into the return statement Done -- To view, visit http://gerrit.cloudera.org:8080/11176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9eab2d3ef0b107ae9a4e971bf8a6514bf425f645 Gerrit-Change-Number: 11176 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 Aug 2018 03:27:54 + Gerrit-HasComments: Yes
[kudu-CR] [build] Move default sanitizer options into build from shell scripts
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11176 to look at the new patch set (#7). Change subject: [build] Move default sanitizer options into build from shell scripts .. [build] Move default sanitizer options into build from shell scripts This moves the sanitizer options and suppressions from the build scripts using TSAN_OPTIONS/LSAN_OPTIONS/etc into the build and linked at build time. This ensures the same configurations are used for C++ and Java tests. The only option that was not provided at build time is the external_symbolizer_path. Instead the llvm_symbolizer from thirdparty is added to the PATH so it can be found if no custom external_symbolizer_path is provided. Change-Id: I9eab2d3ef0b107ae9a4e971bf8a6514bf425f645 --- M README.adoc M build-support/dist_test.py M build-support/jenkins/build-and-test.sh D build-support/lsan-suppressions.txt M build-support/run-test.sh M build-support/run_dist_test.py D build-support/tsan-suppressions.txt M src/kudu/util/CMakeLists.txt A src/kudu/util/sanitizer_options.cc 9 files changed, 207 insertions(+), 178 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/11176/7 -- To view, visit http://gerrit.cloudera.org:8080/11176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9eab2d3ef0b107ae9a4e971bf8a6514bf425f645 Gerrit-Change-Number: 11176 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11260 ) Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. Patch Set 7: Code-Review+2 Carrying the +2 through the rebase. -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 21 Aug 2018 03:02:11 + Gerrit-HasComments: No
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11259 to look at the new patch set (#6). Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. KUDU-2531: (part 1) Ignore invalid tablet metadata files Changes the behavior of FsManager::IsValidTabletId to return false for all files that are named with invalid tabet ids. This results in all files that are not valid table ids being ignored, instead of just tmp files. Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 --- M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h 3 files changed, 37 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/11259/6 -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 6 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [test] Use valid tablet ids in tests
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11272 ) Change subject: [test] Use valid tablet ids in tests .. [test] Use valid tablet ids in tests Some tests used tablet ids that were not valid Kudu object ids. This patch changes them to use a valid id in preperaption for a follow on patch which makes tablet id validation more strict. Change-Id: I83d774abf2cb2ce6f81d1f851f2cc1ddd12e1726 Reviewed-on: http://gerrit.cloudera.org:8080/11272 Reviewed-by: Adar Dembo Reviewed-by: Andrew Wong Reviewed-by: Attila Bukor Tested-by: Kudu Jenkins --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/tablet_server-test-base.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager-test.cc 4 files changed, 37 insertions(+), 35 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Andrew Wong: Looks good to me, approved Attila Bukor: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I83d774abf2cb2ce6f81d1f851f2cc1ddd12e1726 Gerrit-Change-Number: 11272 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Bump Hive version, normalize hive package name
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11277 ) Change subject: Bump Hive version, normalize hive package name .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py File build-support/run_dist_test.py: http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py@141 PS2, Line 141: "))[ > no problem, will update shortly. Done -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 21 Aug 2018 01:09:27 + Gerrit-HasComments: Yes
[kudu-CR] Bump Hive version, normalize hive package name
Hello Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11277 to look at the new patch set (#3). Change subject: Bump Hive version, normalize hive package name .. Bump Hive version, normalize hive package name This commit bumps the Hive version in thirdparty, and strips the 'apache' prefix and 'bin' suffix from the thirdparty tarball package, which brings it inline with the hadoop package naming. Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb --- M build-support/run_dist_test.py M thirdparty/package-hive.sh M thirdparty/vars.sh 3 files changed, 7 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11277/3 -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [hms] Add Hcatalog jars to the HMS classpath
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11273 )
Change subject: [hms] Add Hcatalog jars to the HMS classpath
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11273/1/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11273/1/src/kudu/hms/mini_hms.cc@119
PS1, Line 119: string aux_jars =
Substitute("$0/hms-plugin.jar,$1/hcatalog/share/hcatalog/*", bin_dir,
hive_home);
:
> Nit: I think it's clearer if combined:
Done
--
To view, visit http://gerrit.cloudera.org:8080/11273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifed85fffdabb5af19f22e343843e012b009888a0
Gerrit-Change-Number: 11273
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 21 Aug 2018 01:06:13 +
Gerrit-HasComments: Yes
[kudu-CR] [hms] Add Hcatalog jars to the HMS classpath
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11273 ) Change subject: [hms] Add Hcatalog jars to the HMS classpath .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/11273/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11273/1//COMMIT_MSG@11 PS1, Line 11: listener > listener Done http://gerrit.cloudera.org:8080/#/c/11273/1//COMMIT_MSG@12 PS1, Line 12: adds > adds Done -- To view, visit http://gerrit.cloudera.org:8080/11273 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifed85fffdabb5af19f22e343843e012b009888a0 Gerrit-Change-Number: 11273 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 21 Aug 2018 01:06:03 + Gerrit-HasComments: Yes
[kudu-CR] [hms] Add Hcatalog jars to the HMS classpath
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11273 to look at the new patch set (#2). Change subject: [hms] Add Hcatalog jars to the HMS classpath .. [hms] Add Hcatalog jars to the HMS classpath Due to the packaging differences of Hive distributions, the Hcatalog jars may not be loaded into the classpath of mini HMS, which will cause the ClassNotFoundException when using notification log event listener. This patch explicitly adds these jars. Change-Id: Ifed85fffdabb5af19f22e343843e012b009888a0 --- M src/kudu/hms/mini_hms.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/11273/2 -- To view, visit http://gerrit.cloudera.org:8080/11273 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifed85fffdabb5af19f22e343843e012b009888a0 Gerrit-Change-Number: 11273 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [thirdparty] allow different URL prefix
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11275 )
Change subject: [thirdparty] allow different URL prefix
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh@129
PS1, Line 129: fetch_with_url_and_patch() {
> Could we avoid needing both fetch_with_url_and_patch and fetch_and_patch by
The problem here is fetch_and_patch already has an optional argument for
'COMMANDS'. I do not see a clean way to get away with that. Do you have some
suggestions? Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/11275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004828785d9b0d484806d743d7a1084a3ffd576b
Gerrit-Change-Number: 11275
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 21 Aug 2018 00:49:06 +
Gerrit-HasComments: Yes
[kudu-CR] [hms] use IPv4 for mini HMS
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11276 to look at the new patch set (#4). Change subject: [hms] use IPv4 for mini HMS .. [hms] use IPv4 for mini HMS This patch explictly uses IPv4 protocal in the JVM environment options that pass to the mini HMS, to avoid IPv6 being accidentally used. Since IPv6 is not supported in Hadoop yet (https://wiki.apache.org/hadoop/HadoopIPv6). Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 --- M src/kudu/hms/mini_hms.cc 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/11276/4 -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [hms] use IPv4 for mini HMS
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11276 ) Change subject: [hms] use IPv4 for mini HMS .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG@10 PS1, Line 10: that pass to the mini HMS, to avoid IPv6 being accidentally used. Since > Makes sense. Could you add some of this into the commit message? Done -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 21 Aug 2018 00:23:28 + Gerrit-HasComments: Yes
[kudu-CR] [hms] use IPv4 for mini HMS
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11276 to look at the new patch set (#3). Change subject: [hms] use IPv4 for mini HMS .. [hms] use IPv4 for mini HMS This patch explictly uses IPv4 protocal in the JVM environment options that pass to the mini HMS, to avoid IPv6 being accidentally used. Since IPv6 is not supported in Hadoop yet (wiki.apache.org/hadoop/HadoopIPv6). Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 --- M src/kudu/hms/mini_hms.cc 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/11276/3 -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [hms] update the HIVE/HADOOP package scripts
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11274 to look at the new patch set (#2). Change subject: [hms] update the HIVE/HADOOP package scripts .. [hms] update the HIVE/HADOOP package scripts This patch updates the HIVE/HADOOP package scripts to be more flexible, so that users can choose whether do jar downloading or repackaging. It also adjusts the jars that need to be stripped for universal HIVE and HADOOP distributions. Change-Id: Ie66c4e132800fbf5ea76ad4b1fec8212757c84c2 --- M thirdparty/package-hadoop.sh M thirdparty/package-hive.sh 2 files changed, 151 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/11274/2 -- To view, visit http://gerrit.cloudera.org:8080/11274 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie66c4e132800fbf5ea76ad4b1fec8212757c84c2 Gerrit-Change-Number: 11274 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [hms] update the HIVE/HADOOP package scripts
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11274 )
Change subject: [hms] update the HIVE/HADOOP package scripts
..
Patch Set 2:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG@10
PS1, Line 10: repackagi
> repackaging
Done
http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG@10
PS1, Line 10: choos
> choose
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh
File thirdparty/package-hadoop.sh:
PS1:
> Pretty much all of the feedback here applies to package-hive.sh too.
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@26
PS1, Line 26: optional
> Nit: optional (lower case).
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@39
PS1, Line 39: Download
> Download
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@40
PS1, Line 40: --repackag
> --repackage
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@49
PS1, Line 49: if [ ${OPTS_RESULT} != 0 ] ; then
> Nit: break this up onto separate lines.
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@51
PS1, Line 51: TS
> Nit: separate with a space from the semicolon
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@53
PS1, Line 53:
> This was just for debugging, right?
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@55
PS1, Line 55: case "$1" in
> Nit: -h | --help)
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@69
PS1, Line 69: fi
> Remove this?
Done
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@74
PS1, Line 74: fi
> Can you construct this list more clearly? Like:
Done
--
To view, visit http://gerrit.cloudera.org:8080/11274
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie66c4e132800fbf5ea76ad4b1fec8212757c84c2
Gerrit-Change-Number: 11274
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 21 Aug 2018 00:17:15 +
Gerrit-HasComments: Yes
[kudu-CR] [hms] use IPv4 for mini HMS
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11276 ) Change subject: [hms] use IPv4 for mini HMS .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG@10 PS1, Line 10: that pass to the mini HMS, to avoid IPv6 being accidentally used, which > Hadoop does not support IPv4 yet, here is the wiki page reference: https:// Makes sense. Could you add some of this into the commit message? -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 23:44:45 + Gerrit-HasComments: Yes
[kudu-CR] [hms] use IPv4 for mini HMS
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11276 ) Change subject: [hms] use IPv4 for mini HMS .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG@10 PS1, Line 10: that pass to the mini HMS, to avoid IPv6 being accidentally used, which > Our HMS-based tests are all passing today, so what was the issue with using Hadoop does not support IPv4 yet, here is the wiki page reference: https://wiki.apache.org/hadoop/HadoopIPv6 and the jira to support it: https://issues.apache.org/jira/browse/HADOOP-11890?subTaskView=all. I think our current HMS-based tests are passing today because the Hadoop distribution we are using has the HADOOP_OPTS set with '-Djava.net.preferIPv4Stack=true'. But in case it is not, issues will occur. http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG@11 PS1, Line 11: support > supported Done -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 23:34:00 + Gerrit-HasComments: Yes
[kudu-CR] [hms] use IPv4 for mini HMS
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11276 to look at the new patch set (#2). Change subject: [hms] use IPv4 for mini HMS .. [hms] use IPv4 for mini HMS This patch explictly uses IPv4 protocal in the JVM environment options that pass to the mini HMS, to avoid IPv6 being accidentally used, which is not supported in Hadoop/Hive yet. Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 --- M src/kudu/hms/mini_hms.cc 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/11276/2 -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Bump Hive version, strip apache prefix from package
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11277 ) Change subject: Bump Hive version, strip apache prefix from package .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py File build-support/run_dist_test.py: http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py@141 PS2, Line 141: -bin > Yeah, I think we need to remove -bin here as Adar suggested, otherwise it w no problem, will update shortly. -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 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: Mon, 20 Aug 2018 23:13:09 + Gerrit-HasComments: Yes
[kudu-CR] Bump Hive version, strip apache prefix from package
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11277 ) Change subject: Bump Hive version, strip apache prefix from package .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py File build-support/run_dist_test.py: http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py@141 PS2, Line 141: -bin > Hao, would that cause issues with the other packaging issues you are dealin Yeah, I think we need to remove -bin here as Adar suggested, otherwise it would cause issues. Sorry that I forgot to mention it when we discussed. -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 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: Mon, 20 Aug 2018 22:43:50 + Gerrit-HasComments: Yes
[kudu-CR] [test] Use valid tablet ids in tests
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11272 ) Change subject: [test] Use valid tablet ids in tests .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d774abf2cb2ce6f81d1f851f2cc1ddd12e1726 Gerrit-Change-Number: 11272 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 22:27:28 + Gerrit-HasComments: No
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11260 ) Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 6 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 22:32:37 + Gerrit-HasComments: No
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11259 ) Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 22:30:08 + Gerrit-HasComments: No
[kudu-CR] Bump Hive version, strip apache prefix from package
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11277 ) Change subject: Bump Hive version, strip apache prefix from package .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py File build-support/run_dist_test.py: http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py@141 PS2, Line 141: -bin > Can we drop that suffix for even more consistency? Hao, would that cause issues with the other packaging issues you are dealing with? http://gerrit.cloudera.org:8080/#/c/11277/2/thirdparty/vars.sh File thirdparty/vars.sh: http://gerrit.cloudera.org:8080/#/c/11277/2/thirdparty/vars.sh@211 PS2, Line 211: # TODO(dan): bump to a release version once HIVE-17747 and HIVE-16886/HIVE-18526 : # are published. The SHA below is the current head of branch-2. > I take it this is still at large? Yes, the comment is still correct. -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 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: Mon, 20 Aug 2018 22:20:58 + Gerrit-HasComments: Yes
[kudu-CR] [thirdparty] allow different URL prefix
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11275 )
Change subject: [thirdparty] allow different URL prefix
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh@58
PS1, Line 58: local URL_PREFIX=$3
> I don't think it makes sense to parameterize this, because it's probably go
I misunderstood Hao's use case: she does want to download some dependencies
from an alternative URL, and the rest from the usual Cloudfront URL. So
parameterization is necessary.
http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh@129
PS1, Line 129: fetch_with_url_and_patch() {
Could we avoid needing both fetch_with_url_and_patch and fetch_and_patch by
adding URL_PREFIX as the (optional) last argument to fetch_and_patch? With a
good comment I think it'll be pretty obvious how one should use the function.
--
To view, visit http://gerrit.cloudera.org:8080/11275
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I004828785d9b0d484806d743d7a1084a3ffd576b
Gerrit-Change-Number: 11275
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 20 Aug 2018 22:04:58 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1291: efficiently support predicates on non-prefix key components
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/10983 )
Change subject: KUDU-1291: efficiently support predicates on non-prefix key
components
..
Patch Set 20:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10983/20/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:
http://gerrit.cloudera.org:8080/#/c/10983/20/src/kudu/cfile/cfile_reader.cc@777
PS20, Line 777: if (PREDICT_TRUE(validx_iter_ != nullptr)) {
: RETURN_NOT_OK(StoreCurrentValue());
: }
> To make sure this patch has no side effects when disabled, this should prob
Yep, I think that's a good catch. If we want to have a better assurance that
the feature is truly isolated, it's better to gate it on something specific for
'skip index scan'. However, I don't think we have that gate readily available
in current implementation.
--
To view, visit http://gerrit.cloudera.org:8080/10983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f
Gerrit-Change-Number: 10983
Gerrit-PatchSet: 20
Gerrit-Owner: Anupama Gupta
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Anupama Gupta
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 20 Aug 2018 21:38:06 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11260 ) Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 6 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 21:34:28 + Gerrit-HasComments: No
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11259 ) Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 21:34:02 + Gerrit-HasComments: No
[kudu-CR] Bump Hive version, strip apache prefix from package
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11277 ) Change subject: Bump Hive version, strip apache prefix from package .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py File build-support/run_dist_test.py: http://gerrit.cloudera.org:8080/#/c/11277/2/build-support/run_dist_test.py@141 PS2, Line 141: -bin Can we drop that suffix for even more consistency? http://gerrit.cloudera.org:8080/#/c/11277/2/thirdparty/vars.sh File thirdparty/vars.sh: http://gerrit.cloudera.org:8080/#/c/11277/2/thirdparty/vars.sh@211 PS2, Line 211: # TODO(dan): bump to a release version once HIVE-17747 and HIVE-16886/HIVE-18526 : # are published. The SHA below is the current head of branch-2. I take it this is still at large? -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 21:33:10 + Gerrit-HasComments: Yes
[kudu-CR] Bump Hive version, strip apache prefix from package
Hello Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11277 to look at the new patch set (#2). Change subject: Bump Hive version, strip apache prefix from package .. Bump Hive version, strip apache prefix from package This commit bumps the Hive version in thirdparty, and strips the 'apache' prefix from the thirdparty tarball package, which brings it inline with the hadoop package naming. Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb --- M build-support/run_dist_test.py M thirdparty/package-hive.sh M thirdparty/vars.sh 3 files changed, 13 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11277/2 -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Bump Hive version, strip apache prefix from package
Hello Hao Hao, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11277 to review the following change. Change subject: Bump Hive version, strip apache prefix from package .. Bump Hive version, strip apache prefix from package This commit bumps the Hive version in thirdparty, and strips the 'apache' prefix from the thirdparty tarball package, which brings it inline with the hadoop package naming. Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb --- M build-support/run_dist_test.py M thirdparty/package-hive.sh M thirdparty/vars.sh 3 files changed, 13 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/11277/1 -- To view, visit http://gerrit.cloudera.org:8080/11277 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5cf6696e2a49986aba025f13653dc549e6fc5beb Gerrit-Change-Number: 11277 Gerrit-PatchSet: 1 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Hao Hao
[kudu-CR] WIP: Tool to read a cfile directly
Grant Henke has abandoned this change. ( http://gerrit.cloudera.org:8080/11061 ) Change subject: WIP: Tool to read a cfile directly .. Abandoned not needed -- To view, visit http://gerrit.cloudera.org:8080/11061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ie902fa931ad3a111ec9432bc1c99f2f0d5f021b8 Gerrit-Change-Number: 11061 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2059 fix race in ThreadPool while updating metrics
Alexey Serbin has abandoned this change. ( http://gerrit.cloudera.org:8080/11271 ) Change subject: KUDU-2059 fix race in ThreadPool while updating metrics .. Abandoned That would not help to resolve the actual underneath problem. -- To view, visit http://gerrit.cloudera.org:8080/11271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I930c7e388001446e67b244a2b3e71d5afdcaf516 Gerrit-Change-Number: 11271 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2059 fix race in ThreadPool while updating metrics
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11271 ) Change subject: KUDU-2059 fix race in ThreadPool while updating metrics .. Patch Set 1: > (1 comment) Ah, right -- warding off this issue would not help to resolve that problem. -- To view, visit http://gerrit.cloudera.org:8080/11271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I930c7e388001446e67b244a2b3e71d5afdcaf516 Gerrit-Change-Number: 11271 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 20:06:25 + Gerrit-HasComments: No
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11260 to look at the new patch set (#6). Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool Adds a -nobackup flag to the pbc edit tool to simplify usage when the backup file will not be used and the user doesn’t want to worry about cleanup. Change-Id: If544f7682a30933077db0824452639a68507ba40 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc 2 files changed, 50 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/11260/6 -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 6 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11259 to look at the new patch set (#5). Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. KUDU-2531: (part 1) Ignore invalid tablet metadata files Changes the behavior of FsManager::IsValidTabletId to return false for all files that are named with invalid tabet ids. This results in all files that are not valid table ids being ignored, instead of just tmp files. Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 --- M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h 3 files changed, 36 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/11259/5 -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11260 ) Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/11260/5/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/11260/5/src/kudu/tools/kudu-tool-test.cc@365 PS5, Line 365: *found = false; > Nit: we try to stick to the calling convention that if a function returns n Done http://gerrit.cloudera.org:8080/#/c/11260/5/src/kudu/tools/kudu-tool-test.cc@877 PS5, Line 877: bool found_backup = false; > No need to initialize this or on L889; HasAtLeastOneBackupFile guarantees t Done -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:57:55 + Gerrit-HasComments: Yes
[kudu-CR] [hms] Add Hcatalog jars to the HMS classpath
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11273 )
Change subject: [hms] Add Hcatalog jars to the HMS classpath
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11273/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11273/1//COMMIT_MSG@11
PS1, Line 11: lisenter
listener
http://gerrit.cloudera.org:8080/#/c/11273/1//COMMIT_MSG@12
PS1, Line 12: added
adds
http://gerrit.cloudera.org:8080/#/c/11273/1/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11273/1/src/kudu/hms/mini_hms.cc@119
PS1, Line 119: string hcatalog_jars =
Substitute("$0/hcatalog/share/hcatalog/*", hive_home);
: string aux_jars = Substitute("$0/hms-plugin.jar,$1", bin_dir,
hcatalog_jars);
Nit: I think it's clearer if combined:
string aux_jars =
Substitute("$0/hms-plugin.jar,$1/hcatalog/share/hcatalog/*", bin_dir,
hive_home);
If/when the list gets longer, we should treat it as a list, set it up entry by
entry, and then join it together.
--
To view, visit http://gerrit.cloudera.org:8080/11273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifed85fffdabb5af19f22e343843e012b009888a0
Gerrit-Change-Number: 11273
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 20 Aug 2018 19:53:43 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11259 ) Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11259/4/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/11259/4/src/kudu/fs/fs_manager.cc@633 PS4, Line 633: ObjectIdGenerator oid_generator; > Creating a new one of these for every call seems wasteful when we can have Done -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:51:12 + Gerrit-HasComments: Yes
[kudu-CR] [hms] update the HIVE/HADOOP package scripts
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11274 )
Change subject: [hms] update the HIVE/HADOOP package scripts
..
Patch Set 1:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG@10
PS1, Line 10: chose
choose
http://gerrit.cloudera.org:8080/#/c/11274/1//COMMIT_MSG@10
PS1, Line 10: repackaing
repackaging
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh
File thirdparty/package-hadoop.sh:
PS1:
Pretty much all of the feedback here applies to package-hive.sh too.
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@26
PS1, Line 26: Optional
Nit: optional (lower case).
On L28 too.
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@39
PS1, Line 39: Dowdload
Download
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@40
PS1, Line 40: -repackage
--repackage
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@49
PS1, Line 49: if [ ${OPTS_RESULT} != 0 ] ; then echo "Failed parsing options."
>&2 ; exit ${OPTS_RESULT} ; fi
Nit: break this up onto separate lines.
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@51
PS1, Line 51: do
Nit: separate with a space from the semicolon
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@53
PS1, Line 53: echo here
This was just for debugging, right?
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@55
PS1, Line 55: -h | --help )
Nit: -h | --help)
(to the other cases here too)
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@69
PS1, Line 69: echo here
Remove this?
http://gerrit.cloudera.org:8080/#/c/11274/1/thirdparty/package-hadoop.sh@74
PS1, Line 74: for DIR in client common/jdiff common/sources hdfs/sources httpfs
kms mapreduce/lib-examples mapreduce/sources tools; do
Can you construct this list more clearly? Like:
DIRS="client"
DIRS="$DIRS common/jdiff"
DIRS="$DIRS common/sources"
...
for DIR in $DIRS; do
...
done
While more verbose, this format makes it easier to find individual entries and
to keep them sorted.
--
To view, visit http://gerrit.cloudera.org:8080/11274
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie66c4e132800fbf5ea76ad4b1fec8212757c84c2
Gerrit-Change-Number: 11274
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 20 Aug 2018 19:50:35 +
Gerrit-HasComments: Yes
[kudu-CR] [hms] use IPv4 for mini HMS
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11276 ) Change subject: [hms] use IPv4 for mini HMS .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG@10 PS1, Line 10: that pass to the mini HMS, to avoid IPv6 being accidentally used, which Our HMS-based tests are all passing today, so what was the issue with using IPv6? http://gerrit.cloudera.org:8080/#/c/11276/1//COMMIT_MSG@11 PS1, Line 11: support supported -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:44:14 + Gerrit-HasComments: Yes
[kudu-CR] [test] Use valid tablet ids in tests
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11272 ) Change subject: [test] Use valid tablet ids in tests .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d774abf2cb2ce6f81d1f851f2cc1ddd12e1726 Gerrit-Change-Number: 11272 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:43:37 + Gerrit-HasComments: No
[kudu-CR] [thirdparty] allow different URL prefix
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11275 ) Change subject: [thirdparty] allow different URL prefix .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/11275/1/thirdparty/download-thirdparty.sh@58 PS1, Line 58: local URL_PREFIX=$3 I don't think it makes sense to parameterize this, because it's probably going to be the same for _all_ dependencies in a given run of download-thirdparty.sh. Instead, I would add indirection to the URL prefixing variables. Maybe something like: 1. Keep CLOUDFRONT_URL_PREFIX as a read-only variable with the URL for Cloudfront-based downloads. 2. Introduce a new URL variable in vars.sh which defaults to CLOUDFRONT_URL_PREFIX. 3. Use this new variable as the download source in download-thirdparty.sh. Then anyone who wants to use a different download source need only override that new URL variable in the environment, or patch vars.sh to set it to some other value. -- To view, visit http://gerrit.cloudera.org:8080/11275 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I004828785d9b0d484806d743d7a1084a3ffd576b Gerrit-Change-Number: 11275 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:42:57 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11260 ) Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/11260/5/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/11260/5/src/kudu/tools/kudu-tool-test.cc@365 PS5, Line 365: *found = false; Nit: we try to stick to the calling convention that if a function returns non-OK, we don't modify its OUT parameters. http://gerrit.cloudera.org:8080/#/c/11260/5/src/kudu/tools/kudu-tool-test.cc@877 PS5, Line 877: bool found_backup = false; No need to initialize this or on L889; HasAtLeastOneBackupFile guarantees that it is set if returning OK. -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:38:21 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11259 ) Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11259/4/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/11259/4/src/kudu/fs/fs_manager.cc@633 PS4, Line 633: ObjectIdGenerator oid_generator; Creating a new one of these for every call seems wasteful when we can have thousands of tablets. Can you store one in the FsManager itself? -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:36:30 + Gerrit-HasComments: Yes
[kudu-CR] [hms] update the HIVE/HADOOP package scripts
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11274 Change subject: [hms] update the HIVE/HADOOP package scripts .. [hms] update the HIVE/HADOOP package scripts This patch updates the HIVE/HADOOP package scripts to be more flexible, so that users can chose whether do jar downloading or repackaing. It also adjusts the jars that need to be stripped for universal HIVE and HADOOP distributions. Change-Id: Ie66c4e132800fbf5ea76ad4b1fec8212757c84c2 --- M thirdparty/package-hadoop.sh M thirdparty/package-hive.sh 2 files changed, 113 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/11274/1 -- To view, visit http://gerrit.cloudera.org:8080/11274 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie66c4e132800fbf5ea76ad4b1fec8212757c84c2 Gerrit-Change-Number: 11274 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] [hms] Add Hcatalog jars to the HMS classpath
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11273 Change subject: [hms] Add Hcatalog jars to the HMS classpath .. [hms] Add Hcatalog jars to the HMS classpath Due to the packaging differences of Hive distributions, the Hcatalog jars may not be loaded into the classpath of mini HMS, which will cause the ClassNotFoundException when using notification log event lisenter. This patch explicitly added these jars. Change-Id: Ifed85fffdabb5af19f22e343843e012b009888a0 --- M src/kudu/hms/mini_hms.cc 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/11273/1 -- To view, visit http://gerrit.cloudera.org:8080/11273 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifed85fffdabb5af19f22e343843e012b009888a0 Gerrit-Change-Number: 11273 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] [hms] use IPv4 for mini HMS
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11276 Change subject: [hms] use IPv4 for mini HMS .. [hms] use IPv4 for mini HMS This patch explictly uses IPv4 protocal in the JVM environment options that pass to the mini HMS, to avoid IPv6 being accidentally used, which is not support in Hadoop/Hive yet. Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 --- M src/kudu/hms/mini_hms.cc 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/11276/1 -- To view, visit http://gerrit.cloudera.org:8080/11276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0ceeccb45f3cdd13ccdc5ed77e0675ae5442f808 Gerrit-Change-Number: 11276 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] [thirdparty] allow different URL prefix
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11275 Change subject: [thirdparty] allow different URL prefix .. [thirdparty] allow different URL prefix This patch adds the option to allow different URL prefix when downloading third party dependencies. Change-Id: I004828785d9b0d484806d743d7a1084a3ffd576b --- M thirdparty/download-thirdparty.sh 1 file changed, 41 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/11275/1 -- To view, visit http://gerrit.cloudera.org:8080/11275 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I004828785d9b0d484806d743d7a1084a3ffd576b Gerrit-Change-Number: 11275 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] [test] Use valid tablet ids in tests
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11272 ) Change subject: [test] Use valid tablet ids in tests .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d774abf2cb2ce6f81d1f851f2cc1ddd12e1726 Gerrit-Change-Number: 11272 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:32:25 + Gerrit-HasComments: No
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/11260 )
Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
..
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11260/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11260/4/src/kudu/tools/kudu-tool-test.cc@863
PS4, Line 863: // Test 'edit' by setting up EDITOR to be a sed script which
performs a substitution.
> Nit: Make sure
Done
http://gerrit.cloudera.org:8080/#/c/11260/4/src/kudu/tools/kudu-tool-test.cc@866
PS4, Line 866: ASSERT_OK(DoEdit("exec sed -i -e s/Formatted/Edited/
\"$@\"\n", , nullptr,
> Nit: const auto&
Done
http://gerrit.cloudera.org:8080/#/c/11260/4/src/kudu/tools/kudu-tool-test.cc@877
PS4, Line 877: bool found_backup = false;
> You could decompose this into a short helper that handles L846-869 too. Som
Done
--
To view, visit http://gerrit.cloudera.org:8080/11260
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40
Gerrit-Change-Number: 11260
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 20 Aug 2018 19:31:01 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11260 to look at the new patch set (#5). Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool Adds a -nobackup flag to the pbc edit tool to simplify usage when the backup file will not be used and the user doesn’t want to worry about cleanup. Change-Id: If544f7682a30933077db0824452639a68507ba40 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc 2 files changed, 50 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/11260/5 -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [test] Use valid tablet ids in tests
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11272 Change subject: [test] Use valid tablet ids in tests .. [test] Use valid tablet ids in tests Some tests used tablet ids that were not valid Kudu object ids. This patch changes them to use a valid id in preperaption for a follow on patch which makes tablet id validation more strict. Change-Id: I83d774abf2cb2ce6f81d1f851f2cc1ddd12e1726 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tserver/tablet_server-test-base.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager-test.cc 4 files changed, 37 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/11272/1 -- To view, visit http://gerrit.cloudera.org:8080/11272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I83d774abf2cb2ce6f81d1f851f2cc1ddd12e1726 Gerrit-Change-Number: 11272 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] bitmap: add equality method
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11266 ) Change subject: bitmap: add equality method .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11266 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If53e41250435501e158bdbc076dc8e89ea153256 Gerrit-Change-Number: 11266 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 20 Aug 2018 19:26:31 + Gerrit-HasComments: No
[kudu-CR] bitmap: add equality method
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11266 ) Change subject: bitmap: add equality method .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11266/1/src/kudu/util/bitmap-test.cc File src/kudu/util/bitmap-test.cc: http://gerrit.cloudera.org:8080/#/c/11266/1/src/kudu/util/bitmap-test.cc@241 PS1, Line 241: SCOPED_TRACE(i); > Should this also test equality when bit values are 1? Done. -- To view, visit http://gerrit.cloudera.org:8080/11266 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If53e41250435501e158bdbc076dc8e89ea153256 Gerrit-Change-Number: 11266 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 20 Aug 2018 19:26:29 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2059 fix race in ThreadPool while updating metrics
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11271 ) Change subject: KUDU-2059 fix race in ThreadPool while updating metrics .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11271/1/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/11271/1/src/kudu/util/threadpool.cc@553 PS1, Line 553: guard.Unlock(); > The problem is not with re-entrancy of Increment(), but access to token->me Shutdown() does not destroy tokens; it destroys each token's queued (but not yet run) tasks. Only the token creator may destroy a token; that's why Threadpool::NewToken() returns a token wrapped in a unique_ptr. -- To view, visit http://gerrit.cloudera.org:8080/11271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I930c7e388001446e67b244a2b3e71d5afdcaf516 Gerrit-Change-Number: 11271 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:25:01 + Gerrit-HasComments: Yes
[kudu-CR] bitmap: add equality method
Hello Mike Percy, Kudu Jenkins, Grant Henke, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11266 to look at the new patch set (#2). Change subject: bitmap: add equality method .. bitmap: add equality method This is useful for tests. Change-Id: If53e41250435501e158bdbc076dc8e89ea153256 --- M src/kudu/util/bitmap-test.cc M src/kudu/util/bitmap.h 2 files changed, 55 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/11266/2 -- To view, visit http://gerrit.cloudera.org:8080/11266 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If53e41250435501e158bdbc076dc8e89ea153256 Gerrit-Change-Number: 11266 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2059 fix race in ThreadPool while updating metrics
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11271 ) Change subject: KUDU-2059 fix race in ThreadPool while updating metrics .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11271/1/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/11271/1/src/kudu/util/threadpool.cc@553 PS1, Line 553: guard.Unlock(); > How is this a race? I believe metrics_ is immutable once set, as are its me The problem is not with re-entrancy of Increment(), but access to token->metrics_ when ThreadPoolToken is being destructed. I thinks those tokens are destroyed by ThreadPool::Shutdown() You can find the recent stack at: https://issues.apache.org/jira/browse/KUDU-2059?focusedCommentId=16586333=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16586333 -- To view, visit http://gerrit.cloudera.org:8080/11271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I930c7e388001446e67b244a2b3e71d5afdcaf516 Gerrit-Change-Number: 11271 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:08:24 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2059 fix race in ThreadPool while updating metrics
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11271 ) Change subject: KUDU-2059 fix race in ThreadPool while updating metrics .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11271/1/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/11271/1/src/kudu/util/threadpool.cc@553 PS1, Line 553: guard.Unlock(); > How is this a race? I believe metrics_ is immutable once set, as are its me Oh, I missed that you're publishing this as a fix for KUDU-2059. I don't think this fixes that bug. The TSAN data race shows a KuduClient that's being destroyed alongside an outstanding task in DnsResolver (belonging to that client). The log shows a test fixture going out of scope, causing the last ref to a KuduClient to be dropped. This begins the teardown process for KuduClient, which will destroy its DnsResolver, which destroys its ThreadPool. Importantly, the KuduClient's Messenger is NOT destroyed, because its reactor threads each have an "internal" ref to it, and one of them is initializing a proxy, thus accessing the destroyed DnsResolver. So, the root cause is that we're destroying the KuduClient even though it has active threads that are accessing internal KuduClient state. KUDU-2439 talks about this problem in more detail. -- To view, visit http://gerrit.cloudera.org:8080/11271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I930c7e388001446e67b244a2b3e71d5afdcaf516 Gerrit-Change-Number: 11271 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 19:02:25 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2059 fix race in ThreadPool while updating metrics
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11271 ) Change subject: KUDU-2059 fix race in ThreadPool while updating metrics .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11271/1/src/kudu/util/threadpool.cc File src/kudu/util/threadpool.cc: http://gerrit.cloudera.org:8080/#/c/11271/1/src/kudu/util/threadpool.cc@553 PS1, Line 553: guard.Unlock(); How is this a race? I believe metrics_ is immutable once set, as are its members. And isn't Increment() reentrant? Do you have TSAN output showing the race? -- To view, visit http://gerrit.cloudera.org:8080/11271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I930c7e388001446e67b244a2b3e71d5afdcaf516 Gerrit-Change-Number: 11271 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 18:54:59 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11260 )
Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11260/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11260/4/src/kudu/tools/kudu-tool-test.cc@863
PS4, Line 863: // Make no backup file was written.
Nit: Make sure
http://gerrit.cloudera.org:8080/#/c/11260/4/src/kudu/tools/kudu-tool-test.cc@866
PS4, Line 866: for (const auto : children) {
Nit: const auto&
(Yeah that's what I wrote earlier, but I was trying to draw attention not just
to the lack of const, but also the placement of the ampersand next to the word
'auto').
http://gerrit.cloudera.org:8080/#/c/11260/4/src/kudu/tools/kudu-tool-test.cc@877
PS4, Line 877: // Make sure a backup file was written.
You could decompose this into a short helper that handles L846-869 too.
Something like:
Status HasAtLeastOneBackupFile(const string& dir, bool* found)
--
To view, visit http://gerrit.cloudera.org:8080/11260
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40
Gerrit-Change-Number: 11260
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 20 Aug 2018 18:53:47 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2059 fix race in ThreadPool while updating metrics
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11271 Change subject: KUDU-2059 fix race in ThreadPool while updating metrics .. KUDU-2059 fix race in ThreadPool while updating metrics Fixed race between ThreadPool::DoSubmit() and ThreadPool::~ThreadPool() while updating metrics. Change-Id: I930c7e388001446e67b244a2b3e71d5afdcaf516 --- M src/kudu/util/threadpool.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/11271/1 -- To view, visit http://gerrit.cloudera.org:8080/11271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I930c7e388001446e67b244a2b3e71d5afdcaf516 Gerrit-Change-Number: 11271 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-1291: efficiently support predicates on non-prefix key components
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/10983 )
Change subject: KUDU-1291: efficiently support predicates on non-prefix key
components
..
Patch Set 20: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10983/20/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:
http://gerrit.cloudera.org:8080/#/c/10983/20/src/kudu/cfile/cfile_reader.cc@777
PS20, Line 777: if (PREDICT_TRUE(validx_iter_ != nullptr)) {
: RETURN_NOT_OK(StoreCurrentValue());
: }
To make sure this patch has no side effects when disabled, this should probably
be gated by some `cache_seeked_value`. AFAICT, besides the skip scan logic,
this method is only used in tool_action_fs.cc anyway. WDYT?
--
To view, visit http://gerrit.cloudera.org:8080/10983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f
Gerrit-Change-Number: 10983
Gerrit-PatchSet: 20
Gerrit-Owner: Anupama Gupta
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Anupama Gupta
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 20 Aug 2018 18:24:35 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1291: efficiently support predicates on non-prefix key components
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/10983 ) Change subject: KUDU-1291: efficiently support predicates on non-prefix key components .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/10983 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f Gerrit-Change-Number: 10983 Gerrit-PatchSet: 20 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1291: efficiently support predicates on non-prefix key components
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/10983 )
Change subject: KUDU-1291: efficiently support predicates on non-prefix key
components
..
Patch Set 20:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10983/19//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10983/19//COMMIT_MSG@19
PS19, Line 19: until
> nit: only one 'l'
Done
http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:
http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/cfile/cfile_reader.cc@794
PS19, Line 794: 8192
> nit: should probably be using FLAGS_max_encoded_key_size_bytes here
FLAGS_max_encoded_key_size_bytes is not available here and I'm not sure I want
to move the definition of that flag in here or re-shuffle libraries only to
remove it in the follow-up patch: as we discussed offline, it's better to use
arena passed as a for these methods from upper level (i.e. from cfile_set).
http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/cfile_set.h
File src/kudu/tablet/cfile_set.h:
http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/cfile_set.h@303
PS19, Line 303: & cur_enc_key,
: KuduPartialRow* p_row,
> nit: reverse sigil-space order
Done
http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/cfile_set.h@354
PS19, Line 354: il
> nit: here too
Done
http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/index_skipscan-test.cc
File src/kudu/tablet/index_skipscan-test.cc:
http://gerrit.cloudera.org:8080/#/c/10983/19/src/kudu/tablet/index_skipscan-test.cc@168
PS19, Line 168:
: Status s = writer.Insert(row);
: // As keys are inserted randomly, retry row insertion in
case
: // the current row insertion failed due to duplicate
value.
: if (s.IsAlreadyPresent()) {
: continue;
: } else {
: CHECK_OK(s);
> Does it matter what these values are? It seems like the predicate column wi
No, it's not that important. Yes, there will be only three different values
and that's what was desired, yep. The random approach was exercised by the
other generator, but if you like randomness here as well, I'll add that. The
only important point here is to have not less than num_rows/3 values matching
the predicate.
--
To view, visit http://gerrit.cloudera.org:8080/10983
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f
Gerrit-Change-Number: 10983
Gerrit-PatchSet: 20
Gerrit-Owner: Anupama Gupta
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Anupama Gupta
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 20 Aug 2018 17:50:01 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1291: efficiently support predicates on non-prefix key components
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10983 ) Change subject: KUDU-1291: efficiently support predicates on non-prefix key components .. Patch Set 20: Verified+1 Unrelated failure (due to concurrent builds?): 08:44:42 CMakeFiles/file_cache-stress-test.dir/file_cache-stress-test.cc.o: file not recognized: File truncated 08:44:42 collect2: error: ld returned 1 exit status 08:44:42 make[2]: *** [bin/file_cache-stress-test] Error 1 -- To view, visit http://gerrit.cloudera.org:8080/10983 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f Gerrit-Change-Number: 10983 Gerrit-PatchSet: 20 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 20 Aug 2018 17:50:24 + Gerrit-HasComments: No
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11259 ) Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. Patch Set 3: It turns out we use non canonical tablet ids all over our tests. Things like "test-tablet". So changing FsManager::IsValidTabletId to actually validate tablet ids breaks a lot. -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 20 Aug 2018 17:15:05 + Gerrit-HasComments: No
[kudu-CR] bitmap: add equality method
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/11266 )
Change subject: bitmap: add equality method
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11266/1/src/kudu/util/bitmap-test.cc
File src/kudu/util/bitmap-test.cc:
http://gerrit.cloudera.org:8080/#/c/11266/1/src/kudu/util/bitmap-test.cc@241
PS1, Line 241: for (int i = num_bits - 1; i >= 0; i--) {
Should this also test equality when bit values are 1?
--
To view, visit http://gerrit.cloudera.org:8080/11266
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If53e41250435501e158bdbc076dc8e89ea153256
Gerrit-Change-Number: 11266
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 20 Aug 2018 16:49:34 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] KUDU-2245 Graceful leadership transfer
Hello Fengling Wang, Tidy Bot, Mike Percy, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11251 to look at the new patch set (#2). Change subject: [WIP] KUDU-2245 Graceful leadership transfer .. [WIP] KUDU-2245 Graceful leadership transfer This patch implements graceful leadership transfer, as described in the original Raft thesis. It has the following steps: 1. An admin client sends a request to the tablet leader for it to transfer leadership. The client can indicate a specific voter that it wants to become the leader, or it can allow the current leader to choose its successor. 2. The leader receives the request and beings a leader transfer period. During a leader transfer period, the leader does not accept writes or config change requests. This allows followers to catch up to the leader. A background timer expires the transfer period after one election timeout, since clients should be able to ride over interruptions in service lasting at least that long. 3. During the transfer period, the leader continues to update peers. When it receives a response from a peer, it checks if that peer is a voter and fully caught up to the leader's log. If it is, and if it is the designated successor if one was provided, the leader signals the peer to start an election, which it should win. If no eligible successor appears, the transfer period expires and the leader resumes normal operation. This is an improvement over the current leader step down method, which causes the leader to simply relinquish leadership and snooze its election timer for an extra long period, so another voter will likely become leader. Leadership transfer should usually be much faster and it allows the client to select the new leader among current voters. However, note that it does not provide strictly better guarantees- it is still possible that leadership will not be transferred. I ran TestRepeatLeaderStepDown and TestGracefulLeaderStepDown 1000 times and 200 times each, in debug and TSAN modes, with 4 stress threads, and saw no failures. Still WIP because I want to * Run some dist-test loops of the rebalancer tests, which now use graceful leadership transfer. * Add a test or two for bad cases where the leadership transfer period should expire. * Quantify how much faster leadership transfer is than abrupt stepdown, at least in a lab environment. Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083 --- M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/consensus.proto M src/kudu/consensus/consensus_peers.cc M src/kudu/consensus/consensus_peers.h M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/peer_manager.cc M src/kudu/consensus/peer_manager.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h M src/kudu/tserver/tablet_service.cc 16 files changed, 530 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/11251/2 -- To view, visit http://gerrit.cloudera.org:8080/11251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic97343af9eb349556424c999799ed5e2941f0083 Gerrit-Change-Number: 11251 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/11259 )
Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/fs/fs_manager.cc@625
PS1, Line 625: // Return true if 'fname' is a valid tablet ID.
> yeah, I agree it would be a better approach
I can update this method to use ObjectIdGenerator::Canonicalize to ensure the
ids are valid. Currently we have only filtered theses special file types. This
will technically be more strict. Is there a risk of breaking anything?
http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/fs/fs_manager.cc@625
PS1, Line 625: // Return true if 'fname' is a valid tablet ID.
> Any idea why we don't implement this by verifying that the filename is a 32
I am not sure why. It was added here:
https://github.com/apache/kudu/commit/dc20efa0b66c798f73aafdc694a3566317366fd6
I think the implementation is assuming everything but these exceptions must be
valid. I agree, it would be better to filter on the real pattern.
http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:
http://gerrit.cloudera.org:8080/#/c/11259/1/src/kudu/util/env_util.cc@312
PS1, Line 312: iter->find(kTmpInfix) != string::npos ||
: iter->find(kBakInfix) != string::npos) {
> There are no tests today that will make a backup file. As far as temp files
I removed this change from the patch.
--
To view, visit http://gerrit.cloudera.org:8080/11259
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1
Gerrit-Change-Number: 11259
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 20 Aug 2018 16:13:00 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/11260 )
Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
..
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/11260/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11260/1/src/kudu/tools/kudu-tool-test.cc@845
PS1, Line 845: setenv("EDITOR", editor_path.c_str(), /* overwrite */1);
> What about the other half (i.e. that --backup DOES produce a backup file?)
Done
http://gerrit.cloudera.org:8080/#/c/11260/3/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/11260/3/src/kudu/tools/kudu-tool-test.cc@863
PS3, Line 863: ke no backu
> Nit: const auto&
Done
http://gerrit.cloudera.org:8080/#/c/11260/3/src/kudu/tools/kudu-tool-test.cc@864
PS3, Line 864:
> nit: kBakInfix
In this case I actually want the test to fail if we change that given it's a
sort of "public api".
http://gerrit.cloudera.org:8080/#/c/11260/3/src/kudu/tools/tool_action_pbc.cc
File src/kudu/tools/tool_action_pbc.cc:
http://gerrit.cloudera.org:8080/#/c/11260/3/src/kudu/tools/tool_action_pbc.cc@214
PS3, Line 214: // We successfully wrote the new file.
> nit: I think this comment should be split into two and the backup part move
Done
--
To view, visit http://gerrit.cloudera.org:8080/11260
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40
Gerrit-Change-Number: 11260
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 20 Aug 2018 16:13:09 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2531: (part 1) Ignore invalid tablet metadata files
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11259 to look at the new patch set (#3). Change subject: KUDU-2531: (part 1) Ignore invalid tablet metadata files .. KUDU-2531: (part 1) Ignore invalid tablet metadata files Changes the behavior of FsManager::IsValidTabletId to return false for all files that are named with invalid tabet ids. This results in all files that are not valid table ids being ignored, instead of just tmp files. Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 --- M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc 2 files changed, 27 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/11259/3 -- To view, visit http://gerrit.cloudera.org:8080/11259 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9f0e6dd8925519aa3ed3e735f2987743f2ef1d1 Gerrit-Change-Number: 11259 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11260 to look at the new patch set (#4). Change subject: KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool .. KUDU-2531: (part 2) Add -nobackup flag to pbc edit tool Adds a -nobackup flag to the pbc edit tool to simplify usage when the backup file will not be used and the user doesn’t want to worry about cleanup. Change-Id: If544f7682a30933077db0824452639a68507ba40 --- M src/kudu/tools/kudu-tool-test.cc M src/kudu/tools/tool_action_pbc.cc 2 files changed, 46 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/11260/4 -- To view, visit http://gerrit.cloudera.org:8080/11260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If544f7682a30933077db0824452639a68507ba40 Gerrit-Change-Number: 11260 Gerrit-PatchSet: 4 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1291: efficiently support predicates on non-prefix key components
Alexey Serbin has uploaded a new patch set (#20) to the change originally created by Anupama Gupta. ( http://gerrit.cloudera.org:8080/10983 ) Change subject: KUDU-1291: efficiently support predicates on non-prefix key components .. KUDU-1291: efficiently support predicates on non-prefix key components This patch implements index skip scan approach in the case when there exists an equality predicate on any of the non-first primary key columns. This feature uses the following approach: 1. Seek at the first unique prefix key in the ad-hoc index tree. 2. Seek to the relevant entry corresponding to the unique prefix key found in 1. and the predicate value on the suffix key column. 3. To handle the case where multiple entries exist with respect to a unique prefix key, store the upper bound index of these entries. 4. Repeat steps 1-3, until all the unique prefix keys are scanned. Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f --- M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/cfile/cfile_reader.h M src/kudu/common/encoded_key.cc M src/kudu/common/encoded_key.h M src/kudu/common/partial_row.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/cfile_set.cc M src/kudu/tablet/cfile_set.h A src/kudu/tablet/index_skipscan-test.cc 10 files changed, 1,327 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/10983/20 -- To view, visit http://gerrit.cloudera.org:8080/10983 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I230cd5a288e28ace796b352a603e0d1bcc1e4e0f Gerrit-Change-Number: 10983 Gerrit-PatchSet: 20 Gerrit-Owner: Anupama Gupta Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anupama Gupta Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
