[kudu-CR] [security] clean-up on cert management-test.cc
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/5937 Change subject: [security] clean-up on cert_management-test.cc .. [security] clean-up on cert_management-test.cc A minor clean-up on cert-related tests: moved other than X509 CSR-related tests from cert_management-test.cc into cert-test.cc. Also, removed duplicated key-specific tests from cert_management.cc: they are in crypto-test.cc now (probably, the duplication was the result of merge conflicts resolution). There are no functional changes in this patch. Change-Id: I3e42d8545e783fbc657de9bf2d4d231265cf3f3f --- M src/kudu/security/ca/cert_management-test.cc M src/kudu/security/cert-test.cc 2 files changed, 22 insertions(+), 62 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/5937/1 -- To view, visit http://gerrit.cloudera.org:8080/5937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3e42d8545e783fbc657de9bf2d4d231265cf3f3f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] [security] method to check if X509 cert matches key
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/5936 Change subject: [security] method to check if X509 cert matches key .. [security] method to check if X509 cert matches key Added a method to security::Cert interface to check whether X509 certificate matches the specified private key. Change-Id: I6732f8a1ad9ed1b1ab26b94b582af50e6b6d24b1 --- M src/kudu/security/CMakeLists.txt A src/kudu/security/cert-test.cc M src/kudu/security/cert.cc M src/kudu/security/cert.h 4 files changed, 96 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/5936/1 -- To view, visit http://gerrit.cloudera.org:8080/5936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6732f8a1ad9ed1b1ab26b94b582af50e6b6d24b1 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] WIP [security] load TSK from system table
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/5935 Change subject: WIP [security] load TSK from system table .. WIP [security] load TSK from system table Change-Id: Ie91d4129bda0ca49e81988c28385895a2abcd201 --- M src/kudu/master/authn_token_manager.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.cc M src/kudu/master/master.proto M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/security/token_signer.cc M src/kudu/security/token_signer.h 9 files changed, 263 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/5935/1 -- To view, visit http://gerrit.cloudera.org:8080/5935 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie91d4129bda0ca49e81988c28385895a2abcd201 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] WIP [security] tailored TokenSigner for system catalog
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5930 to look at the new patch set (#3). Change subject: WIP [security] tailored TokenSigner for system catalog .. WIP [security] tailored TokenSigner for system catalog Updated the TokenSigner class in preparation to loading public part of token signing keys from the system catalog. The expected use-case for the TokenSigner is calling AddTokenSigningPublicKey() multiple times while loading public part of TSKs from the system catalog and subsequent call of Init(). That's the sequence to be exercised by a master server upon becoming a leader. It's possible to run this sequence multiple times on the same instance of TokenSigner, generating new signing keys only when already existing signing keys are about to expire. Also, the TokenSigner class now embeds the logic to rotate its signing key on Init(), if necessary. Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c --- M src/kudu/master/authn_token_manager.cc M src/kudu/master/authn_token_manager.h M src/kudu/master/master.cc M src/kudu/security/token-test.cc M src/kudu/security/token_signer.cc M src/kudu/security/token_signer.h M src/kudu/security/token_signing_key.cc M src/kudu/security/token_signing_key.h 8 files changed, 307 insertions(+), 54 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/5930/3 -- To view, visit http://gerrit.cloudera.org:8080/5930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] java: use a netty frame decoder instead of replaying decoder
Jean-Daniel Cryans has posted comments on this change. Change subject: java: use a netty frame decoder instead of replaying decoder .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/5926/2/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java: Line 174: // TODO(todd): this should return here. We seem to lack test coverage! Wouldn't that require a server that doesn't support feature flags... ? -- To view, visit http://gerrit.cloudera.org:8080/5926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] java: fix ability to connect to a real Kerberized cluster
Jean-Daniel Cryans has posted comments on this change. Change subject: java: fix ability to connect to a real Kerberized cluster .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/5922/3/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java: PS3, Line 210: nit Line 245: nit http://gerrit.cloudera.org:8080/#/c/5922/3/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java: PS3, Line 86: . remove http://gerrit.cloudera.org:8080/#/c/5922/3/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java: PS3, Line 328: . remove -- To view, visit http://gerrit.cloudera.org:8080/5922 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add Google Breakpad support to Kudu
Adar Dembo has posted comments on this change. Change subject: Add Google Breakpad support to Kudu .. Patch Set 34: (1 comment) http://gerrit.cloudera.org:8080/#/c/5620/34/src/kudu/util/minidump.h File src/kudu/util/minidump.h: Line 86: __attribute__((unused)) std::atomic user_signal_handler_thread_running_; Hmm, I thought this typically goes after the variable declaration, not before. Also, use ATTRIBUTE_UNUSED from gutil/port.h, I think that's preferable. -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 34 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add Google Breakpad support to Kudu
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5620 to look at the new patch set (#35). Change subject: Add Google Breakpad support to Kudu .. Add Google Breakpad support to Kudu Breakpad creates minidumps upon crash, which are small files that include stack-based information for each thread. These do not include heap information, so they complement, not replace, core files for debugging purposes. This patch enables Google Breakpad for the kudu-tserver and kudu-master processes. Breakpad is part of the crash-reporting infrastructure open-sourced by Google. It is also used by Mozilla Firefox and Google Chrome. When a process crashes, Breakpad writes a small dump file called a minidump to disk. Since these files are typically only a few MB in size, this allows users to share critical crash information with developers for offline analysis. For more information, see the breakpad getting started docs at http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md By default, on crash, minidump files are written to a subdirectory of the log directory for a given Kudu daemon process. This was chosen for the following reasons: 1. The minidump files are relatively small, potentially comparable in size to log files, and the log directory is a place an administrator would look in when debugging. 2. This convention is consistent with what Apache Impala (incubating) does for its minidump files. Changes in this patch: * Add breakpad to thirdparty. * Add breakpad source archive creation script (Breakpad does not do releases). * Add a "hack" to the breakpad thirdparty installation routine to forcibly add a breakpad/ prefix to the breakpad header files. This frees us from having to modify our include path when building. * Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala (incubating). The modifications consist of removing use of boost::filesystem, conformance to the Kudu code style guidelines, and some significant refactoring. * Enable breakpad support for the kudu-tserver and kudu-master processes. * By default, invoke previously-installed signal handler if installed. * Handle SIGUSR1 by safely using sigwait() to create minidumps on demand. * Add #ifdefs and CMake logic for Linux. * Add test for all the deadly signals to ensure we get both a stack trace and a minidump, except for the case of SIGTERM, where we should not get a minidump (by design). * Change a few library unit tests that relied on SIGUSR1 to use another signal for their testing, such as SIGHUP or SIGUSR2. * Remove unused includes from mini_tablet_server.cc * Clean up signal handling logic when forking a subprocess. * Hide breakpad symbols from libkuduclient.so * Install failure function that simply calls abort() instead of first printing a stacktrace. Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh A cmake_modules/FindBreakpadClient.cmake M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M src/kudu/client/client_samples-test.sh M src/kudu/client/symbols.map M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h A src/kudu/integration-tests/minidump_generation-itest.cc M src/kudu/master/mini_master.cc M src/kudu/server/server_base.cc M src/kudu/server/server_base.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/debug-util-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/logging.cc A src/kudu/util/minidump-test.cc A src/kudu/util/minidump.cc A src/kudu/util/minidump.h M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/test_main.cc M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/preflight.py A thirdparty/scripts/make-breakpad-src-archive.sh M thirdparty/vars.sh 30 files changed, 1,027 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/35 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 35 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add Google Breakpad support to Kudu
Hello Mike Percy, Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5620 to look at the new patch set (#34). Change subject: Add Google Breakpad support to Kudu .. Add Google Breakpad support to Kudu Breakpad creates minidumps upon crash, which are small files that include stack-based information for each thread. These do not include heap information, so they complement, not replace, core files for debugging purposes. This patch enables Google Breakpad for the kudu-tserver and kudu-master processes. Breakpad is part of the crash-reporting infrastructure open-sourced by Google. It is also used by Mozilla Firefox and Google Chrome. When a process crashes, Breakpad writes a small dump file called a minidump to disk. Since these files are typically only a few MB in size, this allows users to share critical crash information with developers for offline analysis. For more information, see the breakpad getting started docs at http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md By default, on crash, minidump files are written to a subdirectory of the log directory for a given Kudu daemon process. This was chosen for the following reasons: 1. The minidump files are relatively small, potentially comparable in size to log files, and the log directory is a place an administrator would look in when debugging. 2. This convention is consistent with what Apache Impala (incubating) does for its minidump files. Changes in this patch: * Add breakpad to thirdparty. * Add breakpad source archive creation script (Breakpad does not do releases). * Add a "hack" to the breakpad thirdparty installation routine to forcibly add a breakpad/ prefix to the breakpad header files. This frees us from having to modify our include path when building. * Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala (incubating). The modifications consist of removing use of boost::filesystem, conformance to the Kudu code style guidelines, and some significant refactoring. * Enable breakpad support for the kudu-tserver and kudu-master processes. * By default, invoke previously-installed signal handler if installed. * Handle SIGUSR1 by safely using sigwait() to create minidumps on demand. * Add #ifdefs and CMake logic for Linux. * Add test for all the deadly signals to ensure we get both a stack trace and a minidump, except for the case of SIGTERM, where we should not get a minidump (by design). * Change a few library unit tests that relied on SIGUSR1 to use another signal for their testing, such as SIGHUP or SIGUSR2. * Remove unused includes from mini_tablet_server.cc * Clean up signal handling logic when forking a subprocess. * Hide breakpad symbols from libkuduclient.so * Install failure function that simply calls abort() instead of first printing a stacktrace. Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh A cmake_modules/FindBreakpadClient.cmake M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M src/kudu/client/client_samples-test.sh M src/kudu/client/symbols.map M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h A src/kudu/integration-tests/minidump_generation-itest.cc M src/kudu/master/mini_master.cc M src/kudu/server/server_base.cc M src/kudu/server/server_base.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/debug-util-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/logging.cc A src/kudu/util/minidump-test.cc A src/kudu/util/minidump.cc A src/kudu/util/minidump.h M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/test_main.cc M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/preflight.py A thirdparty/scripts/make-breakpad-src-archive.sh M thirdparty/vars.sh 30 files changed, 1,034 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/34 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 34 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add Google Breakpad support to Kudu
David Ribeiro Alves has posted comments on this change. Change subject: Add Google Breakpad support to Kudu .. Patch Set 33: just posted a couple of fixes that make this compile in macOS. mike feel free to grab and fix whatever you thing needs fixing. -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 33 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add Google Breakpad support to Kudu
Hello Mike Percy, Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5620 to look at the new patch set (#33). Change subject: Add Google Breakpad support to Kudu .. Add Google Breakpad support to Kudu Breakpad creates minidumps upon crash, which are small files that include stack-based information for each thread. These do not include heap information, so they complement, not replace, core files for debugging purposes. This patch enables Google Breakpad for the kudu-tserver and kudu-master processes. Breakpad is part of the crash-reporting infrastructure open-sourced by Google. It is also used by Mozilla Firefox and Google Chrome. When a process crashes, Breakpad writes a small dump file called a minidump to disk. Since these files are typically only a few MB in size, this allows users to share critical crash information with developers for offline analysis. For more information, see the breakpad getting started docs at http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md By default, on crash, minidump files are written to a subdirectory of the log directory for a given Kudu daemon process. This was chosen for the following reasons: 1. The minidump files are relatively small, potentially comparable in size to log files, and the log directory is a place an administrator would look in when debugging. 2. This convention is consistent with what Apache Impala (incubating) does for its minidump files. Changes in this patch: * Add breakpad to thirdparty. * Add breakpad source archive creation script (Breakpad does not do releases). * Add a "hack" to the breakpad thirdparty installation routine to forcibly add a breakpad/ prefix to the breakpad header files. This frees us from having to modify our include path when building. * Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala (incubating). The modifications consist of removing use of boost::filesystem, conformance to the Kudu code style guidelines, and some significant refactoring. * Enable breakpad support for the kudu-tserver and kudu-master processes. * By default, invoke previously-installed signal handler if installed. * Handle SIGUSR1 by safely using sigwait() to create minidumps on demand. * Add #ifdefs and CMake logic for Linux. * Add test for all the deadly signals to ensure we get both a stack trace and a minidump, except for the case of SIGTERM, where we should not get a minidump (by design). * Change a few library unit tests that relied on SIGUSR1 to use another signal for their testing, such as SIGHUP or SIGUSR2. * Remove unused includes from mini_tablet_server.cc * Clean up signal handling logic when forking a subprocess. * Hide breakpad symbols from libkuduclient.so * Install failure function that simply calls abort() instead of first printing a stacktrace. Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh A cmake_modules/FindBreakpadClient.cmake M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M src/kudu/client/client_samples-test.sh M src/kudu/client/symbols.map M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h A src/kudu/integration-tests/minidump_generation-itest.cc M src/kudu/master/mini_master.cc M src/kudu/server/server_base.cc M src/kudu/server/server_base.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/debug-util-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/logging.cc A src/kudu/util/minidump-test.cc A src/kudu/util/minidump.cc A src/kudu/util/minidump.h M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/test_main.cc M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/preflight.py A thirdparty/scripts/make-breakpad-src-archive.sh M thirdparty/vars.sh 30 files changed, 1,032 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/33 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 33 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Make SecureRpcHelper a Netty pipeline stage
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5927 to look at the new patch set (#2). Change subject: Make SecureRpcHelper a Netty pipeline stage .. Make SecureRpcHelper a Netty pipeline stage This decouples SecureRpcHelper from TabletClient. When negotiation is complete, it sends an "event" upstream to the TabletClient and removes itself from the pipeline. I'm hoping this decoupling will make it easier to test SecureRpcHelper in isolation as we add more functionality like TLS negotiation. After this is committed, I'm also hoping to rename SecureRpcHelper to Negotiator. Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b --- M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java 2 files changed, 84 insertions(+), 75 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/5927/2 -- To view, visit http://gerrit.cloudera.org:8080/5927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] java: fix ability to connect to a real Kerberized cluster
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5922 to look at the new patch set (#3). Change subject: java: fix ability to connect to a real Kerberized cluster .. java: fix ability to connect to a real Kerberized cluster This fixes a couple issues seen when I tried to run the Java client from within Impala against a Kerberized Kudu cluster: * Previously, the ServerInfo class remembered the already-resolved address of the server rather than its hostname. This meant that we would try to connect to a Kerberos principal "kudu/1.2.3.4" rather than "kudu/foo.example.com". This typically would not match the actual principal the server was using, resulting in an error that the server principal was not found in the database. * We previously were using 'subject.doAs()' when initializing the SASL server, but not when evaluating challenges. It turns out that the GSSAPI mechanism only looks for the Kerberos credentials while evaluating the challenge. This moves the 'doAs()' to wrap the challenge. * Another issue with the SASL setup was that we were passing all of the available client mechanisms into Sasl.createSaslClient() before seeing which mechanisms the server was advertised. the SASL library seems to always prefer GSSAPI over PLAIN when available. This meant that, if the server had Kerberos credentials, it would attempt to use GSSAPI and not PLAIN even if connecting to a server which only advertised plain. This fixes the negotiation to no longer ignore the server-side advertised mechanisms, and instead actually negotiate by picking the best mechanism which is both advertised by the server and initializable by the client. * We previously assumed that Kerberos credentials would be available from the 'Subject' without any explicit login call. This isn't the case: we have to explicitly set up a LoginContext and Configuration to log in from the credentials cache. This changes the client constructor to login from the ccache if there is no Subject available with Kerberos credentials. With these changes I was able to run an Impala query against a cluster with -server_require_kerberos. Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java A java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKdc.java M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKdc.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestMiniKuduCluster.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java 11 files changed, 281 insertions(+), 129 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/5922/3 -- To view, visit http://gerrit.cloudera.org:8080/5922 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] java: use a netty frame decoder instead of replaying decoder
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5926 to look at the new patch set (#2). Change subject: java: use a netty frame decoder instead of replaying decoder .. java: use a netty frame decoder instead of replaying decoder All of our inbound packets are length-prefixed. Netty provides a nice class for handing length-prefixed messages. This avoids a bunch of more custom logic we were doing for length checking, etc. Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f --- M java/kudu-client/src/main/java/org/apache/kudu/client/CallResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java 5 files changed, 48 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/5926/2 -- To view, visit http://gerrit.cloudera.org:8080/5926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] java: remove unused parts of SecureRpcHelper
Hello Dan Burkert, Jean-Daniel Cryans, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5925 to look at the new patch set (#2). Change subject: java: remove unused parts of SecureRpcHelper .. java: remove unused parts of SecureRpcHelper This removes the unused wrap/unwrap code from SecureRpcHelper. I also removed an unnecessary function in TabletClient. Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df --- M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java 2 files changed, 3 insertions(+), 52 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/5925/2 -- To view, visit http://gerrit.cloudera.org:8080/5925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Make SecureRpcHelper a Netty pipeline stage
Todd Lipcon has posted comments on this change. Change subject: Make SecureRpcHelper a Netty pipeline stage .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/5927/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java: Line 113: > RED yea, I need to figure out how to configure eclipse not to do this. Line 127: RpcHeader.NegotiatePB response = parseSaslMsgResponse(buf); > This needs to check for an error response, or maybe punt and add a TODO. added a TODO -- To view, visit http://gerrit.cloudera.org:8080/5927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] java: use a netty frame decoder instead of replaying decoder
Todd Lipcon has posted comments on this change. Change subject: java: use a netty frame decoder instead of replaying decoder .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java: Line 72: > I see red. Done Line 78:* The Hadoop RPC protocol doesn't do any checksumming as they probably > For my own edification: is this comment implying that the protocol should h yea, I think so. This is just a moved comment. I think it dates back to the OpenTSDB original source, which has plenty of "editorializing". http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java: Line 174: // TODO(todd): shouldn't this return here? > Yes. ok i'll make the TODO more serious and point out there is no coverage of this. -- To view, visit http://gerrit.cloudera.org:8080/5926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] WIP: fixes for java client kerberos against a real cluster
Todd Lipcon has posted comments on this change. Change subject: WIP: fixes for java client kerberos against a real cluster .. Patch Set 2: (10 comments) http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: Line 1830: private static Subject getSubjectOrLogin() { > Seems like this could be extracted into some helper class. Done PS2, Line 1834: " > use {} substitution Done Line 1840: @Override > Pretty sure we normally use double indentation for anonymous classes. Done Line 1850: options.put("debug", "" + Boolean.getBoolean("kudu.jaas.debug")); > Boolean.toString(Boolean.getBoolean("kudu.jaas.debug")) Done Line 1871: LOG.debug("Logged in as subject: " + subject.toString()); > {} substitution Done PS2, Line 1874: c > Could* Done http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java: Line 116: if (saslClient == null || !saslClient.isComplete() || negoUnderway) { > Looks like this could this be simplified to just 'if (negoUnderway)' Done Line 206: for (RpcHeader.NegotiatePB.SaslAuth auth : response.getAuthsList()) { > This shouldn't be done in a loop. The Java client should intersect the ser Done Line 270: if (e instanceof SaslException) { > This would be simpler as a separate catch clause. it complains about that because the Subject.doAs doesn't say it "throws SaslException". I used Throwables from guava here which is cleaner. http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java: Line 85: public InetAddress getResolvedAddress() { > nit: javadoc Done -- To view, visit http://gerrit.cloudera.org:8080/5922 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] java: remove unused parts of SecureRpcHelper
Todd Lipcon has posted comments on this change. Change subject: java: remove unused parts of SecureRpcHelper .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5925/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java: Line 141 > We're going to need unwrap to verify channel bindings. yea, but it doesn't need all this netty junk associated with it - we'll just be unwrapping/wrapping a byte[], which is just the underlying saslClient.unwrap/wrap call -- To view, visit http://gerrit.cloudera.org:8080/5925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add missing pb util proto dependency to token proto
David Ribeiro Alves has submitted this change and it was merged. Change subject: Add missing pb_util_proto dependency to token_proto .. Add missing pb_util_proto dependency to token_proto Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b Reviewed-on: http://gerrit.cloudera.org:8080/5932 Reviewed-by: Alexey Serbin Tested-by: Kudu Jenkins --- M src/kudu/security/CMakeLists.txt 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add Google Breakpad support to Kudu
Adar Dembo has posted comments on this change. Change subject: Add Google Breakpad support to Kudu .. Patch Set 32: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 32 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add Google Breakpad support to Kudu
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5620 to look at the new patch set (#32). Change subject: Add Google Breakpad support to Kudu .. Add Google Breakpad support to Kudu Breakpad creates minidumps upon crash, which are small files that include stack-based information for each thread. These do not include heap information, so they complement, not replace, core files for debugging purposes. This patch enables Google Breakpad for the kudu-tserver and kudu-master processes. Breakpad is part of the crash-reporting infrastructure open-sourced by Google. It is also used by Mozilla Firefox and Google Chrome. When a process crashes, Breakpad writes a small dump file called a minidump to disk. Since these files are typically only a few MB in size, this allows users to share critical crash information with developers for offline analysis. For more information, see the breakpad getting started docs at http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md By default, on crash, minidump files are written to a subdirectory of the log directory for a given Kudu daemon process. This was chosen for the following reasons: 1. The minidump files are relatively small, potentially comparable in size to log files, and the log directory is a place an administrator would look in when debugging. 2. This convention is consistent with what Apache Impala (incubating) does for its minidump files. Changes in this patch: * Add breakpad to thirdparty. * Add breakpad source archive creation script (Breakpad does not do releases). * Add a "hack" to the breakpad thirdparty installation routine to forcibly add a breakpad/ prefix to the breakpad header files. This frees us from having to modify our include path when building. * Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala (incubating). The modifications consist of removing use of boost::filesystem, conformance to the Kudu code style guidelines, and some significant refactoring. * Enable breakpad support for the kudu-tserver and kudu-master processes. * By default, invoke previously-installed signal handler if installed. * Handle SIGUSR1 by safely using sigwait() to create minidumps on demand. * Add #ifdefs and CMake logic for Linux. * Add test for all the deadly signals to ensure we get both a stack trace and a minidump, except for the case of SIGTERM, where we should not get a minidump (by design). * Change a few library unit tests that relied on SIGUSR1 to use another signal for their testing, such as SIGHUP or SIGUSR2. * Remove unused includes from mini_tablet_server.cc * Clean up signal handling logic when forking a subprocess. * Hide breakpad symbols from libkuduclient.so * Install failure function that simply calls abort() instead of first printing a stacktrace. Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh A cmake_modules/FindBreakpadClient.cmake M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M src/kudu/client/client_samples-test.sh M src/kudu/client/symbols.map M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h A src/kudu/integration-tests/minidump_generation-itest.cc M src/kudu/master/mini_master.cc M src/kudu/server/server_base.cc M src/kudu/server/server_base.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/debug-util-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/logging.cc A src/kudu/util/minidump-test.cc A src/kudu/util/minidump.cc A src/kudu/util/minidump.h M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/test_main.cc M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/preflight.py A thirdparty/scripts/make-breakpad-src-archive.sh M thirdparty/vars.sh 30 files changed, 1,027 insertions(+), 85 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/32 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 32 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates
Dan Burkert has submitted this change and it was merged. Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list predicates .. [java client] KUDU-1643 Prune hash partitions based on IN-list predicates PartitionPruner is updated to search all combinations of in-list column values and prune hash partitions where possible. This also fixes a small issue in the C++ version of the algorithm: previously the implementation would always consider the final hash component to be constrained. The bug (as far as I can tell) doesn't lead to erroneous results, but does cause more partition ranges to be created, which results in more memory overhead and compute overhead to compute the pruning. Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Reviewed-on: http://gerrit.cloudera.org:8080/5677 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans --- M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java M src/kudu/common/partial_row.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/common/scan_spec.cc 7 files changed, 535 insertions(+), 339 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list predicates .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add missing pb util proto dependency to token proto
Alexey Serbin has posted comments on this change. Change subject: Add missing pb_util_proto dependency to token_proto .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add missing pb util proto dependency to token proto
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/5932 to review the following change. Change subject: Add missing pb_util_proto dependency to token_proto .. Add missing pb_util_proto dependency to token_proto Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b --- M src/kudu/security/CMakeLists.txt 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/5932/1 -- To view, visit http://gerrit.cloudera.org:8080/5932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I63f743bcf4b36a884c58a292a7eb03bf9098f78b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add Google Breakpad support to Kudu
Adar Dembo has posted comments on this change. Change subject: Add Google Breakpad support to Kudu .. Patch Set 31: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 31 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1856: always truncate containers when they get full
Todd Lipcon has posted comments on this change. Change subject: KUDU-1856: always truncate containers when they get full .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/5852/3/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 137: CHECK_EQ(mode, 0); > That's what I thought too, but Todd asked me to change it. I'm not going to It seems we're not super consistent. Here's some greps looking at whether the member variable comes first or second in various CHECK_EQ: todd@todd-ThinkPad-T540p:~/git/kudu$ git grep 'CHECK_EQ([^,]*, [a-zA-z]*_)' | wc -l 50 todd@todd-ThinkPad-T540p:~/git/kudu$ git grep 'CHECK_EQ([a-zA-z]*_, [^,]*)' | wc -l 114 so seems like we usually prefer CHECK_EQ(foo_, ) rather than the other way around, but not overwhelmingly so I'd misremembered the output from a failed CHECK_EQ though. It seems it doesn't really matter the order because the output doesn't purport to call one side or the other "expected" or "actual" (whereas gtest does). Instead you just get Check failed: 1 == 2 (1 vs. 2) I lean towards 'CHECK_EQ(mode, 0)' for the same reason I lean towards 'if (mode == 0)' - it just is more close to how we usually speak. But don't feel strongly now that I know the output isn't confusing. -- To view, visit http://gerrit.cloudera.org:8080/5852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic959c59489a08f92efa2df5c85b22e56740f1346 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] WIP: enable kerberos support in CSD
Todd Lipcon has posted comments on this change. Change subject: WIP: enable kerberos support in CSD .. Patch Set 1: yea, that's fine. It's useful to have up on gerrit, though -- makes it much easier to test in a kerberized environment, even if you have to do a local build. -- To view, visit http://gerrit.cloudera.org:8080/5921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea2d36b7a0da38cd6ddd646a32f52517cabeb4a6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add Google Breakpad support to Kudu
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5620 to look at the new patch set (#31). Change subject: Add Google Breakpad support to Kudu .. Add Google Breakpad support to Kudu Breakpad creates minidumps upon crash, which are small files that include stack-based information for each thread. These do not include heap information, so they complement, not replace, core files for debugging purposes. This patch enables Google Breakpad for the kudu-tserver and kudu-master processes. Breakpad is part of the crash-reporting infrastructure open-sourced by Google. It is also used by Mozilla Firefox and Google Chrome. When a process crashes, Breakpad writes a small dump file called a minidump to disk. Since these files are typically only a few MB in size, this allows users to share critical crash information with developers for offline analysis. For more information, see the breakpad getting started docs at http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md By default, on crash, minidump files are written to a subdirectory of the log directory for a given Kudu daemon process. This was chosen for the following reasons: 1. The minidump files are relatively small, potentially comparable in size to log files, and the log directory is a place an administrator would look in when debugging. 2. This convention is consistent with what Apache Impala (incubating) does for its minidump files. Changes in this patch: * Add breakpad to thirdparty. * Add breakpad source archive creation script (Breakpad does not do releases). * Add a "hack" to the breakpad thirdparty installation routine to forcibly add a breakpad/ prefix to the breakpad header files. This frees us from having to modify our include path when building. * Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala (incubating). The modifications consist of removing use of boost::filesystem, conformance to the Kudu code style guidelines, and some significant refactoring. * Enable breakpad support for the kudu-tserver and kudu-master processes. * By default, invoke previously-installed signal handler if installed. * Handle SIGUSR1 by safely using sigwait() to create minidumps on demand. * Add #ifdefs and CMake logic for Linux. * Add test for all the deadly signals to ensure we get both a stack trace and a minidump, except for the case of SIGTERM, where we should not get a minidump (by design). * Change a few library unit tests that relied on SIGUSR1 to use another signal for their testing, such as SIGHUP or SIGUSR2. * Remove unused includes from mini_tablet_server.cc * Clean up signal handling logic when forking a subprocess. * Hide breakpad symbols from libkuduclient.so * Install failure function that simply calls abort() instead of first printing a stacktrace. Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh A cmake_modules/FindBreakpadClient.cmake M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M src/kudu/client/client_samples-test.sh M src/kudu/client/symbols.map M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h A src/kudu/integration-tests/minidump_generation-itest.cc M src/kudu/master/mini_master.cc M src/kudu/server/server_base.cc M src/kudu/server/server_base.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/debug-util-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/logging.cc A src/kudu/util/minidump-test.cc A src/kudu/util/minidump.cc A src/kudu/util/minidump.h M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/test_main.cc M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/preflight.py A thirdparty/scripts/make-breakpad-src-archive.sh M thirdparty/vars.sh 30 files changed, 994 insertions(+), 84 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/31 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 31 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add Google Breakpad support to Kudu
Adar Dembo has posted comments on this change. Change subject: Add Google Breakpad support to Kudu .. Patch Set 30: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 30 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1856: always truncate containers when they get full
Adar Dembo has posted comments on this change. Change subject: KUDU-1856: always truncate containers when they get full .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/5852/3/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 137: CHECK_EQ(mode, 0); > nit: expected comes before actual in CHECK_EQ() That's what I thought too, but Todd asked me to change it. I'm not going to switch it back, but we can talk about what the right order should be and hew to that going forward. -- To view, visit http://gerrit.cloudera.org:8080/5852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic959c59489a08f92efa2df5c85b22e56740f1346 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] [thirdparty] Make Boost a regular dependency
Adar Dembo has posted comments on this change. Change subject: [thirdparty] Make Boost a regular dependency .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [thirdparty] Make Boost a regular dependency
Adar Dembo has submitted this change and it was merged. Change subject: [thirdparty] Make Boost a regular dependency .. [thirdparty] Make Boost a regular dependency For the new TIMESTAMP type we must mimic Impala's TimestampValue format to make sure that the types have the same memory format. This will allow to avoid costly conversions at scan time. Impala's type uses boost's ptime and nanosecond resolution and so must Kudu. Previously our use of boost was header-only, but the date_time library (which includes ptime) needs to be compiled. This requires that we actually compile libboost_date_time in thirparty. This patch does that and also changes the way we handle boost to treat it more like a regular dependency, both regarding the new static and shared library files and regarding headers. Since boost uses its own build system this required tinkering with jamfiles to make sure that the library is correctly built under tsan. Finally this patch also addresses a couple of other issues: - Adds "thirdparty/installed/common/include" to the include dirs explicitly (we were doing this indirectly when we added the boost include dir before). - Since FindKuduBoost.cmake now explicitly sets BOOST_INCLUDE_DIR to point to wherever boost was installed, this removes the logic that makes sure we didn't pick up boost from the system path. Change-Id: I277c4fda15575e271c426735552762884cb28c43 Reviewed-on: http://gerrit.cloudera.org:8080/5818 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy Reviewed-by: Adar Dembo --- M CMakeLists.txt A cmake_modules/FindKuduBoost.cmake M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh 4 files changed, 124 insertions(+), 28 deletions(-) Approvals: Mike Percy: Looks good to me, but someone else must approve Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] WIP [security] tailored TokenSigner for system catalog
Alexey Serbin has uploaded a new patch set (#2). Change subject: WIP [security] tailored TokenSigner for system catalog .. WIP [security] tailored TokenSigner for system catalog Updated the TokenSigner class in preparation to loading public part of token signing keys from the system catalog. The expected use-case for the TokenSigner is calling AddTokenSigningPublicKey() multiple times while loading public part of TSKs from the system catalog and subsequent call of Init(). That's the sequence to be exercised by a master server upon becoming a leader. It's possible to run this sequence multiple times on the same instance of TokenSigner, generating new signing keys only when already existing signing keys are about to expire. Also, the TokenSigner class now embeds the logic to rotate its signing key on Init(), if necessary. Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c --- M src/kudu/security/token-test.cc M src/kudu/security/token_signer.cc M src/kudu/security/token_signer.h M src/kudu/security/token_signing_key.cc M src/kudu/security/token_signing_key.h 5 files changed, 302 insertions(+), 50 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/5930/2 -- To view, visit http://gerrit.cloudera.org:8080/5930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5677 to look at the new patch set (#9). Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list predicates .. [java client] KUDU-1643 Prune hash partitions based on IN-list predicates PartitionPruner is updated to search all combinations of in-list column values and prune hash partitions where possible. This also fixes a small issue in the C++ version of the algorithm: previously the implementation would always consider the final hash component to be constrained. The bug (as far as I can tell) doesn't lead to erroneous results, but does cause more partition ranges to be created, which results in more memory overhead and compute overhead to compute the pruning. Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 --- M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java M src/kudu/common/partial_row.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/common/scan_spec.cc 7 files changed, 535 insertions(+), 339 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/5677/9 -- To view, visit http://gerrit.cloudera.org:8080/5677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates
Dan Burkert has posted comments on this change. Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list predicates .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/5677/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java: PS7, Line 51: > nit: remove Done PS7, Line 488: > nit Done -- To view, visit http://gerrit.cloudera.org:8080/5677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5677 to look at the new patch set (#8). Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list predicates .. [java client] KUDU-1643 Prune hash partitions based on IN-list predicates PartitionPruner is updated to search all combinations of in-list column values and prune hash partitions where possible. This also fixes a small issue in the C++ version of the algorithm: previously the implementation would always consider the final hash component to be constrained. The bug (as far as I can tell) doesn't lead to erroneous results, but does cause more partition ranges to be created, which results in more memory overhead and compute overhead to compute the pruning. Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 --- M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java M src/kudu/common/partial_row.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/common/scan_spec.cc 7 files changed, 535 insertions(+), 339 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/5677/8 -- To view, visit http://gerrit.cloudera.org:8080/5677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add Google Breakpad support to Kudu
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5620 to look at the new patch set (#30). Change subject: Add Google Breakpad support to Kudu .. Add Google Breakpad support to Kudu Breakpad creates minidumps upon crash, which are small files that include stack-based information for each thread. These do not include heap information, so they complement, not replace, core files for debugging purposes. This patch enables Google Breakpad for the kudu-tserver and kudu-master processes. Breakpad is part of the crash-reporting infrastructure open-sourced by Google. It is also used by Mozilla Firefox and Google Chrome. When a process crashes, Breakpad writes a small dump file called a minidump to disk. Since these files are typically only a few MB in size, this allows users to share critical crash information with developers for offline analysis. For more information, see the breakpad getting started docs at http://chromium.googlesource.com/breakpad/breakpad/+/master/docs/getting_started_with_breakpad.md By default, on crash, minidump files are written to a subdirectory of the log directory for a given Kudu daemon process. This was chosen for the following reasons: 1. The minidump files are relatively small, potentially comparable in size to log files, and the log directory is a place an administrator would look in when debugging. 2. This convention is consistent with what Apache Impala (incubating) does for its minidump files. Changes in this patch: * Add breakpad to thirdparty. * Add breakpad source archive creation script (Breakpad does not do releases). * Add a "hack" to the breakpad thirdparty installation routine to forcibly add a breakpad/ prefix to the breakpad header files. This frees us from having to modify our include path when building. * Pull in (heavily modified) util/minidump.{cc,h} from Apache Impala (incubating). The modifications consist of removing use of boost::filesystem, conformance to the Kudu code style guidelines, and some significant refactoring. * Enable breakpad support for the kudu-tserver and kudu-master processes. * By default, invoke previously-installed signal handler if installed. * Handle SIGUSR1 by safely using sigwait() to create minidumps on demand. * Add #ifdefs and CMake logic for Linux. * Add test for all the deadly signals to ensure we get both a stack trace and a minidump, except for the case of SIGTERM, where we should not get a minidump (by design). * Change a few library unit tests that relied on SIGUSR1 to use another signal for their testing, such as SIGHUP or SIGUSR2. * Remove unused includes from mini_tablet_server.cc * Clean up signal handling logic when forking a subprocess. * Hide breakpad symbols from libkuduclient.so * Install failure function that simply calls abort() instead of first printing a stacktrace. Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f --- M CMakeLists.txt M build-support/jenkins/build-and-test.sh A cmake_modules/FindBreakpadClient.cmake M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java M src/kudu/client/client_samples-test.sh M src/kudu/client/symbols.map M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h A src/kudu/integration-tests/minidump_generation-itest.cc M src/kudu/master/mini_master.cc M src/kudu/server/server_base.cc M src/kudu/server/server_base.h M src/kudu/tserver/mini_tablet_server.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/debug-util-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/logging.cc A src/kudu/util/minidump-test.cc A src/kudu/util/minidump.cc A src/kudu/util/minidump.h M src/kudu/util/subprocess-test.cc M src/kudu/util/subprocess.cc M src/kudu/util/test_main.cc M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh M thirdparty/preflight.py A thirdparty/scripts/make-breakpad-src-archive.sh M thirdparty/vars.sh 30 files changed, 994 insertions(+), 84 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5620/30 -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 30 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add Google Breakpad support to Kudu
Mike Percy has posted comments on this change. Change subject: Add Google Breakpad support to Kudu .. Patch Set 27: (3 comments) http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/client/symbols.map File src/kudu/client/symbols.map: Line 26: # breakpad > Can we wildcard some of these? Maybe ConvertUTF* and my_*? Done http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/util/minidump.cc File src/kudu/util/minidump.cc: Line 217: google::InstallFailureFunction(&AbortFailureFunction); > Do you even need the redirection through AbortFailureFunction? abort() has Yeah, just passing abort works, but I thought it was helpful to have it jump through AbortFailureFunction so it would be obvious that it was triggered through this code path. http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/util/subprocess.cc File src/kudu/util/subprocess.cc: Line 432: ResetSigPipeHandlerToDefault(); > Why do we only reset SIGPIPE to SIG_DFL and not every signal? What about SI I added a short comment, but it's for the following reasons: 1. The shell or parent process invoking Kudu may set the signal disposition to ignore and they might know better than us what they want, e.g. for HUP. 2. As far as I could find, we only explicitly set the signal disposition to IGN for PIPE; we don't ignore any other signals. 3. Setting the signal handler to DFL is only necessary if it was previously set to IGN; if it was set to be caught then execve() automatically sets it to DFL. -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 27 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Make SecureRpcHelper a Netty pipeline stage
Dan Burkert has posted comments on this change. Change subject: Make SecureRpcHelper a Netty pipeline stage .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5927/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java: Line 127: RpcHeader.NegotiatePB response = parseSaslMsgResponse(buf); This needs to check for an error response, or maybe punt and add a TODO. -- To view, visit http://gerrit.cloudera.org:8080/5927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] java: use a netty frame decoder instead of replaying decoder
Dan Burkert has posted comments on this change. Change subject: java: use a netty frame decoder instead of replaying decoder .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java: Line 78:* The Hadoop RPC protocol doesn't do any checksumming as they probably For my own edification: is this comment implying that the protocol should have included a CRC of the length immediately following the length bytes? I think you would still want this check in order to limit the damage of a bad actor. -- To view, visit http://gerrit.cloudera.org:8080/5926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] WIP [security] tailored TokenSigner for system catalog
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/5930 Change subject: WIP [security] tailored TokenSigner for system catalog .. WIP [security] tailored TokenSigner for system catalog Updated the TokenSigner class in preparation to loading public part of token signing keys from the system catalog. The expected use-case for the TokenSigner is calling AddTokenSigningPublicKey() multiple times while loading public part of TSKs from the system catalog and subsequent call of Init(). That's the sequence to be exercised by a master server upon becoming a leader. It's possible to run this sequence multiple times on the same instance of TokenSigner, generating new signing keys only when already existing signing keys are about to expire. Also, the TokenSigner class now embeds the logic to rotate its signing key on Init(), if necessary. Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c --- M src/kudu/security/token-test.cc M src/kudu/security/token_signer.cc M src/kudu/security/token_signer.h M src/kudu/security/token_signing_key.cc M src/kudu/security/token_signing_key.h 5 files changed, 293 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/5930/1 -- To view, visit http://gerrit.cloudera.org:8080/5930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-1856: always truncate containers when they get full
Mike Percy has posted comments on this change. Change subject: KUDU-1856: always truncate containers when they get full .. Patch Set 3: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/5852/3/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 137: CHECK_EQ(mode, 0); nit: expected comes before actual in CHECK_EQ() -- To view, visit http://gerrit.cloudera.org:8080/5852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic959c59489a08f92efa2df5c85b22e56740f1346 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] java: remove unused parts of SecureRpcHelper
Dan Burkert has posted comments on this change. Change subject: java: remove unused parts of SecureRpcHelper .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5925/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java: Line 141 We're going to need unwrap to verify channel bindings. -- To view, visit http://gerrit.cloudera.org:8080/5925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] WIP: fixes for java client kerberos against a real cluster
Dan Burkert has posted comments on this change. Change subject: WIP: fixes for java client kerberos against a real cluster .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: PS2, Line 1834: " use {} substitution Line 1850: options.put("debug", "" + Boolean.getBoolean("kudu.jaas.debug")); Boolean.toString(Boolean.getBoolean("kudu.jaas.debug")) Line 1871: LOG.debug("Logged in as subject: " + subject.toString()); {} substitution http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java: Line 116: if (saslClient == null || !saslClient.isComplete() || negoUnderway) { Looks like this could this be simplified to just 'if (negoUnderway)' Line 206: for (RpcHeader.NegotiatePB.SaslAuth auth : response.getAuthsList()) { This shouldn't be done in a loop. The Java client should intersect the server-supported mechs with it's own, and then preferentially pick from that set (GSSAPI over PLAIN). See C++ client. https://github.com/apache/kudu/blob/master/src/kudu/rpc/client_negotiation.cc#L329-L377 Line 270: if (e instanceof SaslException) { This would be simpler as a separate catch clause. } catch (RuntimeException|SaslException e) { throw e; } catch (Exception e) { throw new RuntimeException(e); } -- To view, visit http://gerrit.cloudera.org:8080/5922 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] compaction: Add additional validation in DeltaTracker
Mike Percy has submitted this change and it was merged. Change subject: compaction: Add additional validation in DeltaTracker .. compaction: Add additional validation in DeltaTracker This adds DEBUG-mode validation to AtomicUpdateStores() in the DeltaTracker. It also updates the doc comment on the method. Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a Reviewed-on: http://gerrit.cloudera.org:8080/5919 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans Reviewed-by: David Ribeiro Alves --- M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltafile.h M src/kudu/util/status.h 5 files changed, 87 insertions(+), 16 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot
[kudu-CR] compaction: Flush tablet metadata before updating stores
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5920 to look at the new patch set (#3). Change subject: compaction: Flush tablet metadata before updating stores .. compaction: Flush tablet metadata before updating stores We should always flush the rowset metadata before making changes visible to the delta tracker. In this case, because we are just doing a compaction, it is likely not a crash consistency issue, however we should always follow the same order of operations when operating on tablet metadata. This change also allows us to defer a crash to a point higher in the call stack. Also, add LogPrefix() for the log lines printed from the delta tracker. Change-Id: I958f4744f1b95c6a63e8c9105bd1c7552e43e6b9 --- M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h 2 files changed, 60 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5920/3 -- To view, visit http://gerrit.cloudera.org:8080/5920 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I958f4744f1b95c6a63e8c9105bd1c7552e43e6b9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [thirdparty] Make Boost a regular dependency
Mike Percy has posted comments on this change. Change subject: [thirdparty] Make Boost a regular dependency .. Patch Set 10: Code-Review+1 Ah, nice to see you found the magical incantation for jam. :) -- To view, visit http://gerrit.cloudera.org:8080/5818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates
Jean-Daniel Cryans has posted comments on this change. Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list predicates .. Patch Set 7: (2 comments) Two nits and please add a comment regarding string allocations in PartitionPruner as we discussed elsewhere. http://gerrit.cloudera.org:8080/#/c/5677/7/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java: PS7, Line 51: . nit: remove PS7, Line 488: . nit -- To view, visit http://gerrit.cloudera.org:8080/5677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] compaction: Flush tablet metadata before updating stores
David Ribeiro Alves has posted comments on this change. Change subject: compaction: Flush tablet metadata before updating stores .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/5920/1/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: PS1, Line 281: TODO(mpercy): Is there ever a case where this could cause dangling : // references in the in-memory portion? Do we need to make this more atomic? would it be possible to add a test for this? PS1, Line 285: UNDO fishy... no need to use 'type'? PS1, Line 324: LOG(INFO) << "Opening delta block for read: " << new_block_id.ToString(); without tablet info this isn't likely valuable at info level. -- To view, visit http://gerrit.cloudera.org:8080/5920 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I958f4744f1b95c6a63e8c9105bd1c7552e43e6b9 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] compaction: Flush tablet metadata before updating stores
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5920 to look at the new patch set (#2). Change subject: compaction: Flush tablet metadata before updating stores .. compaction: Flush tablet metadata before updating stores We should always flush the rowset metadata before making changes visible to the delta tracker. In this case, because we are just doing a compaction, it is likely not a crash consistency issue, however we should always follow the same order of operations when operating on tablet metadata. This change also allows us to defer a crash to a point higher in the call stack. Also, add LogPrefix() to the log lines in the delta tracker. Change-Id: I958f4744f1b95c6a63e8c9105bd1c7552e43e6b9 --- M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h 2 files changed, 56 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/5920/2 -- To view, visit http://gerrit.cloudera.org:8080/5920 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I958f4744f1b95c6a63e8c9105bd1c7552e43e6b9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] WIP: enable kerberos support in CSD
Dan Burkert has posted comments on this change. Change subject: WIP: enable kerberos support in CSD .. Patch Set 1: I think we should hold off on committing this until we review all of the new security flags comprehensively. -- To view, visit http://gerrit.cloudera.org:8080/5921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea2d36b7a0da38cd6ddd646a32f52517cabeb4a6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] client: trust master's cert, adopt authn token
Dan Burkert has posted comments on this change. Change subject: client: trust master's cert, adopt authn token .. Patch Set 7: Code-Review+2 restarted verify, I think it was fallout from the workspace issues. -- To view, visit http://gerrit.cloudera.org:8080/5899 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] compaction: Add additional validation in DeltaTracker
David Ribeiro Alves has posted comments on this change. Change subject: compaction: Add additional validation in DeltaTracker .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] tablet: Include peer uuid in log prefix
Jean-Daniel Cryans has posted comments on this change. Change subject: tablet: Include peer uuid in log prefix .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I21c8f568ab3e05cc9e9376f391477c5c35336983 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] tablet: Include peer uuid in log prefix
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: tablet: Include peer uuid in log prefix .. tablet: Include peer uuid in log prefix Also, add LogPrefix() to the tablet mm op. Change-Id: I21c8f568ab3e05cc9e9376f391477c5c35336983 Reviewed-on: http://gerrit.cloudera.org:8080/5918 Tested-by: Kudu Jenkins Reviewed-by: Jean-Daniel Cryans --- M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_mm_ops.cc M src/kudu/tablet/tablet_mm_ops.h 4 files changed, 43 insertions(+), 25 deletions(-) Approvals: Jean-Daniel Cryans: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I21c8f568ab3e05cc9e9376f391477c5c35336983 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] compaction: Add additional validation in DeltaTracker
Jean-Daniel Cryans has posted comments on this change. Change subject: compaction: Add additional validation in DeltaTracker .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05fed515bc5d6a09484fadb61e73ac1346f6654a Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] Fix TSAN segfault on Centos 6
Dan Burkert has submitted this change and it was merged. Change subject: Fix TSAN segfault on Centos 6 .. Fix TSAN segfault on Centos 6 The OpenSSL version shipped with Centos 6 appears to segfault if a nullptr is passed to OBJ_find_sigid_algs with TSAN. I haven't been able to actually figure out why, I haven't been succesful in finding the Centos 6 source for OpenSSL, and the version it's based on correctly checks for null[1]. Attempting to set a breakpoint on the function yielded no source, even with the debuginfo OpenSSL rpm installed. [1] https://github.com/openssl/openssl/blob/OpenSSL_1_0_1e/crypto/objects/obj_xref.c#L115 Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5 Reviewed-on: http://gerrit.cloudera.org:8080/5924 Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins --- M src/kudu/security/cert.cc 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] client: trust master's cert, adopt authn token
Alexey Serbin has posted comments on this change. Change subject: client: trust master's cert, adopt authn token .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5899 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] client: trust master's cert, adopt authn token
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5899 to look at the new patch set (#7). Change subject: client: trust master's cert, adopt authn token .. client: trust master's cert, adopt authn token This changes the client to actually act on the security-related data coming back from the ConnectToMaster() RPC. The client now adds the master's CA cert to its trusted list, and adopts the authentication token in a class member. The token isn't used yet -- that's waiting on some concurrent work by Dan to use tokens during RPC negotiation. But, a unit test verifies that the token is getting set. Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05 --- 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/master/master.proto M src/kudu/rpc/messenger.h M src/kudu/security/tls_context.cc M src/kudu/security/tls_context.h 8 files changed, 79 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/5899/7 -- To view, visit http://gerrit.cloudera.org:8080/5899 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] client: trust master's cert, adopt authn token
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5899 to look at the new patch set (#6). Change subject: client: trust master's cert, adopt authn token .. client: trust master's cert, adopt authn token This changes the client to actually act on the security-related data coming back from the ConnectToMaster() RPC. The client now adds the master's CA cert to its trusted list, and adopts the authentication token in a class member. The token isn't used yet -- that's waiting on some concurrent work by Dan to use tokens during RPC negotiation. But, a unit test verifies that the token is getting set. Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05 --- 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/master/master.proto M src/kudu/rpc/messenger.h M src/kudu/security/tls_context.cc M src/kudu/security/tls_context.h 8 files changed, 79 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/5899/6 -- To view, visit http://gerrit.cloudera.org:8080/5899 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaa3eec178110b77d81608cfdce989e0fe494ee05 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] master: issue authentication tokens and CA certs to clients
Todd Lipcon has submitted this change and it was merged. Change subject: master: issue authentication tokens and CA certs to clients .. master: issue authentication tokens and CA certs to clients This adds the code to the ConnectToMaster RPC to issue an authentication token for the connecting user, as well as to send it the current master CA cert. The client side currently ignores both things, but a new unit test verifies that they are getting properly set. As with most of these early integration patches, there are plenty of TODOs about integrating with the catalog manager, etc, but this should be enough to unblock other client-side work of propagating the CA and tokens into the RPC system. Change-Id: I5969b8e125633b3b14364b98c0d0a992b162f302 Reviewed-on: http://gerrit.cloudera.org:8080/5871 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/master/CMakeLists.txt A src/kudu/master/authn_token_manager.cc A src/kudu/master/authn_token_manager.h M src/kudu/master/master-test.cc M src/kudu/master/master.cc M src/kudu/master/master.h M src/kudu/master/master.proto M src/kudu/master/master_cert_authority.cc M src/kudu/master/master_cert_authority.h M src/kudu/master/master_service.cc M src/kudu/rpc/outbound_call.h M src/kudu/security/token.proto 12 files changed, 256 insertions(+), 3 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5969b8e125633b3b14364b98c0d0a992b162f302 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] master rpc: pass back more details from ConnectToCluster
Todd Lipcon has submitted this change and it was merged. Change subject: master_rpc: pass back more details from ConnectToCluster .. master_rpc: pass back more details from ConnectToCluster This plumbs back the ConnectToMasterResponsePB from the ConnectToCluster RPC. This brings us one step closer to inserting the CA cert into the TlsContext and passing the token somewhere useful. Along the way, I switched to std::function instead of gutil Callback since it allows the slightly easier lambda syntax. No new tests here since all the existing client tests cover this code path. Change-Id: I3b5056c9f31237249516a2e7ae68d641c9f6bd02 Reviewed-on: http://gerrit.cloudera.org:8080/5892 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/integration-tests/external_mini_cluster.cc 5 files changed, 63 insertions(+), 56 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5892 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3b5056c9f31237249516a2e7ae68d641c9f6bd02 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] master rpc: pass back more details from ConnectToCluster
Alexey Serbin has posted comments on this change. Change subject: master_rpc: pass back more details from ConnectToCluster .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5892 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b5056c9f31237249516a2e7ae68d641c9f6bd02 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] master: issue authentication tokens and CA certs to clients
Alexey Serbin has posted comments on this change. Change subject: master: issue authentication tokens and CA certs to clients .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5871 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5969b8e125633b3b14364b98c0d0a992b162f302 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [thirdparty] Make Boost a regular dependency
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5818 to look at the new patch set (#10). Change subject: [thirdparty] Make Boost a regular dependency .. [thirdparty] Make Boost a regular dependency For the new TIMESTAMP type we must mimic Impala's TimestampValue format to make sure that the types have the same memory format. This will allow to avoid costly conversions at scan time. Impala's type uses boost's ptime and nanosecond resolution and so must Kudu. Previously our use of boost was header-only, but the date_time library (which includes ptime) needs to be compiled. This requires that we actually compile libboost_date_time in thirparty. This patch does that and also changes the way we handle boost to treat it more like a regular dependency, both regarding the new static and shared library files and regarding headers. Since boost uses its own build system this required tinkering with jamfiles to make sure that the library is correctly built under tsan. Finally this patch also addresses a couple of other issues: - Adds "thirdparty/installed/common/include" to the include dirs explicitly (we were doing this indirectly when we added the boost include dir before). - Since FindKuduBoost.cmake now explicitly sets BOOST_INCLUDE_DIR to point to wherever boost was installed, this removes the logic that makes sure we didn't pick up boost from the system path. Change-Id: I277c4fda15575e271c426735552762884cb28c43 --- M CMakeLists.txt A cmake_modules/FindKuduBoost.cmake M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh 4 files changed, 124 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/5818/10 -- To view, visit http://gerrit.cloudera.org:8080/5818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [thirdparty] Make Boost a regular dependency
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5818 to look at the new patch set (#9). Change subject: [thirdparty] Make Boost a regular dependency .. [thirdparty] Make Boost a regular dependency For the new TIMESTAMP type we must mimic Impala's TimestampValue format to make sure that the types have the same memory format. This will allow to avoid costly conversions at scan time. Impala's type uses boost's ptime and nanosecond resolution and so must Kudu. Previously our use of boost was header-only, but the date_time library (which includes ptime) needs to be compiled. This requires that we actually compile libboost_date_time in thirparty. This patch does that and also changes the way we handle boost to treat it more like a regular dependency, both regarding the new static and shared library files and regarding headers. Since boost uses its own build system this required tinkering with jamfiles to make sure that the library is correctly built under tsan. Finally this patch also addresses a couple of other issues: - Adds "thirdparty/installed/common/include" to the include dirs explicitly (we were doing this indirectly when we added the boost include dir before). - Since FindKuduBoost.cmake now explicitly sets BOOST_INCLUDE_DIR to point to wherever boost was installed, this removes the logic that makes sure we didn't pick up boost from the system path. Change-Id: I277c4fda15575e271c426735552762884cb28c43 --- M CMakeLists.txt A cmake_modules/FindKuduBoost.cmake M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh 4 files changed, 125 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/5818/9 -- To view, visit http://gerrit.cloudera.org:8080/5818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [thirdparty] Make Boost a regular dependency
David Ribeiro Alves has posted comments on this change. Change subject: [thirdparty] Make Boost a regular dependency .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/5818/5//COMMIT_MSG Commit Message: PS5, Line 9: However the : new timestamp type makes extensive use of boost's date_time : and, as such, we need to compile at least libboostdate_time. > Makes sense. Could you add this information to the commit message? Done http://gerrit.cloudera.org:8080/#/c/5818/8/cmake_modules/FindKuduBoost.cmake File cmake_modules/FindKuduBoost.cmake: PS8, Line 18: BOOST_DATE_TIME - libboost_date_time.a, libboost_date_time.so, : # and libboost_date_time.so.1 > Nit: this will be an onerous list to maintain once other boost components a Done -- To view, visit http://gerrit.cloudera.org:8080/5818 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I277c4fda15575e271c426735552762884cb28c43 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Make SecureRpcHelper a Netty pipeline stage
Jean-Daniel Cryans has posted comments on this change. Change subject: Make SecureRpcHelper a Netty pipeline stage .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5927/1/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java: Line 113: RED And same farther below. -- To view, visit http://gerrit.cloudera.org:8080/5927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] java: use a netty frame decoder instead of replaying decoder
Jean-Daniel Cryans has posted comments on this change. Change subject: java: use a netty frame decoder instead of replaying decoder .. Patch Set 1: (2 comments) Good on you for addressing this. I'd be curious to see how it improves performance. http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java: Line 72: I see red. http://gerrit.cloudera.org:8080/#/c/5926/1/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java: Line 174: // TODO(todd): shouldn't this return here? Yes. -- To view, visit http://gerrit.cloudera.org:8080/5926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] java: remove unused parts of SecureRpcHelper
Jean-Daniel Cryans has posted comments on this change. Change subject: java: remove unused parts of SecureRpcHelper .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] WIP: fixes for java client kerberos against a real cluster
Jean-Daniel Cryans has posted comments on this change. Change subject: WIP: fixes for java client kerberos against a real cluster .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: Line 1830: private static Subject getSubjectOrLogin() { Seems like this could be extracted into some helper class. Line 1840: @Override Pretty sure we normally use double indentation for anonymous classes. PS2, Line 1874: c Could* http://gerrit.cloudera.org:8080/#/c/5922/2/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java: Line 85: public InetAddress getResolvedAddress() { nit: javadoc -- To view, visit http://gerrit.cloudera.org:8080/5922 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6b96fad3cfb40500d7a75e5070ea21bc8e00cbd8 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Make SecureRpcHelper a Netty pipeline stage
Hello Dan Burkert, Jean-Daniel Cryans, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/5927 to review the following change. Change subject: Make SecureRpcHelper a Netty pipeline stage .. Make SecureRpcHelper a Netty pipeline stage This decouples SecureRpcHelper from TabletClient. When negotiation is complete, it sends an "event" upstream to the TabletClient and removes itself from the pipeline. I'm hoping this decoupling will make it easier to test SecureRpcHelper in isolation as we add more functionality like TLS negotiation. After this is committed, I'm also hoping to rename SecureRpcHelper to Negotiator. Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b --- M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java 2 files changed, 85 insertions(+), 76 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/5927/1 -- To view, visit http://gerrit.cloudera.org:8080/5927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5f4b4516219b8eebf24786b4ceb13f2e6260f03b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] java: use a netty frame decoder instead of replaying decoder
Hello Dan Burkert, Jean-Daniel Cryans, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/5926 to review the following change. Change subject: java: use a netty frame decoder instead of replaying decoder .. java: use a netty frame decoder instead of replaying decoder All of our inbound packets are length-prefixed. Netty provides a nice class for handing length-prefixed messages. This avoids a bunch of more custom logic we were doing for length checking, etc. Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f --- M java/kudu-client/src/main/java/org/apache/kudu/client/CallResponse.java M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java 5 files changed, 48 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/5926/1 -- To view, visit http://gerrit.cloudera.org:8080/5926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1ec1e6f9cbbdf694fe85b7668a3c349efc26c08f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] java: remove unused parts of SecureRpcHelper
Hello Dan Burkert, Jean-Daniel Cryans, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/5925 to review the following change. Change subject: java: remove unused parts of SecureRpcHelper .. java: remove unused parts of SecureRpcHelper This removes the unused wrap/unwrap code from SecureRpcHelper. I also removed an unnecessary function in TabletClient. Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df --- M java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java 2 files changed, 3 insertions(+), 55 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/5925/1 -- To view, visit http://gerrit.cloudera.org:8080/5925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3b70e0319060e3f2d9cc9a0ed23da7fe51b174df Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] Add KuduTable::formatted range partitions in cpp client.
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5893 to look at the new patch set (#3). Change subject: Add KuduTable::formatted_range_partitions in cpp client. .. Add KuduTable::formatted_range_partitions in cpp client. This add the KuduTable::formatted_range_partitions method in cpp client to retrive the list of range partitions in a table. Change-Id: I7a379fb29932c4160724fe8c71f85f2c8e994469 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h 5 files changed, 167 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/5893/3 -- To view, visit http://gerrit.cloudera.org:8080/5893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7a379fb29932c4160724fe8c71f85f2c8e994469 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: zhen.zhang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add KuduTable::formatted range partitions in cpp client.
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5893 to look at the new patch set (#2). Change subject: Add KuduTable::formatted_range_partitions in cpp client. .. Add KuduTable::formatted_range_partitions in cpp client. This add the KuduTable::formatted_range_partitions method in cpp client to retrive the list of range partitions in a table. Change-Id: I7a379fb29932c4160724fe8c71f85f2c8e994469 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h 5 files changed, 167 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/5893/2 -- To view, visit http://gerrit.cloudera.org:8080/5893 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7a379fb29932c4160724fe8c71f85f2c8e994469 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: zhen.zhang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates
Dan Burkert has posted comments on this change. Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list predicates .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/5677/2/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java: PS2, Line 489: > indentation Done http://gerrit.cloudera.org:8080/#/c/5677/6/src/kudu/common/partition_pruner-test.cc File src/kudu/common/partition_pruner-test.cc: Line 51:const vector& partitions, > warning: the const qualified parameter 'partitions' is copied for each invo Done -- To view, visit http://gerrit.cloudera.org:8080/5677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [java client] KUDU-1643 Prune hash partitions based on IN-list predicates
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5677 to look at the new patch set (#7). Change subject: [java client] KUDU-1643 Prune hash partitions based on IN-list predicates .. [java client] KUDU-1643 Prune hash partitions based on IN-list predicates PartitionPruner is updated to search all combinations of in-list column values and prune hash partitions where possible. This also fixes a small issue in the C++ version of the algorithm: previously the implementation would always consider the final hash component to be constrained. The bug (as far as I can tell) doesn't lead to erroneous results, but does cause more partition ranges to be created, which results in more memory overhead and compute overhead to compute the pruning. Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 --- M java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java M src/kudu/common/partial_row.h M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/partition_pruner.h M src/kudu/common/scan_spec.cc 7 files changed, 520 insertions(+), 332 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/5677/7 -- To view, visit http://gerrit.cloudera.org:8080/5677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8a793c23ff00d19b3d3d062bb222d2c725a93724 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Haijie Hong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fix TSAN segfault on Centos 6
Todd Lipcon has posted comments on this change. Change subject: Fix TSAN segfault on Centos 6 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Fix TSAN segfault on Centos 6
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/5924 to review the following change. Change subject: Fix TSAN segfault on Centos 6 .. Fix TSAN segfault on Centos 6 The OpenSSL version shipped with Centos 6 appears to segfault if a nullptr is passed to OBJ_find_sigid_algs with TSAN. I haven't been able to actually figure out why, I haven't been succesful in finding the Centos 6 source for OpenSSL, and the version it's based on correctly checks for null[1]. Attempting to set a breakpoint on the function yielded no source, even with the debuginfo OpenSSL rpm installed. [1] https://github.com/openssl/openssl/blob/OpenSSL_1_0_1e/crypto/objects/obj_xref.c#L115 Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5 --- M src/kudu/security/cert.cc 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/5924/1 -- To view, visit http://gerrit.cloudera.org:8080/5924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5ea3ad372f98ff423425858c09cb0977f372b2e5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Todd Lipcon