[kudu-CR] bitshuffle: stop using uninitialized data as padding
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15647 ) Change subject: bitshuffle: stop using uninitialized data as padding .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/15647/2/src/kudu/cfile/bshuf_block.h File src/kudu/cfile/bshuf_block.h: http://gerrit.cloudera.org:8080/#/c/15647/2/src/kudu/cfile/bshuf_block.h@186 PS2, Line 186: With this fix the data_ buffer will indeed be multiple of 8 unlike earlier unless kHeaderSize happens to be multiple of 8 as well. DCHECK on data_.size() will be good. -- To view, visit http://gerrit.cloudera.org:8080/15647 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Gerrit-Change-Number: 15647 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 04 Apr 2020 05:49:12 + Gerrit-HasComments: Yes
[kudu-CR] scripts: fix benchmark parsing for wire protocol-test
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15651 ) Change subject: scripts: fix benchmark parsing for wire_protocol-test .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/15651/1/src/kudu/scripts/benchmarks.sh File src/kudu/scripts/benchmarks.sh: http://gerrit.cloudera.org:8080/#/c/15651/1/src/kudu/scripts/benchmarks.sh@221 PS1, Line 221: 1 Nit: 100 percent? -- To view, visit http://gerrit.cloudera.org:8080/15651 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d1ac45cf730674a4aa3b7d1f7f60b6919e3eeb1 Gerrit-Change-Number: 15651 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 04 Apr 2020 05:38:36 + Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: ranger: enable log4j2 logging to files .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 10 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 05:23:10 + Gerrit-HasComments: No
[kudu-CR] scripts: fix benchmark parsing for wire protocol-test
Hello Adar Dembo, Bankim Bhavsar, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15651 to review the following change. Change subject: scripts: fix benchmark parsing for wire_protocol-test .. scripts: fix benchmark parsing for wire_protocol-test This fixes the existing parsing for row-wise conversion, and adds parsing for the new columnar conversion, writing the columnar stats as a new workload "WireProtocolBenchmark_columnar" and plotting results as "wire-protocol-test_columnar". Change-Id: I9d1ac45cf730674a4aa3b7d1f7f60b6919e3eeb1 --- M src/kudu/scripts/benchmarks.sh 1 file changed, 16 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/15651/1 -- To view, visit http://gerrit.cloudera.org:8080/15651 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9d1ac45cf730674a4aa3b7d1f7f60b6919e3eeb1 Gerrit-Change-Number: 15651 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
waleed.fat...@gmail.com has abandoned this change. ( http://gerrit.cloudera.org:8080/15646 ) Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. Abandoned Just added patch to change 15638 which has Grant's review comments. -- To view, visit http://gerrit.cloudera.org:8080/15646 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I7014c3c4eac0f622afb0508cd97226b1d32fb904 Gerrit-Change-Number: 15646 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15638 to look at the new patch set (#2). Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore The System.exit() calls have a side effect that can cause Spark to fail even if the run function returns 0 on success. Rather than call System.exit() the run() method will return true on a successful run. We then throw a RuntimeException() in main if we find that run() failed, otherwise we call SparkSession's stop() method to cleanly shutdown Spark. Unfortunately the issue isn't easy to reproduce but we had one environment exhibiting the problem and we confirmed that this patch fixes the issue. TestKuduBackup.scala was modified where assertFalse() is used to check for failure and assertTrue() for success. Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 3 files changed, 36 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/15638/2 -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] ranger: allow overwriting of the log4j2 properties file
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15650 Change subject: ranger: allow overwriting of the log4j2 properties file .. ranger: allow overwriting of the log4j2 properties file It seems important for users to have the ability to have subsequent runs of the master honor any new logging configurations specified via gflag. As such, it seems important to allow users to regenerate the log4j2 properties file used by the Ranger client, even if one exists. This patch enables this by introducing the --ranger_overwrite_log_config gflag, which is set to true by default. Change-Id: I4a06f8a1b3328cfd4029295527b5ba61a03efbfa --- M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc 2 files changed, 109 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/50/15650/1 -- To view, visit http://gerrit.cloudera.org:8080/15650 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4a06f8a1b3328cfd4029295527b5ba61a03efbfa Gerrit-Change-Number: 15650 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] WIP: MSAN support
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15649 to review the following change. Change subject: WIP: MSAN support .. WIP: MSAN support WIP because I can't seem to get suppression of third party code to work, and this is a complete non-starter otherwise (i.e. too many unsafe accesses in libunwind, libsasl, and openssl for starters). Change-Id: Iff02896fa7e3adf2a920214d8bc3d040226ebe6f --- M CMakeLists.txt M cmake_modules/KuduLinker.cmake M src/kudu/client/CMakeLists.txt M src/kudu/fs/block_manager-test.cc M src/kudu/fs/log_block_manager-test.cc M src/kudu/gutil/dynamic_annotations.h M src/kudu/gutil/port.h M src/kudu/util/CMakeLists.txt M src/kudu/util/bitmap-test.cc M src/kudu/util/crc-test.cc M src/kudu/util/env-test.cc M src/kudu/util/env_posix.cc M src/kudu/util/pb_util-test.cc M src/kudu/util/sanitizer_options.cc M thirdparty/build-definitions.sh M thirdparty/build-if-necessary.sh M thirdparty/build-thirdparty.sh M thirdparty/vars.sh 18 files changed, 339 insertions(+), 54 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/15649/1 -- To view, visit http://gerrit.cloudera.org:8080/15649 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iff02896fa7e3adf2a920214d8bc3d040226ebe6f Gerrit-Change-Number: 15649 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] bitshuffle: stop using uninitialized data as padding
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15647 ) Change subject: bitshuffle: stop using uninitialized data as padding .. bitshuffle: stop using uninitialized data as padding The resize() incorrectly accounted for a block header that was never actually written to data_. The result was that added padding was actually kHeaderSize bytes "to the right", and the call to compress_lz4() read uninitialized data from this part of data_ rather than the added padding. What's the effect? Up to padding_bytes of uninitialized data gets bitshuffled, compressed, and written to the block. At read time, it is decompressed, debitshuffled, but ultimately ignored, as it was expected to be just padded zeroes. I observed this in cfile-test's TestMetadata built with MSAN instrumentation, but oddly enough not in any other test in cfile-test, even though others use bitshuffle and padding. Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Reviewed-on: http://gerrit.cloudera.org:8080/15647 Reviewed-by: Andrew Wong Tested-by: Kudu Jenkins --- M src/kudu/cfile/bshuf_block.h 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Andrew Wong: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/15647 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Gerrit-Change-Number: 15647 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [python] KUDU-2632 Add DATE type support
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15645 ) Change subject: [python] KUDU-2632 Add DATE type support .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/tests/util.py File python/kudu/tests/util.py: http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/tests/util.py@112 PS1, Line 112: datetime.date(2020,1,1) For the sake of covering corner cases and making sure datetime.date() properly maps into the underlying types, does it make sense to add a scenario to write a rows with DATE column values of datetime.date(datetime.MINYEAR, 1, 1) datetime.date(datetime.MAXYEAR, 12, 31) ? -- To view, visit http://gerrit.cloudera.org:8080/15645 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f08946e9ba56dab5e5b43e2bf65bc535c26ab25 Gerrit-Change-Number: 15645 Gerrit-PatchSet: 1 Gerrit-Owner: Volodymyr Verovkin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 04 Apr 2020 01:21:10 + Gerrit-HasComments: Yes
[kudu-CR] [python] KUDU-2632 Add DATE type support
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15645 ) Change subject: [python] KUDU-2632 Add DATE type support .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/tests/test_scanner.py File python/kudu/tests/test_scanner.py: http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/tests/test_scanner.py@349 PS1, Line 349: self.assertEqual(types[0], np.int64) : self.assertEqual(types[1], 'datetime64[ns, UTC]') : self.assertEqual(types[2], np.object) : self.assertEqual(types[3], np.object) : self.assertEqual(types[4], np.bool) : self.assertEqual(types[5], np.float64) : self.assertEqual(types[6], np.int8) : self.assertEqual(types[7], np.object) : self.assertEqual(types[8], np.object) : self.assertEqual(types[9], np.float32) : else: : self.assertEqual(types[0], np.int64) : self.assertEqual(types[1], 'datetime64[ns, UTC]') : self.assertEqual(types[2], np.object) : self.assertEqual(types[3], np.bool) : self.assertEqual(types[4], np.float64) : self.assertEqual(types[5], np.int8) : self.assertEqual(types[6], np.object) : self.assertEqual(types[7], np.object) It seems this needs to be updated to accommodate for newly added columns of DATE type. -- To view, visit http://gerrit.cloudera.org:8080/15645 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f08946e9ba56dab5e5b43e2bf65bc535c26ab25 Gerrit-Change-Number: 15645 Gerrit-PatchSet: 1 Gerrit-Owner: Volodymyr Verovkin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:54:09 + Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: ranger: enable log4j2 logging to files .. Patch Set 10: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 10 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:48:19 + Gerrit-HasComments: No
[kudu-CR] ranger: enable log4j2 logging to files
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15628 to look at the new patch set (#10). Change subject: ranger: enable log4j2 logging to files .. ranger: enable log4j2 logging to files This patch enables Kudu to generate a log4j2 properties file and pass it to the Ranger client. This file is created in a directory specified by the new --ranger_logging_config_dir flag, which, by default, will direct the properties file to --log_dir. If the properties file already exists, the existing file is honored. This means a file will not be regenerated if one has already been generated -- I will add a follow up patch to allow users to overwrite the file. This properties file will direct logs to the directory pointed to by --log_dir, and will honor the existing --max_log_files and --max_log_size flags. I've found logging to stdout and adjusting log42j log-level to be useful, so this also adds --ranger_logtostdout and --ranger_log_level for these respectively. I opted not to use the glog --minloglevel flag since they don't quite map to log4j2 log levels (e.g. glog doesn't have debug-level logging). I manually tested that an existing properties file that has been modified will be honored. Here's what's in a Ranger-configured Kudu master's log directory: kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-122527.0.17165 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165 kudu-ranger-subprocess-log4j2.properties kudu-ranger-subprocess.awong-MBP16-21621.local.log Change-Id: I7efa631832c219fce214304538e6ab6442062752 --- M java/gradle/dependencies.gradle M java/kudu-subprocess/build.gradle M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/subprocess/CMakeLists.txt A src/kudu/subprocess/subprocess_proxy.cc M src/kudu/subprocess/subprocess_proxy.h M src/kudu/util/env.h 10 files changed, 376 insertions(+), 65 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/10 -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 10 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. remove last vestiges of boost::bind, boost::function, and std::bind With this patch, all Kudu functors use std::function, and all capturing or binding is done via C++ lambdas. The lack of support for capture-via-move rears its ugly head in raft_consensus.cc, but I verified that this only leads to inefficiency, not incorrectness (i.e. lifecycle issues). C++14 (or 17) support can't come soon enough! Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Reviewed-on: http://gerrit.cloudera.org:8080/15639 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin Reviewed-by: Andrew Wong --- M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.cc M src/kudu/benchmarks/tpch/rpc_line_item_dao.h M src/kudu/benchmarks/tpch/tpch1.cc M src/kudu/benchmarks/tpch/tpch_real_world.cc M src/kudu/cfile/encoding-test.cc M src/kudu/client/client-internal.cc M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client-unittest.cc M src/kudu/client/client.cc M src/kudu/client/master_proxy_rpc.cc M src/kudu/client/master_rpc.cc M src/kudu/client/master_rpc.h M src/kudu/client/session-internal.cc M src/kudu/consensus/consensus-test-util.h M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/raft_consensus.cc M src/kudu/integration-tests/linked_list-test-util.h M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/token_signer-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master_path_handlers.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/proxy.cc M src/kudu/rpc/reactor-test.cc M src/kudu/rpc/reactor.cc M src/kudu/rpc/reactor.h M src/kudu/rpc/response_callback.h M src/kudu/rpc/retriable_rpc.h M src/kudu/rpc/rpc-bench.cc M src/kudu/rpc/rpc-test.cc M src/kudu/rpc/rpc.cc M src/kudu/rpc/rpc.h M src/kudu/rpc/rpc_stub-test.cc M src/kudu/security/init.cc M src/kudu/server/default_path_handlers.cc M src/kudu/server/rpcz-path-handler.cc M src/kudu/server/server_base.cc M src/kudu/server/tracing_path_handlers.cc M src/kudu/server/webserver.cc M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/transactions/transaction_driver.cc M src/kudu/tools/ksck_remote-test.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver_path_handlers.cc M src/kudu/util/debug/trace_event_impl.cc M src/kudu/util/url-coding.cc M src/kudu/util/web_callback_registry.h 57 files changed, 522 insertions(+), 550 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 2 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:43:31 + Gerrit-HasComments: No
[kudu-CR] [python] KUDU-2632 Add DATE type support
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15645 ) Change subject: [python] KUDU-2632 Add DATE type support .. Patch Set 1: (7 comments) Python test kudu.tests.test_scanner.TestScanner.test_scanner_to_pandas_types is failing: http://jenkins.kudu.apache.org/job/kudu-gerrit/21294/BUILD_TYPE=DEBUG/testReport/junit/kudu.tests.test_scanner/TestScanner/test_scanner_to_pandas_types/ http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/util.py File python/kudu/util.py: http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/util.py@186 PS1, Line 186: Converts If following the same style of the docs as for the other functions in this file, this should be 'Convert'. http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/util.py@188 PS1, Line 188: of seconds per day (86400) nit: add a period http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/util.py@189 PS1, Line 189: extra spaces http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/util.py@190 PS1, Line 190: Returns Could you add information about the parameter of this function into the doc header using the same style as in other functions? http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/util.py@198 PS1, Line 198: Converts Convert http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/util.py@199 PS1, Line 199: the number of days since the unix epoch nit: add a period http://gerrit.cloudera.org:8080/#/c/15645/1/python/kudu/util.py@203 PS1, Line 203: timestamp Is this a mismatch between the actual name of the parameter and the documented name? -- To view, visit http://gerrit.cloudera.org:8080/15645 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f08946e9ba56dab5e5b43e2bf65bc535c26ab25 Gerrit-Change-Number: 15645 Gerrit-PatchSet: 1 Gerrit-Owner: Volodymyr Verovkin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:36:53 + Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: ranger: enable log4j2 logging to files .. Patch Set 9: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc File src/kudu/ranger/ranger_client.cc: http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@296 PS8, Line 296: K(WriteStringToFileSync( > That doesn't seem to work (it's not honored by log4j2 at least). Leaving th Oh I misunderstood; I thought this was "our" parameter. -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 9 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:19:17 + Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15628 to look at the new patch set (#9). Change subject: ranger: enable log4j2 logging to files .. ranger: enable log4j2 logging to files This patch enables Kudu to generate a log4j2 properties file and pass it to the Ranger client. This file is created in a directory specified by the new --ranger_logging_config_dir flag, which, by default, will direct the properties file to --log_dir. If the properties file already exists, the existing file is honored. This means a file will not be regenerated if one has already been generated -- I will add a follow up patch to allow users to overwrite the file. This properties file will direct logs to the directory pointed to by --log_dir, and will honor the existing --max_log_files and --max_log_size flags. I've found logging to stdout and adjusting log42j log-level to be useful, so this also adds --ranger_logtostdout and --ranger_log_level for these respectively. I opted not to use the glog --minloglevel flag since they don't quite map to log4j2 log levels (e.g. glog doesn't have debug-level logging). I manually tested that an existing properties file that has been modified will be honored. Here's what's in a Ranger-configured Kudu master's log directory: kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-122527.0.17165 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165 kudu-ranger-subprocess-log4j2.properties kudu-ranger-subprocess.awong-MBP16-21621.local.log Change-Id: I7efa631832c219fce214304538e6ab6442062752 --- M java/gradle/dependencies.gradle M java/kudu-subprocess/build.gradle M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/subprocess/CMakeLists.txt A src/kudu/subprocess/subprocess_proxy.cc M src/kudu/subprocess/subprocess_proxy.h M src/kudu/util/env.h 10 files changed, 365 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/9 -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 9 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
..
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@14
PS8, Line 14: on
> one
Done
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@13
PS8, Line 13: This means a file will not be regenerated
: if on has already been generated
> One downside to this approach is that we can't automatically update log4j2.
Yeah, I will likely default the "overwrite" flag in a later patch to 'true'.
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@17
PS8, Line 17: directory
> Nit: the directory
Done
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@255
PS8, Line 255: if (!env->FileExists(log_conf_dir)) {
: RETURN_NOT_OK(env->CreateDir(log_conf_dir));
: }
> I don't think we should be creating FLAGS_log_dir. Maybe condition this on
I'd rather not add more work for admins, so I'll condition on using an explicit
config dir.
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@296
PS8, Line 296: -Dlog4j2.configurationFile
> Nit: maybe -Dlog4j2.configuration.filename?
That doesn't seem to work (it's not honored by log4j2 at least). Leaving this
as is.
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/subprocess/subprocess_proxy.cc
File src/kudu/subprocess/subprocess_proxy.cc:
PS8:
> We're doing more and more of this "template composition", and it's getting
Yeah, I left a TODO complaining about this. Builder, mustache, whatever is easy
to write would get points in my book.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 04 Apr 2020 00:14:09 +
Gerrit-HasComments: Yes
[kudu-CR] bitshuffle: stop using uninitialized data as padding
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15647 ) Change subject: bitshuffle: stop using uninitialized data as padding .. Patch Set 1: Code-Review+2 Nice find -- To view, visit http://gerrit.cloudera.org:8080/15647 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Gerrit-Change-Number: 15647 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Sat, 04 Apr 2020 00:04:25 + Gerrit-HasComments: No
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@166 PS1, Line 166: void AsyncHandler(ev::async& watcher, int revents); > I think that's already implied in the existing comment, no? "libev callback Indeed, that information can be easily derived from current comment. http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@169 PS1, Line 169: void TimerHandler(ev::timer& watcher, int revents); > See above. Ack. -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 04 Apr 2020 00:02:17 + Gerrit-HasComments: Yes
[kudu-CR] bitshuffle: stop using uninitialized data as padding
Hello Andrew Wong, Bankim Bhavsar, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/15647 to review the following change. Change subject: bitshuffle: stop using uninitialized data as padding .. bitshuffle: stop using uninitialized data as padding The resize() incorrectly accounted for a block header that was never actually written to data_. The result was that added padding was actually kHeaderSize bytes "to the right", and the call to compress_lz4() read uninitialized data from this part of data_ rather than the added padding. What's the effect? Up to padding_bytes of uninitialized data gets bitshuffled, compressed, and written to the block. At read time, it is decompressed, debitshuffled, but ultimately ignored, as it was expected to be just padded zeroes. I observed this in cfile-test's TestMetadata built with MSAN instrumentation, but oddly enough not in any other test in cfile-test, even though others use bitshuffle and padding. Change-Id: I25eba027ba356774173b2313c68436d7baddaddc --- M src/kudu/cfile/bshuf_block.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/15647/1 -- To view, visit http://gerrit.cloudera.org:8080/15647 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I25eba027ba356774173b2313c68436d7baddaddc Gerrit-Change-Number: 15647 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
waleed.fat...@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15646 Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore The System.exit() calls have a side effect that can cause Spark to fail even if the run function returns 0 on success. Rather than call System.exit() the run() method will return true on a successful run. We then throw a RuntimeException() in main if we find that run() failed, otherwise we call SparkSession's stop() method to cleanly shutdown Spark. Unfortunately the issue isn't easy to reproduce but we had one environment exhibiting the problem and we confirmed that this patch fixes the issue. TestKuduBackup.scala was modified where assertFalse() is used to check for failure and assertTrue() for success. Change-Id: I7014c3c4eac0f622afb0508cd97226b1d32fb904 --- M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 3 files changed, 36 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/15646/1 -- To view, visit http://gerrit.cloudera.org:8080/15646 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7014c3c4eac0f622afb0508cd97226b1d32fb904 Gerrit-Change-Number: 15646 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward
[kudu-CR] [python] KUDU-2632 Add DATE type support
Volodymyr Verovkin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15645 Change subject: [python] KUDU-2632 Add DATE type support .. [python] KUDU-2632 Add DATE type support There are two utility functions which perfrom conversion between Kudu DATE and Python datetime.date: unix_epoch_days_to_date() - converts number of days since Unix ecpoch to datetime.date() date_to_unix_epoch_days() - converts datetime.date() to number of days since Unix ecpoch Change-Id: I1f08946e9ba56dab5e5b43e2bf65bc535c26ab25 --- M python/kudu/__init__.py M python/kudu/client.pyx M python/kudu/libkudu_client.pxd M python/kudu/schema.pyx M python/kudu/tests/test_scanner.py M python/kudu/tests/test_scantoken.py M python/kudu/tests/test_schema.py M python/kudu/tests/util.py M python/kudu/util.py 9 files changed, 104 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/15645/1 -- To view, visit http://gerrit.cloudera.org:8080/15645 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1f08946e9ba56dab5e5b43e2bf65bc535c26ab25 Gerrit-Change-Number: 15645 Gerrit-PatchSet: 1 Gerrit-Owner: Volodymyr Verovkin
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15042 ) Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. Patch Set 5: > Patch Set 5: Verified-1 > > Build Failed > > http://jenkins.kudu.apache.org/job/kudu-gerrit/21289/ : FAILURE TSAN test error: TabletServerDiskError/TabletServerDiskErrorITest.TestFailDuringScanWorkload/0 This test doesn't show up as flaky on the dashboard. At the same TSAN build that includes a change on top of this didn't fail https://gerrit.cloudera.org/c/15043/ Looks like some delay but not sure whether that's the reason for failure. I0403 19:02:27.387187 1261 disk_failure-itest.cc:271] Currently has 9 failed tablets /home/jenkins-slave/workspace/kudu-master/1/src/kudu/integration-tests/disk_failure-itest.cc:272: Failure Expected: num_failed Which is: 10 To be equal to: failed_on_ts Which is: 9 /home/jenkins-slave/workspace/kudu-master/1/src/kudu/util/test_util.cc:349: Failure Failed Timed out waiting for assertion to pass. /home/jenkins-slave/workspace/kudu-master/1/src/kudu/integration-tests/disk_failure-itest.cc:313: Failure -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Apr 2020 22:00:21 + Gerrit-HasComments: No
[kudu-CR] ranger: enable log4j2 logging to files
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
..
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@13
PS8, Line 13: This means a file will not be regenerated
: if on has already been generated
One downside to this approach is that we can't automatically update
log4j2.properties in subsequent releases.
I think that's actually more important than providing a mechanism for admins to
supply their own log4j2.properties.
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@14
PS8, Line 14: on
one
http://gerrit.cloudera.org:8080/#/c/15628/8//COMMIT_MSG@17
PS8, Line 17: directory
Nit: the directory
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@255
PS8, Line 255: if (!env->FileExists(log_conf_dir)) {
: RETURN_NOT_OK(env->CreateDir(log_conf_dir));
: }
I don't think we should be creating FLAGS_log_dir. Maybe condition this on
whether we're using FLAGS_ranger_log_config_dir? Or even then, force admins to
create it?
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/ranger/ranger_client.cc@296
PS8, Line 296: -Dlog4j2.configurationFile
Nit: maybe -Dlog4j2.configuration.filename?
http://gerrit.cloudera.org:8080/#/c/15628/8/src/kudu/subprocess/subprocess_proxy.cc
File src/kudu/subprocess/subprocess_proxy.cc:
PS8:
We're doing more and more of this "template composition", and it's getting
increasingly complicated. At some point we're better off using a real
templating system, which should be able to handle composition based on a
variety of conditions (loops, if/else, etc.).
We've got mustache in the codebase and I think it'd be a reasonable fit.
Currently mustache expects template files to live on disk, but there's no
reason we can't modify it to reference them from memory.
Doesn't apply to this patch, but something to think about.
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 03 Apr 2020 21:30:54 +
Gerrit-HasComments: Yes
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@166 PS1, Line 166: void AsyncHandler(ev::async& watcher, int revents); > nit: while you are here, maybe add a comment that the signature of this fun I think that's already implied in the existing comment, no? "libev callback for handling ...". http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@169 PS1, Line 169: void TimerHandler(ev::timer& watcher, int revents); > ditto See above. -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 Apr 2020 21:12:04 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3099: Remove System.exit() calls in KuduBackup/KuduRestore
Grant Henke has abandoned this change. ( http://gerrit.cloudera.org:8080/15626 ) Change subject: KUDU-3099: Remove System.exit() calls in KuduBackup/KuduRestore .. Abandoned Superseded by https://gerrit.cloudera.org/#/c/15638/ -- To view, visit http://gerrit.cloudera.org:8080/15626 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Iafcab9ef456b6f50357b8840a1eecc95f753513b Gerrit-Change-Number: 15626 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15638 ) Change subject: KUDU-3099: Remove System.exit() calls from KuduBackup/KuduRestore .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15638/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala: http://gerrit.cloudera.org:8080/#/c/15638/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala@174 PS1, Line 174: thrown I think this is a typo and fails to compile. You can edit and test by using the below in the java directory: `./gradlew :kudu-backup:check` I also suspect you will need to adjust the tests slightly. Like here for example: https://github.com/apache/kudu/blob/master/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala#L698 -- To view, visit http://gerrit.cloudera.org:8080/15638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d1b4796b6280adecd7dab685a0281af6b2570ce Gerrit-Change-Number: 15638 Gerrit-PatchSet: 1 Gerrit-Owner: Anonymous Coward Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 03 Apr 2020 20:51:14 + Gerrit-HasComments: Yes
[kudu-CR] remove last vestiges of boost::bind, boost::function, and std::bind
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15639 ) Change subject: remove last vestiges of boost::bind, boost::function, and std::bind .. Patch Set 1: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/client/client.cc File src/kudu/client/client.cc: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/client/client.cc@1684 PS1, Line 1684: // CloseCallback::Callback() deletes the closer. Thank you for adding this comment. http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h File src/kudu/rpc/reactor.h: http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@166 PS1, Line 166: void AsyncHandler(ev::async& watcher, int revents); > warning: non-const reference parameter 'watcher', make it const or use a po nit: while you are here, maybe add a comment that the signature of this function is dictated by libev? http://gerrit.cloudera.org:8080/#/c/15639/1/src/kudu/rpc/reactor.h@169 PS1, Line 169: void TimerHandler(ev::timer& watcher, int revents); > warning: non-const reference parameter 'watcher', make it const or use a po ditto -- To view, visit http://gerrit.cloudera.org:8080/15639 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd876f200f680f3d3617d78852f1e0a9c5649372 Gerrit-Change-Number: 15639 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 Apr 2020 20:26:20 + Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15628 to look at the new patch set (#8). Change subject: ranger: enable log4j2 logging to files .. ranger: enable log4j2 logging to files This patch enables Kudu to generate a log4j2 properties file and pass it to the Ranger client. This file is created in a directory specified by the new --ranger_logging_config_dir flag, which, by default, will direct the properties file to --log_dir. If the properties file already exists, the existing file is honored. This means a file will not be regenerated if on has already been generated -- I will add a follow up patch to allow users to overwrite the file. This properties file will direct logs to directory pointed to by --log_dir, and will honor the existing --max_log_files and --max_log_size flags. I've found logging to stdout and adjusting log42j log-level to be useful, so this also adds --ranger_logtostdout and --ranger_log_level for these respectively. I opted not to use the glog --minloglevel flag since they don't quite map to log4j2 log levels (e.g. glog doesn't have debug-level logging). I manually tested that an existing properties file that has been modified will be honored. Here's what's in a Ranger-configured Kudu master's log directory: kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-122527.0.17165 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-122527.17165 kudu-ranger-subprocess-log4j2.properties kudu-ranger-subprocess.awong-MBP16-21621.local.log Change-Id: I7efa631832c219fce214304538e6ab6442062752 --- M java/gradle/dependencies.gradle M java/kudu-subprocess/build.gradle M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/subprocess/CMakeLists.txt A src/kudu/subprocess/subprocess_proxy.cc M src/kudu/subprocess/subprocess_proxy.h M src/kudu/util/env.h 10 files changed, 339 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/8 -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 8 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15628 )
Change subject: ranger: enable log4j2 logging to files
..
Patch Set 7:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@12
PS7, Line 12: file
> the properties file? Does this mean the properties file cannot be updated o
Users can manually modify the file and the changes will take effect. But
updating flags and restarting won't update the file unless the conf file is
deleted. I can add a --ranger_overwrite_logging_config or something in a follow
up patch.
http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@21
PS7, Line 21: ranger_enable_debug_logging
> Would it make sense to do `--ranger_log_level` and accept standard log4j le
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc
File src/kudu/ranger/ranger_client.cc:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@77
PS7, Line 77: client
> nit: should we remove `client`. This can contain subprocess logs too right?
I'll replace this with "subprocess".
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@81
PS7, Line 81: TAG_FLAG(ranger_logging_config_dir, advanced);
> Should we mark this as evolving?
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@85
PS7, Line 85: TAG_FLAG(ranger_enable_debug_logging, advanced);
> Should we mark this as evolving?
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@89
PS7, Line 89: TAG_FLAG(ranger_logtostdout, hidden);
> Should we also mark this as `unsafe` since it's for testing only?
There's nothing unsafe about it per se. I'll just make this advanced.
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h
File src/kudu/subprocess/subprocess_proxy.h:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@38
PS7, Line 38: namespace {
> warning: do not use unnamed namespaces in header files [google-build-namesp
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@50
PS7, Line 50: static const char* kLog4j2RollingPropertiesTemplate = R"(
> warning: variable 'kLog4j2RollingPropertiesTemplate' defined in a header fi
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@66
PS7, Line 66: static const char* kLog4j2ConsoleProperties = R"(
> warning: 'kLog4j2ConsoleProperties' is a static definition in anonymous nam
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@79
PS7, Line 79: static const char* kLog4j2PropertiesTemplate = R"(
> warning: 'kLog4j2PropertiesTemplate' is a static definition in anonymous na
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@92
PS7, Line 92: string
> nit: remove
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@122
PS7, Line 122: appender_ref_name_specs.emplace_back(
> warning: 'emplace_back' is called inside a loop; consider pre-allocating th
Done
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/util/env.h
File src/kudu/util/env.h:
http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/util/env.h@694
PS7, Line 694: WriteStringToFileSync
> Can you comment on the difference between the above method?
Done
--
To view, visit http://gerrit.cloudera.org:8080/15628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752
Gerrit-Change-Number: 15628
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 03 Apr 2020 19:28:23 +
Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: ranger: enable log4j2 logging to files .. Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@12 PS7, Line 12: file the properties file? Does this mean the properties file cannot be updated once created? http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h File src/kudu/subprocess/subprocess_proxy.h: http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/subprocess/subprocess_proxy.h@92 PS7, Line 92: string nit: remove http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/util/env.h File src/kudu/util/env.h: http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/util/env.h@694 PS7, Line 694: WriteStringToFileSync Can you comment on the difference between the above method? -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 Apr 2020 19:05:51 + Gerrit-HasComments: Yes
[kudu-CR] columnar serialization: use AVX2 for int32 and int64 copying
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15634 ) Change subject: columnar_serialization: use AVX2 for int32 and int64 copying .. Patch Set 2: > Patch Set 2: Verified-1 > > Build Failed > > http://jenkins.kudu.apache.org/job/kudu-gerrit/21281/ : FAILURE Github was down. Will need to rebase/retrigger the jenkins verification job. -- To view, visit http://gerrit.cloudera.org:8080/15634 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c9a536b78a524e8178f5d4a0d2dea04deedbd78 Gerrit-Change-Number: 15634 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Apr 2020 18:45:12 + Gerrit-HasComments: No
[kudu-CR] rowblock: use BMI instruction set when available for GetSelectedRows
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15635 ) Change subject: rowblock: use BMI instruction set when available for GetSelectedRows .. Patch Set 2: Github was down when jenkins verification ran. Will need to rebase/retrigger the jenkins job. -- To view, visit http://gerrit.cloudera.org:8080/15635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ec74bc5db07c18d0e36de14a2343f49fc5c2859 Gerrit-Change-Number: 15635 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Apr 2020 18:43:53 + Gerrit-HasComments: No
[kudu-CR] cfile: change BlockBuilder API to yield a vector of Slices
Bankim Bhavsar has uploaded a new patch set (#5) to the change originally created by Todd Lipcon. ( http://gerrit.cloudera.org:8080/15042 ) Change subject: cfile: change BlockBuilder API to yield a vector of Slices .. cfile: change BlockBuilder API to yield a vector of Slices When blocks are appended to cfiles at the IO layer, we already have the ability to write multiple slices using a vectored IO. Prior to this patch, the BlockBuilder API was restricted to returning a single slice, whereas it would be more convenient in some cases to be able to return multiple slices (eg separating the header from the data). This new functionality is used by BinaryDictBlockBuilder to avoid an extra copy in Finish(). Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db --- M src/kudu/cfile/binary_dict_block.cc M src/kudu/cfile/binary_dict_block.h M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/binary_plain_block.h M src/kudu/cfile/binary_prefix_block.cc M src/kudu/cfile/binary_prefix_block.h M src/kudu/cfile/block_encodings.h M src/kudu/cfile/bshuf_block.cc M src/kudu/cfile/bshuf_block.h M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/encoding-test.cc M src/kudu/cfile/plain_bitmap_block.h M src/kudu/cfile/plain_block.h M src/kudu/cfile/rle_block.h 14 files changed, 121 insertions(+), 107 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/15042/5 -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [docker] Remove `set -x` from entrypoint scripts
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15643 ) Change subject: [docker] Remove `set -x` from entrypoint scripts .. [docker] Remove `set -x` from entrypoint scripts This patch removes the `set -x` from entrypoint scripts to prevent noise in scripts that users interact with. Change-Id: I4b14a6358035cf399887a1059bf0176c59411686 Reviewed-on: http://gerrit.cloudera.org:8080/15643 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M docker/impala-entrypoint.sh M docker/kudu-entrypoint.sh 2 files changed, 2 insertions(+), 2 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/15643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4b14a6358035cf399887a1059bf0176c59411686 Gerrit-Change-Number: 15643 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [docker] Remove `set -x` from entrypoint scripts
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15643 ) Change subject: [docker] Remove `set -x` from entrypoint scripts .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b14a6358035cf399887a1059bf0176c59411686 Gerrit-Change-Number: 15643 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 03 Apr 2020 17:57:01 + Gerrit-HasComments: No
[kudu-CR] [docker] Remove `set -x` from entrypoint scripts
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15643 Change subject: [docker] Remove `set -x` from entrypoint scripts .. [docker] Remove `set -x` from entrypoint scripts This patch removes the `set -x` from entrypoint scripts to prevent noise in scripts that users interact with. Change-Id: I4b14a6358035cf399887a1059bf0176c59411686 --- M docker/impala-entrypoint.sh M docker/kudu-entrypoint.sh 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/15643/1 -- To view, visit http://gerrit.cloudera.org:8080/15643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4b14a6358035cf399887a1059bf0176c59411686 Gerrit-Change-Number: 15643 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] subprocess: enable log4j debug logging
Grant Henke has abandoned this change. ( http://gerrit.cloudera.org:8080/15627 ) Change subject: subprocess: enable log4j debug logging .. Abandoned I think the later patch supersedes this one. https://gerrit.cloudera.org/c/15628 Feel free to re-open if not. -- To view, visit http://gerrit.cloudera.org:8080/15627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: If22dfdb4e8ba7814c0a91ba53ec4964220a9d139 Gerrit-Change-Number: 15627 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] ranger: enable log4j2 logging to files
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: ranger: enable log4j2 logging to files .. Patch Set 7: (6 comments) http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@11 PS7, Line 11: logging nit: Maybe just `log` http://gerrit.cloudera.org:8080/#/c/15628/7//COMMIT_MSG@21 PS7, Line 21: ranger_enable_debug_logging Would it make sense to do `--ranger_log_level` and accept standard log4j levels and default to INFO? (TRACE,DEBUG,INFO,WARN,ERROR) http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc File src/kudu/ranger/ranger_client.cc: http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@77 PS7, Line 77: client nit: should we remove `client`. This can contain subprocess logs too right? http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@81 PS7, Line 81: TAG_FLAG(ranger_logging_config_dir, advanced); Should we mark this as evolving? http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@85 PS7, Line 85: TAG_FLAG(ranger_enable_debug_logging, advanced); Should we mark this as evolving? http://gerrit.cloudera.org:8080/#/c/15628/7/src/kudu/ranger/ranger_client.cc@89 PS7, Line 89: TAG_FLAG(ranger_logtostdout, hidden); Should we also mark this as `unsafe` since it's for testing only? -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 03 Apr 2020 14:49:15 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3007. Support building Kudu on aarch64 platform
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, huangtianhua...@gmail.com, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14964 to look at the new patch set (#12). Change subject: KUDU-3007. Support building Kudu on aarch64 platform .. KUDU-3007. Support building Kudu on aarch64 platform This change try to modify and make Kudu can build sucessfully on both x86_64 and aarch64 platform. Change-Id: I2953519c5d28de17e6b2bb7094abab0c1cd12c97 --- M CMakeLists.txt M README.adoc M build-support/jenkins/build-and-test.sh M java/kudu-client/build.gradle M java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java M src/kudu/cfile/binary_plain_block.cc M src/kudu/cfile/bitshuffle_arch_wrapper.cc M src/kudu/codegen/codegen-test.cc M src/kudu/common/columnar_serialization.cc M src/kudu/common/key_encoder.h M src/kudu/common/zp7.cc A src/kudu/gutil/atomicops-internals-arm64.h M src/kudu/gutil/atomicops-internals-x86.cc M src/kudu/gutil/atomicops.h M src/kudu/gutil/cpu.cc M src/kudu/gutil/cycleclock-inl.h M src/kudu/gutil/dynamic_annotations.h M src/kudu/gutil/port.h M src/kudu/gutil/spinlock.h M src/kudu/gutil/spinlock_linux-inl.h M src/kudu/util/block_bloom_filter.cc M src/kudu/util/block_bloom_filter.h M src/kudu/util/char_util.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h M src/kudu/util/group_varint-inl.h M src/kudu/util/group_varint-test.cc M src/kudu/util/init.cc M src/kudu/util/memory/memory.cc M src/kudu/util/notification.h A src/kudu/util/sse2neon.h M src/kudu/util/striped64.cc M thirdparty/build-definitions.sh 33 files changed, 3,798 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/14964/12 -- To view, visit http://gerrit.cloudera.org:8080/14964 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2953519c5d28de17e6b2bb7094abab0c1cd12c97 Gerrit-Change-Number: 14964 Gerrit-PatchSet: 12 Gerrit-Owner: liusheng Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: liusheng
[kudu-CR] ranger: enable log4j2 logging to files
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15628 to look at the new patch set (#7). Change subject: ranger: enable log4j2 logging to files .. ranger: enable log4j2 logging to files This patch enables Kudu to generate a log4j2 properties file and pass it to the Ranger client. This file is created in a directory specified by the new --ranger_logging_config_dir flag, which, by default, will direct the properties file to --log_dir. If the file already exists, the existing file is honored. This properties file will direct logs to directory pointed to by --log_dir, and will honor the existing --max_log_files and --max_log_size flags. I've found logging to stdout and log42j debug-level logging to be useful, so this also adds --ranger_logtostdout and --ranger_enable_debug_logging for these respectively. I opted not to use the glog --minloglevel flag since they don't quite map to log4j2 log levels (e.g. glog doesn't have debug-level logging). I manually tested that an existing properties file that has been modified will be honored. Here's what's in a Ranger-configured Kudu master's log directory: kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005921.5395 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005349.0.5236 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005609.0.5320 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005921.0.5395 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005349.5236 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005609.5320 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005921.5395 kudu-ranger-client-log4j2.properties kudu-ranger-client.awong-MBP16-21621.local.20200403-005349.log.gz kudu-ranger-client.awong-MBP16-21621.local.log Change-Id: I7efa631832c219fce214304538e6ab6442062752 --- M java/gradle/dependencies.gradle M java/kudu-subprocess/build.gradle M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/subprocess/subprocess_proxy.h M src/kudu/util/env.h 8 files changed, 295 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/7 -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 7 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] ranger: enable log4j2 logging to files
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15628 ) Change subject: ranger: enable log4j2 logging to files .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java: http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java@252 PS5, Line 252: > Should this be the default if a file is not provided? If we remove the abil Yeah, I'm going to punt on this for now; the new patch doesn't touch this code at all. http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/resources/log4j2.properties File java/kudu-subprocess/src/main/resources/log4j2.properties: http://gerrit.cloudera.org:8080/#/c/15628/5/java/kudu-subprocess/src/main/resources/log4j2.properties@30 PS5, Line 30: > If we allow generating the log4j conf in the master based on my review in t Done -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 03 Apr 2020 08:10:49 + Gerrit-HasComments: Yes
[kudu-CR] subprocess: enable log4j debug logging
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15627 ) Change subject: subprocess: enable log4j debug logging .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/15627/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java: http://gerrit.cloudera.org:8080/#/c/15627/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java@164 PS2, Line 164: final String debugLogLongOpt = "debugLog"; > Another option is to use customer system properties in the log template to I went the -Dlog4j.configurationFile route since it gives us a bit more flexibility. Put all the behavior into the next patch though: https://gerrit.cloudera.org/c/15628 http://gerrit.cloudera.org:8080/#/c/15627/2/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessConfiguration.java@209 PS2, Line 209: // If we're using System.out for IO, don't log to it. > This might be a seperate patch, but can we remove the option to use output Yeah, I'm punting on this for now just because I haven't yet seen an elegant way to get a fifo via Java. It's test-only for now though. -- To view, visit http://gerrit.cloudera.org:8080/15627 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If22dfdb4e8ba7814c0a91ba53ec4964220a9d139 Gerrit-Change-Number: 15627 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 03 Apr 2020 08:10:16 + Gerrit-HasComments: Yes
[kudu-CR] ranger: enable log4j2 logging to files
Hello Attila Bukor, Kudu Jenkins, Adar Dembo, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15628 to look at the new patch set (#6). Change subject: ranger: enable log4j2 logging to files .. ranger: enable log4j2 logging to files This patch enables Kudu to generate a log4j2 properties file and pass it to the Ranger client. This file is created in a directory specified by the new --ranger_logging_config_dir flag, which, by default, will direct the properties file to --log_dir. If the file already exists, the existing file is honored. This properties file will direct logs to directory pointed to by --log_dir, and will honor the existing --max_log_files and --max_log_size flags. I've found logging to stdout and log42j debug-level logging to be useful, so this also adds --ranger_logtostdout and --ranger_enable_debug_logging for these respectively. I opted not to use the glog --minloglevel flag since they don't quite map to log4j2 log levels (e.g. glog doesn't have debug-level logging). I manually tested that an existing properties file that has been modified will be honored. Here's what's in a Ranger-configured Kudu master's log directory: kudu-master.INFO -> kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005921.5395 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005349.0.5236 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005609.0.5320 kudu-master.awong-MBP16-21621.local.awong.diagnostics.20200403-005921.0.5395 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005349.5236 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005609.5320 kudu-master.awong-MBP16-21621.local.awong.log.INFO.20200403-005921.5395 kudu-ranger-client-log4j2.properties kudu-ranger-client.awong-MBP16-21621.local.20200403-005349.log.gz kudu-ranger-client.awong-MBP16-21621.local.log Change-Id: I7efa631832c219fce214304538e6ab6442062752 --- M java/gradle/dependencies.gradle M java/kudu-subprocess/build.gradle M src/kudu/ranger/mini_ranger.cc M src/kudu/ranger/mini_ranger.h M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/subprocess/subprocess_proxy.h M src/kudu/util/env.h 8 files changed, 293 insertions(+), 60 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/15628/6 -- To view, visit http://gerrit.cloudera.org:8080/15628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7efa631832c219fce214304538e6ab6442062752 Gerrit-Change-Number: 15628 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
