[kudu-CR] [fs]: delete the (orphaned) metadata file when repairing
Hello Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12239 to look at the new patch set (#4). Change subject: [fs]: delete the (orphaned) metadata file when repairing .. [fs]: delete the (orphaned) metadata file when repairing This patch intends to delete the (orphaned) metadata file at startup while the data file has gone missing and the metadata file has no live blocks at the same time. In the previous patch KUDU-2636, it has supported deleting the dead container which includes data file and metadata file. So, there will be a time window while deleting both of these two files in sequential, and it will make a half-container in extreme cases, especially in a crash or sent the tserver a kill -9. Indeed, Adar has captured this type of case under 1000 runs of dense_node-itest. Change-Id: I2a4223d0e6385a7f7776f8fe2679a3bb16d832ab --- M src/kudu/fs/log_block_manager-test.cc M src/kudu/fs/log_block_manager.cc 2 files changed, 351 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/12239/4 -- To view, visit http://gerrit.cloudera.org:8080/12239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2a4223d0e6385a7f7776f8fe2679a3bb16d832ab Gerrit-Change-Number: 12239 Gerrit-PatchSet: 4 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2543 pt 2: pass around default authz tokens
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12122 ) Change subject: KUDU-2543 pt 2: pass around default authz tokens .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7971d652d6adc822167cf959bffd5f994a7ca565 Gerrit-Change-Number: 12122 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Jan 2019 07:32:52 + Gerrit-HasComments: No
[kudu-CR] KUDU-2543 pt 1: basic checks for authz tokens
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11751 ) Change subject: KUDU-2543 pt 1: basic checks for authz tokens .. Patch Set 11: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11751 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I99555e0ab2d09d4abcbc12b1100658a9a17590f4 Gerrit-Change-Number: 11751 Gerrit-PatchSet: 11 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Jan 2019 07:31:45 + Gerrit-HasComments: No
[kudu-CR] De-flake sentry tests
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11961 ) Change subject: De-flake sentry tests .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id112634ad5e6f0e2c48b7e0e89389faff4f5656b Gerrit-Change-Number: 11961 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Jan 2019 07:29:47 + Gerrit-HasComments: No
[kudu-CR] [sentry] add mini Sentry to the external mini cluster
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11956 ) Change subject: [sentry] add mini Sentry to the external mini cluster .. Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/mini-cluster/external_mini_cluster.cc@257 PS2, Line 257: string ktpath; > Sorry that I fail to see why. With the current approach, for macOS env we r You're right; not sure how I thought this would help macOS. It would add a bit of clarity to the UNIQUE_LOOPBACK case (in that there would be more symmetry between HMS and Sentry), but that's about it. Maybe note it in a TODO here instead, citing the upstream bug? http://gerrit.cloudera.org:8080/#/c/11956/11/src/kudu/sentry/mini_sentry.cc File src/kudu/sentry/mini_sentry.cc: http://gerrit.cloudera.org:8080/#/c/11956/11/src/kudu/sentry/mini_sentry.cc@144 PS11, Line 144: // Check that the port number hasn't changed if it has been set (not zero). Nit: this is a bit unclear. Reword as "Check that the port number only changed if the original port was 0 (i.e. if we asked to bind to an ephemeral port)". Maybe also rename 'port' to 'orig_port' to help clarify? -- To view, visit http://gerrit.cloudera.org:8080/11956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 Gerrit-Change-Number: 11956 Gerrit-PatchSet: 11 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Jan 2019 07:28:48 + Gerrit-HasComments: Yes
[kudu-CR] python: fix Python 3.4 based tests
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12255 ) Change subject: python: fix Python 3.4 based tests .. Patch Set 2: Code-Review+2 (2 comments) Addressed my own feedback in PS2. http://gerrit.cloudera.org:8080/#/c/12255/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12255/1//COMMIT_MSG@9 PS1, Line 9: Pip released 19.0 recently and tests based on Python 3.4 started to fail. In : addition, Python > Would be good to include the error message in fail, to help searching in th Done http://gerrit.cloudera.org:8080/#/c/12255/1/build-support/jenkins/build-and-test.sh File build-support/jenkins/build-and-test.sh: http://gerrit.cloudera.org:8080/#/c/12255/1/build-support/jenkins/build-and-test.sh@518 PS1, Line 518: # : # pip 19.1 doesn't support Python 3.4, which is the version of Python 3 : # shipped with Ubuntu 14.04. However, the > I filed an upstream bug with pip: https://github.com/pypa/pip/issues/6175. Done -- To view, visit http://gerrit.cloudera.org:8080/12255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecbc55877e832a0ef437d2799e09c39fb314cb95 Gerrit-Change-Number: 12255 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Jan 2019 07:13:43 + Gerrit-HasComments: Yes
[kudu-CR] python: fix Python 3.4 based tests
Adar Dembo has uploaded a new patch set (#2) to the change originally created by Hao Hao. ( http://gerrit.cloudera.org:8080/12255 ) Change subject: python: fix Python 3.4 based tests .. python: fix Python 3.4 based tests Pip released 19.0 recently and tests based on Python 3.4 started to fail. In addition, Python 3.4 will no longer be supported beginning with pip 19.1. Therefore, this patch updates the pip version to the last one that works. The full error message is below. $ pip install pandas DEPRECATION: Python 3.4 support has been deprecated. pip 19.1 will be the last one supporting it. Please upgrade your Python as Python 3.4 won't be maintained after March 2019 (cf PEP 429). Collecting pandas Using cached https://files.pythonhosted.org/packages/08/01/803834bc8a4e708aedebb133095a88a4dad9f45bbaf5ad777d2bea543c7e/pandas-0.22.0.tar.gz Installing build dependencies ... done Getting requirements to build wheel ... error Complete output from command /home/jenkins-slave/a/bin/python3 /home/jenkins-slave/a/lib/python3.4/site-packages/pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmpsknqc6o3: Traceback (most recent call last): File "/home/jenkins-slave/a/lib/python3.4/site-packages/pip/_vendor/pep517/_in_process.py", line 207, in main() File "/home/jenkins-slave/a/lib/python3.4/site-packages/pip/_vendor/pep517/_in_process.py", line 197, in main json_out['return_val'] = hook(**hook_input['kwargs']) File "/home/jenkins-slave/a/lib/python3.4/site-packages/pip/_vendor/pep517/_in_process.py", line 54, in get_requires_for_build_wheel return hook(config_settings) File "/tmp/pip-build-env-z5ha9gs9/overlay/lib/python3.4/site-packages/setuptools/build_meta.py", line 115, in get_requires_for_build_wheel return _get_build_requires(config_settings, requirements=['wheel']) File "/tmp/pip-build-env-z5ha9gs9/overlay/lib/python3.4/site-packages/setuptools/build_meta.py", line 101, in _get_build_requires _run_setup() File "/tmp/pip-build-env-z5ha9gs9/overlay/lib/python3.4/site-packages/setuptools/build_meta.py", line 85, in _run_setup exec(compile(code, __file__, 'exec'), locals()) File "setup.py", line 27, in import versioneer ImportError: No module named 'versioneer' Change-Id: Iecbc55877e832a0ef437d2799e09c39fb314cb95 --- M build-support/jenkins/build-and-test.sh 1 file changed, 8 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/12255/2 -- To view, visit http://gerrit.cloudera.org:8080/12255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iecbc55877e832a0ef437d2799e09c39fb314cb95 Gerrit-Change-Number: 12255 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2665: deflake block manager-stress-test
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. Patch Set 1: Code-Review+2 Given that block_manager-stress-test is failing so much according to the flaky test dashboard, I'm going to treat He Lifu's +1 as a +2 and merge this. -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Jan 2019 07:00:34 + Gerrit-HasComments: No
[kudu-CR] KUDU-2665: deflake block manager-stress-test
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. KUDU-2665: deflake block_manager-stress-test After commit 0c501979b was merged, this test became really flaky (like 50% flaky in some environments). I think it's due to the new nature of the log block manager, which may now delete dead containers in the background. Specifically, if two transactions delete the last blocks from a full container, it's possible for one to get scheduled for an (asynchronous) hole punch, and for the other to set the container as dead. Later, when the hole punch runs, the container's last ref will be dropped, causing the dead container to be deleted. While perhaps surprising, this new behavior is desirable, and it's now incorrect to assume that a cessation in user threads implies an end to LBM activity. block_manager-stress-test makes this assumption by using the LBMCorruptor to inject inconsistencies after test threads have been joined. To fix, we must explicitly quiesce the LBM; destroying it will do the trick. What is surprising is that, for the life of me, I can't reproduce the failure. Not locally, not on a CentOS 6.6 machine, not looped in dist-test with stress threads, not ever. I even tried adding some "creative" sleep calls in a few places to tickle the race, to no avail. Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Reviewed-on: http://gerrit.cloudera.org:8080/12254 Reviewed-by: helifu Tested-by: Adar Dembo Reviewed-by: Adar Dembo --- M src/kudu/fs/block_manager-stress-test.cc 1 file changed, 5 insertions(+), 0 deletions(-) Approvals: helifu: Looks good to me, but someone else must approve Adar Dembo: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2665: deflake block manager-stress-test
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. Patch Set 1: Verified+1 Overriding Jenkins; the Python3 build breakages are unrelated. -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Jan 2019 06:57:57 + Gerrit-HasComments: No
[kudu-CR] KUDU-2665: deflake block manager-stress-test
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. Patch Set 1: > Reread the ThreadPool code, it seems that 'dd_manager_->WaitOnClosures()' in > the destructor of LBM ensures that all tasks can be completed. Exactly; that's why destroying the LBM has the effect of waiting for any outstanding dead container deletion to finish. -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Jan 2019 06:57:37 + Gerrit-HasComments: No
[kudu-CR] KUDU-2665: deflake block manager-stress-test
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] python: fix Python 3.4 based tests
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12255 ) Change subject: python: fix Python 3.4 based tests .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/12255/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12255/1//COMMIT_MSG@9 PS1, Line 9: Pip released 19.0 recently and tests based on Python 3.4 started to fail : with ImportError Would be good to include the error message in fail, to help searching in the future. http://gerrit.cloudera.org:8080/#/c/12255/1/build-support/jenkins/build-and-test.sh File build-support/jenkins/build-and-test.sh: http://gerrit.cloudera.org:8080/#/c/12255/1/build-support/jenkins/build-and-test.sh@518 PS1, Line 518: # Even though Python 3.4 is no longer supported starting from pip 19.1, attempting : # to upgrade to pip 19.0 on Python 3.4 with pandas 0.22 yields errors. Therefore, : # pin to the last pip version that works. I filed an upstream bug with pip: https://github.com/pypa/pip/issues/6175. Could you reword to link to that bug report, so that we'll have more information? Something like: # new version of pip to proceed. # # pip 19.1 doesn't support Python 3.4, which is the version of Python 3 # shipped with Ubuntu 14.04. However, there appears to be a bug[1] in pip 19.0 # preventing it from working properly with Python 3.4 as well. Therefore we # must pin to a pip version from before 19.0. # # 1. https://github.com/pypa/pip/issues/6175 -- To view, visit http://gerrit.cloudera.org:8080/12255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecbc55877e832a0ef437d2799e09c39fb314cb95 Gerrit-Change-Number: 12255 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Jan 2019 06:53:41 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2665: deflake block manager-stress-test
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. Patch Set 1: Code-Review+1 Reread the ThreadPool code, it seems that 'dd_manager_->WaitOnClosures()' in the destructor of LBM ensures that all tasks can be completed. -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Jan 2019 06:39:32 + Gerrit-HasComments: No
[kudu-CR] [docker] Add an initial docker build
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 ) Change subject: [docker] Add an initial docker build .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh File thirdparty/build-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh@248 PS2, Line 248: # Install Hadoop, Hive, and Sentry by copying their source directories into $PREFIX/opt. : # Copying allows the source directory to be removed once thirdparty is built. > This will add ~400MB of space. But it allows me to save almost 2 GiB in the Hmm, that's a fair amount of space relative to the size of the thirdparty install tree right now: counting both common and uninstrumented, we're sitting at 1.8 GB. Not to mention it appears to have broken dist-test in some way. Can we keep the symlinks but special-case the docker build to remove them and copy the files in? -- To view, visit http://gerrit.cloudera.org:8080/12173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42 Gerrit-Change-Number: 12173 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Jan 2019 05:34:09 + Gerrit-HasComments: Yes
[kudu-CR] [spark] Add some logging to trace KuduContext operations
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12252 ) Change subject: [spark] Add some logging to trace KuduContext operations .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12252/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala: http://gerrit.cloudera.org:8080/#/c/12252/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@246 PS1, Line 246: log.info Maybe, move this into writeRows() itself? It seems there are enough parameters for writeRows() to differentiate among various ops and tables. -- To view, visit http://gerrit.cloudera.org:8080/12252 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6741f2584c1bc6b229d10d37297515474318f94c Gerrit-Change-Number: 12252 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Jan 2019 05:12:17 + Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 ) Change subject: [docker] Add an initial docker build .. Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/12173/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12173/2//COMMIT_MSG@20 PS2, Line 20: muti > multi Done http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile File docker/Dockerfile: http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@71 PS2, Line 71: # TODO: Remove build dir too? > In theory you could remove thirdparty/build too, except that the clang-tool Right, that's the issue I ran into and I didn't try to fix it yet. Being able to remove this directory would make the docker image ~2.5 GiB smaller. Here is the code comment that explains why it's there: > # Create a link from Clang to thirdparty/clang-toolchain. This path is used # for all Clang invocations. The link can't point to the Clang installed in # the prefix directory, since this confuses CMake into believing the # thirdparty prefix directory is the system-wide prefix, and it omits the # thirdparty prefix directory from the rpath of built binaries. http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@110 PS2, Line 110: COPY ./docs/support docs/support This is actually used in the standard build, without it the build below fails. The first failure is: > CMake Error: File /kudu/docs/support/doxygen/client_api.doxy.in does not > exist. > CMake Error at CMakeLists.txt:1418 (configure_file): configure_file Problem configuring file It looks like if doxygen is installed it is required, otherwise it's ignored. I installed doxygen in the base image so it could be used to build docs if needed. http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile-files File docker/Dockerfile-files: http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile-files@18 PS2, Line 18: # This docker file is a minimal build usefull for debugging > useful Done http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh File docker/kudu-entrypoint.sh: http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh@49 PS2, Line 49: KUDU_OPTS="$KUDU_FLAGS \ > Is the idea that KUDU_FLAGS can be set by users to do more configuration? M Done http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh@52 PS2, Line 52: --fs_data_dirs=/var/lib/kudu/master \ > You can omit this if the intent is to use the same value as --fs_wal_dir. B Done http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh File thirdparty/build-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh@248 PS2, Line 248: # Install Hadoop, Hive, and Sentry by copying their source directories into $PREFIX/opt. : # Copying allows the source directory to be removed once thirdparty is built. > How much additional disk space will this consume? This will add ~400MB of space. But it allows me to save almost 2 GiB in the image by removing the source after the build. This change does break the build, I will add a call to ensure the existing directory is wiped before installing. -- To view, visit http://gerrit.cloudera.org:8080/12173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42 Gerrit-Change-Number: 12173 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Jan 2019 03:51:27 + Gerrit-HasComments: Yes
[kudu-CR] [docker] Add an initial docker build
Hello Attila Bukor, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12173 to look at the new patch set (#3). Change subject: [docker] Add an initial docker build .. [docker] Add an initial docker build This patch adds an initial Docker file which defines a multistage build that supports creating various Docker images for Apache Kudu development. This work is experimental and the choices and structure may change wih follow on commits. Some things these images could be used for include: - demos/examples - pre-built binaries including thirdparty - multi-os testing - multi-os tools like build_mini_cluster_binaries.sh - external integration testing Some of the open tasks for the future are: - Handle all build types. - Optimize image sizes. - Add kudu user to runtime images. - Add an upload script with good tagging. - Add healthchecks. Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42 --- A .dockerignore A docker/Dockerfile A docker/Dockerfile-files A docker/README.adoc A docker/bootstrap-env.sh A docker/docker-build.sh A docker/kudu-entrypoint.sh M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh 9 files changed, 786 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/12173/3 -- To view, visit http://gerrit.cloudera.org:8080/12173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42 Gerrit-Change-Number: 12173 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2665: deflake block manager-stress-test
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. Patch Set 1: my preference 2) is not safe because we can't be sure when the threads in 'dd_manager_' will be over. ~~><~~ -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Jan 2019 03:33:28 + Gerrit-HasComments: No
[kudu-CR] KUDU-2665: deflake block manager-stress-test
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12254/1/src/kudu/fs/block_manager-stress-test.cc File src/kudu/fs/block_manager-stress-test.cc: http://gerrit.cloudera.org:8080/#/c/12254/1/src/kudu/fs/block_manager-stress-test.cc@550 PS1, Line 550: // Quiesce the block manager before injecting inconsistencies so that the two : // don't interfere with one another. : this->bm_.reset(); > I have just read through the code of 'block_manager-stress-test'. I agree w BTW, i prefer 2), and let 'dd_manager_' continue to run. -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Jan 2019 03:20:32 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2665: deflake block manager-stress-test
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12254 ) Change subject: KUDU-2665: deflake block_manager-stress-test .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12254/1/src/kudu/fs/block_manager-stress-test.cc File src/kudu/fs/block_manager-stress-test.cc: http://gerrit.cloudera.org:8080/#/c/12254/1/src/kudu/fs/block_manager-stress-test.cc@550 PS1, Line 550: // Quiesce the block manager before injecting inconsistencies so that the two : // don't interfere with one another. : this->bm_.reset(); I have just read through the code of 'block_manager-stress-test'. I agree with adar's analysis that it's now incorrect to assume that a cessation in user threads implies an end to LBM activity. Please take a look at the code of log_block_manager-test-util.cc:Line70/Line95, it is not safe while the threads in 'dd_manager_' are deleting dead container via punch hole and drop container's last ref. And i am sure Percy's issue is from Line95. Maybe we can have two ways: 1)destroy the 'dd_manager_' before injecting inconsistencies, 2)check data file existence at Line95. Sorry for leaving this issue:( -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 23 Jan 2019 03:11:47 + Gerrit-HasComments: Yes
[kudu-CR] python: fix Python 3.4 based tests
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12255 Change subject: python: fix Python 3.4 based tests .. python: fix Python 3.4 based tests Pip released 19.0 recently and tests based on Python 3.4 started to fail with ImportError. In addition, Python 3.4 will no longer supported starting from pip 19.1. Therefore, this patch updates the pip version to the last one that works. Change-Id: Iecbc55877e832a0ef437d2799e09c39fb314cb95 --- M build-support/jenkins/build-and-test.sh 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/12255/1 -- To view, visit http://gerrit.cloudera.org:8080/12255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iecbc55877e832a0ef437d2799e09c39fb314cb95 Gerrit-Change-Number: 12255 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] [spark] Add some logging to trace KuduContext operations
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12252 ) Change subject: [spark] Add some logging to trace KuduContext operations .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12252 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6741f2584c1bc6b229d10d37297515474318f94c Gerrit-Change-Number: 12252 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 23 Jan 2019 02:37:31 + Gerrit-HasComments: No
[kudu-CR] [spark] Add metrics to kudu-spark
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12251 ) Change subject: [spark] Add metrics to kudu-spark .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12251/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala: http://gerrit.cloudera.org:8080/#/c/12251/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala@156 PS1, Line 156: rowsRead.add(currentIterator.getNumRows) This is a bit tricky. The rows have been read from Kudu but not necessarily read/returned to the spark application via the get call below. The best/right choice of incrementing rowsRead here vs the get call isn't clear to me. Just out of interest, any reason you picked doing it here? -- To view, visit http://gerrit.cloudera.org:8080/12251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied81c890b3d4510f767d8f023963ff878f398140 Gerrit-Change-Number: 12251 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 23 Jan 2019 02:35:03 + Gerrit-HasComments: Yes
[kudu-CR] De-flake sentry tests
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11961 to look at the new patch set (#6). Change subject: De-flake sentry tests .. De-flake sentry tests Currently, sentry_client-test and sentry_authz_provider-test are still flaky due to possible port collision. To avoid such race condition, this patch moves the util function that finds the bind address for a daemon based on binding mode to test_util, and updates sentry-test-base to use it to pick up a unique bind address. Change-Id: Id112634ad5e6f0e2c48b7e0e89389faff4f5656b --- M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/integration-tests/security-itest.cc M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/mini-cluster/internal_mini_cluster.cc M src/kudu/mini-cluster/internal_mini_cluster.h M src/kudu/mini-cluster/mini_cluster.cc M src/kudu/mini-cluster/mini_cluster.h M src/kudu/sentry/sentry-test-base.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h 13 files changed, 168 insertions(+), 142 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/11961/6 -- To view, visit http://gerrit.cloudera.org:8080/11961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id112634ad5e6f0e2c48b7e0e89389faff4f5656b Gerrit-Change-Number: 11961 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2543 pt 2: pass around default authz tokens
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12122 ) Change subject: KUDU-2543 pt 2: pass around default authz tokens .. Patch Set 6: (13 comments) http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/authz_token_cache.h File src/kudu/client/authz_token_cache.h: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/authz_token_cache.h@134 PS5, Line 134: authz_rpcs_; > nit: why not name it 'authz_rpcs_' to match 'authz_tokens_'? Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/client-internal.cc File src/kudu/client/client-internal.cc: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/client-internal.cc@466 PS5, Line 466: Parse the server part > nit: use StoreAuthzToken() instead? Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/client/client-test.cc@5853 PS5, Line 5853: client_->data_->RetrieveAuthzTokenAsync(client_table_.get(), s.AsStatusCallback(), > Is it possible to change the logic to create a new client and only retrieve That's possible, we could also not run RetrieveAuthzTokenAsync if there is a token in the cache. I'm going to hold off on it though since I don't think that adds to the test coverage and would make this test more complicated. http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/auth_token_expire-itest.cc File src/kudu/integration-tests/auth_token_expire-itest.cc: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/auth_token_expire-itest.cc@143 PS5, Line 143: i > nit: extra space. Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/auth_token_expire-itest.cc@254 PS5, Line 254: p > nit: space. Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc File src/kudu/integration-tests/authz_token-itest.cc: http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@184 PS5, Line 184: Insert > nit: 'Inserts' to aligned with other comments style. Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@205 PS5, Line 205: Get > nit: 'Gets' Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@281 PS5, Line 281: > Can you add a comment to explain what is the expected behavior in this case Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@352 PS5, Line 352: auto& e : er > Nit: got two spaces here. Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@362 PS5, Line 362: > Why not parameterized the functor here as well? Done http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@369 PS5, Line 369: yPB tsk; > Should a similar test be exist for authn token as well? A similar test exists for authn tokens in security-unknown-tsk-itest. I chose to put this test here because the logic differed enough that parameterizing security-unknown-tsk-itest would have been a lot of work and I think would've made the test harder to follow. http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@477 PS5, Line 477: token_ratio = 1.0; > Will a similar also apply for authn tokens? Also, wondering how you consid auth_token_expire-itest has some tests that explicitly test for expired tokens / token reacquisition during master leader changes (e.g. MultiMasterIdleConnectionsITest), which is similar to this (invalid tokens during master election storms). I opted to not reuse some of the existing tests because parameterizing them would be non-trivial and make them harder to understand (and I found some tests a bit confusing to start with, given a few of them are targeting very specific error scenarios). auth_token_expire-itest tests generally target expiration of tokens. authz_token-itest targets other error scenarios. http://gerrit.cloudera.org:8080/#/c/12122/5/src/kudu/integration-tests/authz_token-itest.cc@523 PS5, Line 523: // Client should have no problems connecting to an old cluster. : ASSERT_OK(Inse > What happens if a old client try to communicate with servers that require a That'd be the same as sending requests with no authz tokens, so the client would get an error. That's harder to test, since I'm not maintaining the old behavior of the client through some config flag, but https://gerrit.cloudera.org/c/11751/10/src/kudu/tserver/tablet_server_authorization-test.cc#259 shows the behavior of using the proxy with no token. -- To view, visit http://gerrit.cloudera.org:8080/12122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master
[kudu-CR] KUDU-2543 pt 1: basic checks for authz tokens
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11751 ) Change subject: KUDU-2543 pt 1: basic checks for authz tokens .. Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/11751/10/src/kudu/tserver/tablet_server_authorization-test.cc File src/kudu/tserver/tablet_server_authorization-test.cc: http://gerrit.cloudera.org:8080/#/c/11751/10/src/kudu/tserver/tablet_server_authorization-test.cc@182 PS10, Line 182: anua > nit: can you add a comment on why this number is used? Done http://gerrit.cloudera.org:8080/#/c/11751/10/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/11751/10/src/kudu/tserver/tablet_service.cc@152 PS10, Line 152: tserver_inject_invalid_authz_token_ratio > Can you add a comment on when this is expected to be used? Done http://gerrit.cloudera.org:8080/#/c/11751/9/src/kudu/tserver/tserver_service.proto File src/kudu/tserver/tserver_service.proto: http://gerrit.cloudera.org:8080/#/c/11751/9/src/kudu/tserver/tserver_service.proto@100 PS9, Line 100: optional ResourceMetricsPB resource_metrics = 7; : } : > Can you add a comment in ChecksumRequestPB that the authz token is embedded Done -- To view, visit http://gerrit.cloudera.org:8080/11751 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I99555e0ab2d09d4abcbc12b1100658a9a17590f4 Gerrit-Change-Number: 11751 Gerrit-PatchSet: 11 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 23 Jan 2019 01:29:51 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2665: deflake block manager-stress-test
Hello Mike Percy, helifu, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/12254 to review the following change. Change subject: KUDU-2665: deflake block_manager-stress-test .. KUDU-2665: deflake block_manager-stress-test After commit 0c501979b was merged, this test became really flaky (like 50% flaky in some environments). I think it's due to the new nature of the log block manager, which may now delete dead containers in the background. Specifically, if two transactions delete the last blocks from a full container, it's possible for one to get scheduled for an (asynchronous) hole punch, and for the other to set the container as dead. Later, when the hole punch runs, the container's last ref will be dropped, causing the dead container to be deleted. While perhaps surprising, this new behavior is desirable, and it's now incorrect to assume that a cessation in user threads implies an end to LBM activity. block_manager-stress-test makes this assumption by using the LBMCorruptor to inject inconsistencies after test threads have been joined. To fix, we must explicitly quiesce the LBM; destroying it will do the trick. What is surprising is that, for the life of me, I can't reproduce the failure. Not locally, not on a CentOS 6.6 machine, not looped in dist-test with stress threads, not ever. I even tried adding some "creative" sleep calls in a few places to tickle the race, to no avail. Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 --- M src/kudu/fs/block_manager-stress-test.cc 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/12254/1 -- To view, visit http://gerrit.cloudera.org:8080/12254 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0be328f740056cd6b64c9881759225c8b961a935 Gerrit-Change-Number: 12254 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2543 pt 1: basic checks for authz tokens
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11751 to look at the new patch set (#11). Change subject: KUDU-2543 pt 1: basic checks for authz tokens .. KUDU-2543 pt 1: basic checks for authz tokens In preparation for passing around authorization tokens, the tservers are now fitted with minimal token-verifying logic that protects the write and the various scan-like endpoints (i.e. scans, checksum scans, and split-key requests), optionally enforcing that the client has provided a valid authz token. I put the negotiation authn token verification logic into its own function for reuse in the tserver layer. It's worth noting that scan-like requests that have a concept of a "new" request vs a "continue" request (i.e. scans, checksum scans) will only need verification on "new" requests. "Continue" requests are handled in that a scanner cannot be hijacked by a user who didn't create it. A test is added to test various scenarios at the tserver level. Change-Id: I99555e0ab2d09d4abcbc12b1100658a9a17590f4 --- M src/kudu/rpc/CMakeLists.txt M src/kudu/rpc/rpc_header.proto A src/kudu/rpc/rpc_verification_util.cc A src/kudu/rpc/rpc_verification_util.h M src/kudu/rpc/server_negotiation.cc M src/kudu/security/token_verifier.cc M src/kudu/tserver/CMakeLists.txt M src/kudu/tserver/tablet_server-test-base.h A src/kudu/tserver/tablet_server_authorization-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver.proto M src/kudu/tserver/tserver_service.proto 12 files changed, 600 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/11751/11 -- To view, visit http://gerrit.cloudera.org:8080/11751 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I99555e0ab2d09d4abcbc12b1100658a9a17590f4 Gerrit-Change-Number: 11751 Gerrit-PatchSet: 11 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2543 pt 2: pass around default authz tokens
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12122 to look at the new patch set (#6). Change subject: KUDU-2543 pt 2: pass around default authz tokens .. KUDU-2543 pt 2: pass around default authz tokens Adds authz token generation to the master's GetTableSchema endpoint, with which clients can authorize themselves for specific tables. A client will cache these tokens and use them appropriately for RPCs that need them (e.g. Writes and Scans), reacquiring them when receiving word that they are expired. This is tested in the following ways: - unit tests for the new client-side cache for authz tokens - parameterized the token expiration test for authn and authz tokens to have varying token expirations, testing when authn tokens expire but not authz tokens, and vice versa - added various tests to ensure the client behaves correctly in various non-happy cases Change-Id: I7971d652d6adc822167cf959bffd5f994a7ca565 --- M src/kudu/client/CMakeLists.txt A src/kudu/client/authz_token_cache.cc A src/kudu/client/authz_token_cache.h M src/kudu/client/batcher.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.h M src/kudu/client/scanner-internal.cc M src/kudu/client/scanner-internal.h M src/kudu/integration-tests/CMakeLists.txt R src/kudu/integration-tests/auth_token_expire-itest.cc A src/kudu/integration-tests/authz_token-itest.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master-test.cc M src/kudu/master/master.proto M src/kudu/master/master_service.cc M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc.h M src/kudu/util/test_util.h 20 files changed, 1,331 insertions(+), 96 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/12122/6 -- To view, visit http://gerrit.cloudera.org:8080/12122 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7971d652d6adc822167cf959bffd5f994a7ca565 Gerrit-Change-Number: 12122 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [spark] Add metrics to kudu-spark
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12251 ) Change subject: [spark] Add metrics to kudu-spark .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12251/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala: http://gerrit.cloudera.org:8080/#/c/12251/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala@130 PS1, Line 130: val rowsRead: LongAccumulator doc? -- To view, visit http://gerrit.cloudera.org:8080/12251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied81c890b3d4510f767d8f023963ff878f398140 Gerrit-Change-Number: 12251 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 23 Jan 2019 00:51:06 + Gerrit-HasComments: Yes
[kudu-CR] [spark] Add metrics to kudu-spark
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12251 ) Change subject: [spark] Add metrics to kudu-spark .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12251/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala: http://gerrit.cloudera.org:8080/#/c/12251/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@69 PS1, Line 69: KA A -- To view, visit http://gerrit.cloudera.org:8080/12251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied81c890b3d4510f767d8f023963ff878f398140 Gerrit-Change-Number: 12251 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 23 Jan 2019 00:13:20 + Gerrit-HasComments: Yes
[kudu-CR] [spark] Add metrics to kudu-spark
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12251 Change subject: [spark] Add metrics to kudu-spark .. [spark] Add metrics to kudu-spark This adds some basic accumulator metrics to kudu-spark. These accumulators appear for each stage involving a KuduContext, with values broken down by executor. Follow-ups will add some more sophisticated metrics and use metrics to add additional logging. In addition to the unit tests, I tested this on a 3-node cluster and confirmed I was able to see the accumulator values for stages and individual tasks on the Spark application's web UI, and that were correct. Change-Id: Ied81c890b3d4510f767d8f023963ff878f398140 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala 6 files changed, 73 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/12251/1 -- To view, visit http://gerrit.cloudera.org:8080/12251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ied81c890b3d4510f767d8f023963ff878f398140 Gerrit-Change-Number: 12251 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] [spark] Add some logging to trace KuduContext operations
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12252 Change subject: [spark] Add some logging to trace KuduContext operations .. [spark] Add some logging to trace KuduContext operations This patch adds some logging to help track how long an entire KuduContext operation takes and also how long each part takes on each executor. This information has been sorely lacking in some cases where Spark's laziness makes attributing slowness to Kudu (vs other components of the Spark job) very difficult. Unfortunately, it's not as straightforward to add this sort of logging to reading from Kudu (KuduRDD) because Spark may lazily read batches from Kudu, and batches may be small enough that logging for each batch is so verbose that it is not useful. I tested this patch manually on a 3-node cluster and confirmed I saw the expected log messages on the driver and on the executors, e.g. 19/01/22 15:18:13 INFO kudu.KuduContext: applying operations of type 'insert' to table 'impala::default.aaa' 19/01/22 15:18:13 INFO kudu.KuduContext: applied 1 operations of type 'insert' to table 'impala::default.aaa' Change-Id: I6741f2584c1bc6b229d10d37297515474318f94c --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/OperationType.scala 2 files changed, 26 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/12252/1 -- To view, visit http://gerrit.cloudera.org:8080/12252 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6741f2584c1bc6b229d10d37297515474318f94c Gerrit-Change-Number: 12252 Gerrit-PatchSet: 1 Gerrit-Owner: Will Berkeley
[kudu-CR] De-flake sentry tests
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11961 )
Change subject: De-flake sentry tests
..
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11961/4/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/11961/4/src/kudu/util/net/net_util.cc@451
PS4, Line 451: string GetBindIpForDaemon(int index, BindMode bind_mode) {
> You can reuse this constant in kServersMaxNum too.
Done
http://gerrit.cloudera.org:8080/#/c/11961/4/src/kudu/util/net/net_util.cc@452
PS4, Line 452: // The server index should range from (0, max_servers] since
> I think I missed something: why did the requirements of 'index' change such
I changed it as I removed the logic of assign index based on the type to
GetBindIpForDaemonWithType(). My understanding of the reason why we were doing
this, is 'the last octet of a valid unicast IP address is (0, 255)'. Therefore
the index should start from 1 and <= max_servers.
I will add a comment here.
--
To view, visit http://gerrit.cloudera.org:8080/11961
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id112634ad5e6f0e2c48b7e0e89389faff4f5656b
Gerrit-Change-Number: 11961
Gerrit-PatchSet: 5
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 22 Jan 2019 22:00:37 +
Gerrit-HasComments: Yes
[kudu-CR] [sentry] add mini Sentry to the external mini cluster
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11956 )
Change subject: [sentry] add mini Sentry to the external mini cluster
..
Patch Set 11:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/mini-cluster/external_mini_cluster.cc@257
PS2, Line 257: string ktpath;
> I think so, because it'll allow macOS to benefit from a restart-less Extern
Sorry that I fail to see why. With the current approach, for macOS env we
restart Sentry once, since assign a random port can cause port collision. I
don't see how the restart can be avoided even HMS can bind to a specific host.
http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/mini-cluster/external_mini_cluster.cc@233
PS10, Line 233: we cannot choose a port at random as that can cause a port co
> Nit: "we cannot choose a port at random as that can cause a port collision.
Done
http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/mini-cluster/external_mini_cluster.cc@236
PS10, Line 236: Sentry servi
> Nit: reconfigure
Done
http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/sentry/mini_sentry.cc
File src/kudu/sentry/mini_sentry.cc:
http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/sentry/mini_sentry.cc@68
PS10, Line 68: void MiniSentry::EnableHms(string hms_uris) {
> We should CHECK(!sentry_process_) here and in the other new setters.
Done
http://gerrit.cloudera.org:8080/#/c/11956/10/src/kudu/sentry/mini_sentry.cc@137
PS10, Line 137:
> If port_ isn't 0 before this call, we might want to check that its value ha
Done
--
To view, visit http://gerrit.cloudera.org:8080/11956
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59
Gerrit-Change-Number: 11956
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 22 Jan 2019 21:59:52 +
Gerrit-HasComments: Yes
[kudu-CR] De-flake sentry tests
Hello Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11961 to look at the new patch set (#5). Change subject: De-flake sentry tests .. De-flake sentry tests Currently, sentry_client-test and sentry_authz_provider-test are still flaky due to possible port collision. To avoid such race condition, this patch moves the util function that finds the bind address for a daemon based on binding mode to test_util, and updates sentry-test-base to use it to pick up a unique bind address. Change-Id: Id112634ad5e6f0e2c48b7e0e89389faff4f5656b --- M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/integration-tests/security-itest.cc M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/mini-cluster/internal_mini_cluster.cc M src/kudu/mini-cluster/internal_mini_cluster.h M src/kudu/mini-cluster/mini_cluster.cc M src/kudu/mini-cluster/mini_cluster.h M src/kudu/sentry/sentry-test-base.h M src/kudu/tools/kudu-tool-test.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h 13 files changed, 165 insertions(+), 139 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/11961/5 -- To view, visit http://gerrit.cloudera.org:8080/11961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id112634ad5e6f0e2c48b7e0e89389faff4f5656b Gerrit-Change-Number: 11961 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [sentry] add mini Sentry to the external mini cluster
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11956 to look at the new patch set (#11). Change subject: [sentry] add mini Sentry to the external mini cluster .. [sentry] add mini Sentry to the external mini cluster This commit enables external mini cluster to be able to start with both mini sentry and mini hms. A major challenge is the circular dependency between the HMS and Sentry in terms of configuring each with the other's address, including port which is currently discovered by polling lsof. To work around it, one approach would be to: 1. Start the Sentry service. Find out which port it's on. At this stage, the Sentry service has no knowledge of any HMS service. 2. Start the HMS, configured to talk to Sentry's port. Find out which port it's on. 3. Restart the Sentry service on the same port, reconfigured to talk to the HMS's port. However, there could be a race condition where another program binds to the same port during the restart in step 3. One option is to use SO_REUSEPORT socket option, which permits multiple sockets to be bound to an identical socket address. Although both Sentry and HMS are written in Java, only JDK9 (and above) supports this socket option[1]. Methods such as Java reflection can be used to invoke private methods to set up this option, but they are hacky. This patch extends the UNIQUE_LOOPBACK usage in mini cluster, so that external servers such as Sentry can pick up a unique bind address, knowing that in doing so there'll never be any danger of a port collision. I looped master_sentry-itest 1000 times (which used UNIQUE_LOOPBACK mode) and all passed: http://dist-test.cloudera.org/job?job_id=hao.hao.1548189107.129474 [1]: https://docs.oracle.com/javase/9/docs/api/java/net/StandardSocketOptions.html#SO_REUSEPORT Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 --- M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/master_sentry-itest.cc M src/kudu/mini-cluster/CMakeLists.txt M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/mini-cluster/mini_cluster.cc M src/kudu/mini-cluster/mini_cluster.h M src/kudu/security/test/mini_kdc.cc M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry-test-base.h M src/kudu/util/test_util.cc M src/kudu/util/test_util.h 15 files changed, 526 insertions(+), 77 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/11956/11 -- To view, visit http://gerrit.cloudera.org:8080/11956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 Gerrit-Change-Number: 11956 Gerrit-PatchSet: 11 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [docker] Add an initial docker build
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 ) Change subject: [docker] Add an initial docker build .. Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/12173/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12173/2//COMMIT_MSG@20 PS2, Line 20: muti multi http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile File docker/Dockerfile: http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@71 PS2, Line 71: # TODO: Remove build dir too? In theory you could remove thirdparty/build too, except that the clang-toolchain symlink points to llvm-6.0.0 within it. Off the top of my head I can't remember why that's the case (i.e. why it doesn't point to somewhere in thirdparty/installed). http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile@110 PS2, Line 110: COPY ./docs/support docs/support Can we skip this? Not seeing where we build docs below. http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile-files File docker/Dockerfile-files: http://gerrit.cloudera.org:8080/#/c/12173/2/docker/Dockerfile-files@18 PS2, Line 18: # This docker file is a minimal build usefull for debugging useful http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh File docker/kudu-entrypoint.sh: http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh@49 PS2, Line 49: KUDU_OPTS="$KUDU_FLAGS \ Is the idea that KUDU_FLAGS can be set by users to do more configuration? Maybe doc that at the top of the file? http://gerrit.cloudera.org:8080/#/c/12173/2/docker/kudu-entrypoint.sh@52 PS2, Line 52: --fs_data_dirs=/var/lib/kudu/master \ You can omit this if the intent is to use the same value as --fs_wal_dir. Below too. http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh File thirdparty/build-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/12173/2/thirdparty/build-thirdparty.sh@248 PS2, Line 248: # Install Hadoop, Hive, and Sentry by copying their source directories into $PREFIX/opt. : # Copying allows the source directory to be removed once thirdparty is built. How much additional disk space will this consume? Also, will the migration be smooth for existing thirdparty trees where these are symlinks? Looking at the build results for this patch, looks like the migration doesn't work at all. Maybe we need to rm -rf the existing symlinks first? I wonder if we should use rsync to manage this instead, as it'll do a better job of synchronizing the source and destination. -- To view, visit http://gerrit.cloudera.org:8080/12173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42 Gerrit-Change-Number: 12173 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 22 Jan 2019 21:44:59 + Gerrit-HasComments: Yes
