[kudu-CR] build: stop generating md5 file
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9504 ) Change subject: build: stop generating md5 file .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9504 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b0359d63b660b285c0853cd9b88654a400f14c2 Gerrit-Change-Number: 9504 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 06 Mar 2018 02:31:37 + Gerrit-HasComments: No
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9496 ) Change subject: KUDU-721: [Python] Add DECIMAL column type support .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx File python/kudu/client.pyx: http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@1319 PS2, Line 1319: cdef inline get_unscaled_decimal(self, int i): > should this also have a method to get the scaled decimal? seems weird that Actually this should probably just be private and I should just have a public get_decimal. I was using this for get_slot and not really thinking about the public usage. It looks like get_unixtime_micros does something similar where get_slot does the transformation but there isn't a getter to do so. That might be a good change for a different patch too. http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@1319 PS2, Line 1319: cdef inline get_unscaled_decimal(self, int i): : cdef int128_t val : check_status(self.row.GetUnscaledDecimal(i, )) : return val > worth a note on what is an unscaled decimal With this being private (comment above) we shouldn't need to explain it. http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@1324 PS2, Line 1324: cdef inline get_scale(self, int i): > it's not clear that this "scale" refers to scale if this is a decimal, hard The default is 0 for scale because that's the common default for most databases. We default to 0 for precision for convenience on non-decimal columns and also to represent an "invalid" precision in cases where type attributes is used on a non-decimal column. http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@2468 PS2, Line 2468: to_unscaled_decimal(value))) > nit: indentation Done http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/libkudu_client.pxd File python/kudu/libkudu_client.pxd: http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/libkudu_client.pxd@65 PS2, Line 65: cdef extern from "kudu/util/int128.h" namespace "kudu" nogil: : ctypedef int int128_t > do we really need this extern to get a typedef and, if we do, do we need th int128_t isn't actually a c++ type. We define it in int128.h based on signed __int128. Without this all the int128_t usage below fails. Reading up on GIL and nogil it looks like it's something that should be done for thread safe code. Which I suspect a typedef has no issues with. It's used quite a bit in this file. http://cython.readthedocs.io/en/latest/src/userguide/external_C_code.html#acquiring-and-releasing-the-gil http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/tests/test_scantoken.py File python/kudu/tests/test_scantoken.py: http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/tests/test_scantoken.py@246 PS2, Line 246: # Test unixtime_micros value predicate > copy pasta artifact Done -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 2 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Tue, 06 Mar 2018 02:03:04 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Hello a...@phdata.io, David Ribeiro Alves, Wes McKinney, Kudu Jenkins, Jordan Birdsell, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9496 to look at the new patch set (#3). Change subject: KUDU-721: [Python] Add DECIMAL column type support .. KUDU-721: [Python] Add DECIMAL column type support This patch adds basic support to the Python client to create, read, and write tables with DECIMAL columns. Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 --- M python/kudu/__init__.py M python/kudu/client.pyx M python/kudu/libkudu_client.pxd M python/kudu/schema.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 A python/kudu/tests/test_util.py M python/kudu/tests/util.py M python/kudu/util.py 11 files changed, 369 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/9496/3 -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 3 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io
[kudu-CR] decimal util: do not produce warnings when building with gcc
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9499 ) Change subject: decimal_util: do not produce warnings when building with gcc .. Patch Set 1: Code-Review+2 Sorry I missed this pattern. Thanks for fixing it. -- To view, visit http://gerrit.cloudera.org:8080/9499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac9b145a914d551e3a18d62a0984aad34b95f4dd Gerrit-Change-Number: 9499 Gerrit-PatchSet: 1 Gerrit-Owner: Adar DemboGerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 06 Mar 2018 01:55:23 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191 (6/n): Hive Metastore catalog manager integration
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8312 ) Change subject: KUDU-2191 (6/n): Hive Metastore catalog manager integration .. Patch Set 4: (39 comments) http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@144 PS2, Line 144: db.name = hms_database_name; > I think you can use ContainsKey here, from map-util.h. ContainsKey doesn't work on vector. http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/integration-tests/master_hms-itest.cc@155 PS2, Line 155: s = CreateTable(hms_database_name, hms_table_name); > ContainsKey here too. Ack http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/integration-tests/master_hms-itest.cc@102 PS3, Line 102: b.AddColumn("int8_val")->Type(KuduColumnSchema::INT8); > warning: passing result of std::move() as a const reference argument; no mo Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.h File src/kudu/master/catalog_manager.h: http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.h@33 PS2, Line 33: #include > Not used? Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.cc@231 PS2, Line 231: TAG_FLAG(catalog_manager_inject_latency_load_ca_info_ms, hidden); > Should probably beef this up a bit to explain what setting this actually do Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.cc@634 PS2, Line 634: // Propagate the 'read_default' to the 'write_default' in 'col', > 9083 is the default HMS port? Probably deserves more visibility than inline Done http://gerrit.cloudera.org:8080/#/c/8312/2/src/kudu/master/catalog_manager.cc@1415 PS2, Line 1415: > It's worth noting what happens to this table if step e fails. Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/catalog_manager.cc@231 PS3, Line 231: TAG_FLAG(catalog_manager_inject_latency_load_ca_info_ms, hidden); > a bit more docs here, eg what the multiple addrs mean, etc. Also does the H Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/catalog_manager.cc@1419 PS3, Line 1419: } > I think it's worth logging such errors as well, since it may be an end user Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.h File src/kudu/master/hms_catalog.h: http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.h@29 PS3, Line 29: #include "kudu/util/net/net_util.h" > Seems like you may be able to forward declare HmsClient. optional seems to require the include in order to compile. http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.h@42 PS3, Line 42: // > Maybe define this as a private nested class of HmsCatalog, to make it clear Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.h@70 PS3, Line 70: st, Te > Can you get away with forward declaring Schema? Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.h@90 PS3, Line 90:std::string* hms_table) WARN_UNUSED_RESULT; > Maybe it shouldn't be a class member then? It could be declared on the stac Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.h@91 PS3, Line 91: > Worth a comment explaining why this is optional. Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc File src/kudu/master/hms_catalog.cc: http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@31 PS3, Line 31: #include "kudu/common/schema.h" : #include "kudu/gutil/strings/split.h > Why do you need these? Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@34 PS3, Line 34: #include "kudu/hms/hive_metastore_constants.h" > Not used? Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@38 PS3, Line 38: #include "kudu/util/net/net_util.h" : #include "kudu/util/thr > These aren't used. Done http://gerrit.cloudera.org:8080/#/c/8312/3/src/kudu/master/hms_catalog.cc@73 PS3, Line 73: RETURN_NOT_OK(ThreadPoolBuilder("hms-catalog") > Should this also null out worker_? What's the expected behavior of multiple Stop() should be idempotent, which I think it is here, assuming ThreadJoiner::Join is idempotent (which it appears to be from reading its source).
[kudu-CR] KUDU-2191 (6/n): Hive Metastore catalog manager integration
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8312 to look at the new patch set (#4). Change subject: KUDU-2191 (6/n): Hive Metastore catalog manager integration .. KUDU-2191 (6/n): Hive Metastore catalog manager integration This commit adds integration with the Hive Metastore to CatalogManager, which can be enabled with a new flag: 'hive-metastore-addresses'. When the flag is provided, the Master will push DDL changes in its catalog to the Hive Metastore. In particular: - Create table, delete table, and alter table operations in the Kudu master will synchronously update the corresponding entry in the HMS. - New table and column names are required to be valid according to the Hive identifier rules, which are much stricter than Kudu's existing identifier rules. I think the code-paths in hms_catalog are pretty well covered by the tests added in this commit, however there is a dearth of stress and chaos tests covering the actual CatalogManager integration. In particular I've left some TODOs of roll-back code that could benefit from more coverage, as well as a TODO in master-stress-test about enabling HMS integration. Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf --- M src/kudu/hms/hms_client-test.cc M src/kudu/hms/hms_client.cc M src/kudu/hms/hms_client.h M src/kudu/hms/mini_hms.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/master-stress-test.cc A src/kudu/integration-tests/master_hms-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h A src/kudu/master/hms_catalog-test.cc A src/kudu/master/hms_catalog.cc A src/kudu/master/hms_catalog.h M src/kudu/master/master.proto M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/util/net/net_util.cc M src/kudu/util/net/net_util.h 19 files changed, 1,214 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/8312/4 -- To view, visit http://gerrit.cloudera.org:8080/8312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1b8f360408b05d6ac4f7359a8ac0dd889b196baf Gerrit-Change-Number: 8312 Gerrit-PatchSet: 4 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] build: adjust the number of processors for various tests
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9503 ) Change subject: build: adjust the number of processors for various tests .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/9503/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9503/1//COMMIT_MSG@27 PS1, Line 27: I used this as a rough guide to find tests : with inappropriate settings in CMakeLists.txt. What's the net effect of these changes? What can I expect if I run "ctest -j$(nproc)" before vs. after? http://gerrit.cloudera.org:8080/#/c/9503/1/CMakeLists.txt File CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/9503/1/CMakeLists.txt@699 PS1, Line 699: # a heavy consumer of CPU. The PROCESSORS flag allows ctest to ensure So the idea is that any test NOT tagged with PROCESSORS or RUN_SERIAL is a test that uses a negligible amount of CPU? If that's the case, maybe you could add that as guidance here? http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc File src/kudu/util/test_main.cc: http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc@77 PS1, Line 77: const auto kInterval = MonoDelta::FromMilliseconds(500); How did you arrive at this particular interval value? http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc@78 PS1, Line 78: Stopwatch sw(Stopwatch::ALL_THREADS); > this isn't great because it doesn't include child processes, but I couldn't What about the answers in https://stackoverflow.com/questions/12871090/how-to-calculate-cpu-utilization-of-a-process-all-its-child-processes-in-linux? The "waited-for" thing is a bit of a bummer since in our case that usually only happens at the end of a test when we tear down a cluster. But, maybe you could get an average that way, by dividing the total times by the test's total running time? Would that be useful? http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc@104 PS1, Line 104: Nit: revert this? -- To view, visit http://gerrit.cloudera.org:8080/9503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa Gerrit-Change-Number: 9503 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 06 Mar 2018 01:33:47 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 8: Verified+1 The failed test is unrelated flaky -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 8 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 06 Mar 2018 01:26:18 + Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Hao Hao has removed a vote on this change. Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 8 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] build: stop generating md5 file
Hello Dan Burkert, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9504 to review the following change. Change subject: build: stop generating md5 file .. build: stop generating md5 file New guidance from the ASF is to not generate md5 files when producing source releases. Instead we just generate the sha checksum. Change-Id: I6b0359d63b660b285c0853cd9b88654a400f14c2 --- M build-support/build_source_release.py 1 file changed, 11 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/9504/1 -- To view, visit http://gerrit.cloudera.org:8080/9504 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6b0359d63b660b285c0853cd9b88654a400f14c2 Gerrit-Change-Number: 9504 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Dan Burkert
[kudu-CR] dist test: fix the ability to pass flags to the 'run' option
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9502 ) Change subject: dist_test: fix the ability to pass flags to the 'run' option .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id191a6f35520678ce48e78be0507d9f4607a2ea8 Gerrit-Change-Number: 9502 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 06 Mar 2018 01:19:58 + Gerrit-HasComments: No
[kudu-CR] build: adjust the number of processors for various tests
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9503 ) Change subject: build: adjust the number of processors for various tests .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc File src/kudu/util/test_main.cc: http://gerrit.cloudera.org:8080/#/c/9503/1/src/kudu/util/test_main.cc@78 PS1, Line 78: Stopwatch sw(Stopwatch::ALL_THREADS); this isn't great because it doesn't include child processes, but I couldn't find any reasonable way to include them, so figured this is better than nothing. LMK if you want me to not include this part of the patch since it was more of a short-term hack -- To view, visit http://gerrit.cloudera.org:8080/9503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa Gerrit-Change-Number: 9503 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 06 Mar 2018 01:09:24 + Gerrit-HasComments: Yes
[kudu-CR] Make metrics name matching case-insensitive
Andrew Wong has removed a vote on this change. Change subject: Make metrics name matching case-insensitive .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7 Gerrit-Change-Number: 9462 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] Make metrics name matching case-insensitive
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 ) Change subject: Make metrics name matching case-insensitive .. Patch Set 2: Verified+1 > Patch Set 2: Verified-1 > > Build Failed > > http://jenkins.kudu.apache.org/job/kudu-gerrit/12283/ : FAILURE The build failure seems unrelated, but scary. Looking into it. -- To view, visit http://gerrit.cloudera.org:8080/9462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7 Gerrit-Change-Number: 9462 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Mar 2018 01:04:09 + Gerrit-HasComments: No
[kudu-CR] build: adjust the number of processors for various tests
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9503 to review the following change. Change subject: build: adjust the number of processors for various tests .. build: adjust the number of processors for various tests Many of our tests are multithreaded and can utilize many cores worth of CPU during execution. Other tests are somewhat timing-sensitive and might time out if run in coordination with a stress test, particularly on a small machine which might only have four CPUs total. This patch adjusts the core count property for a bunch of tests based on some empirical analysis using a new '--test_report_cpu_usage' flag. I built a release build and then submitted a dist-test run with this set. I then collected the artifacts and analyzed them with the following command: for x in dist-test-results/*/build/*/*/*.txt ; do echo -n $(basename $x) grep -h 'Cores used over' $x | grep -o ': .*$' | sort -nk2 | tail -1 echo done | sort -nk2 This resulted in a list of tests along with their peak CPU usage (from their busiest 500ms period). I used this as a rough guide to find tests with inappropriate settings in CMakeLists.txt. Change-Id: I922031af69178b98459d0d59287442b692425afa --- M CMakeLists.txt M src/kudu/cfile/CMakeLists.txt M src/kudu/client/CMakeLists.txt M src/kudu/clock/CMakeLists.txt M src/kudu/consensus/CMakeLists.txt M src/kudu/integration-tests/CMakeLists.txt M src/kudu/rpc/CMakeLists.txt M src/kudu/security/CMakeLists.txt M src/kudu/tablet/CMakeLists.txt M src/kudu/util/CMakeLists.txt M src/kudu/util/test_main.cc 11 files changed, 68 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/9503/1 -- To view, visit http://gerrit.cloudera.org:8080/9503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I922031af69178b98459d0d59287442b692425afa Gerrit-Change-Number: 9503 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo
[kudu-CR] dist test: fix the ability to pass flags to the 'run' option
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9502 to review the following change. Change subject: dist_test: fix the ability to pass flags to the 'run' option .. dist_test: fix the ability to pass flags to the 'run' option The python 'argparse' module ends up considering the special '--' flag an actual flag in the "remainder". So, for a typical command line like: dist_test.py run -- --stress_cpu_threads 3 ... it was ending up passing too many '--' through to the underlying binary, which caused the arguments after it to be ignored. This simply removes the extra '--' when it is detected. Change-Id: Id191a6f35520678ce48e78be0507d9f4607a2ea8 --- M build-support/dist_test.py 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/9502/1 -- To view, visit http://gerrit.cloudera.org:8080/9502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id191a6f35520678ce48e78be0507d9f4607a2ea8 Gerrit-Change-Number: 9502 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo
[kudu-CR] [docs] Improvements to multi-master migration doc
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9466 ) Change subject: [docs] Improvements to multi-master migration doc .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc File docs/administration.adoc: http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@354 PS2, Line 354: . If your Kudu cluster is secure, in addition to running as the Kudu UNIX user, you must > Hmm, breaking it out like this will make it look like a separate step. What I agree, I think pushing this into a WARNING would read better. I'm not sure if we have any documentation on this particular step. If we have some, maybe link it here, and if not, should we mention what this means from an administrative POV (running `kinit` or something)? -- To view, visit http://gerrit.cloudera.org:8080/9466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc Gerrit-Change-Number: 9466 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Mar 2018 00:48:23 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/9496 ) Change subject: KUDU-721: [Python] Add DECIMAL column type support .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx File python/kudu/client.pyx: http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@1319 PS2, Line 1319: cdef inline get_unscaled_decimal(self, int i): should this also have a method to get the scaled decimal? seems weird that the get_slot method returns a decimal, but the caller has to call from_unscaled_decimal if using the particular getter http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@1319 PS2, Line 1319: cdef inline get_unscaled_decimal(self, int i): : cdef int128_t val : check_status(self.row.GetUnscaledDecimal(i, )) : return val worth a note on what is an unscaled decimal http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@1324 PS2, Line 1324: cdef inline get_scale(self, int i): it's not clear that this "scale" refers to scale if this is a decimal, hard to navigate to the c++ header, since this is python. likely docs enough. on a totally side note, as I was browsing ColumnTypeAttributes i noticed that the defaults for precision and scale are 0. was wondering why not 1. http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/client.pyx@2468 PS2, Line 2468: to_unscaled_decimal(value))) nit: indentation http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/libkudu_client.pxd File python/kudu/libkudu_client.pxd: http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/libkudu_client.pxd@65 PS2, Line 65: cdef extern from "kudu/util/int128.h" namespace "kudu" nogil: : ctypedef int int128_t do we really need this extern to get a typedef and, if we do, do we need the nogil? http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/tests/test_scantoken.py File python/kudu/tests/test_scantoken.py: http://gerrit.cloudera.org:8080/#/c/9496/2/python/kudu/tests/test_scantoken.py@246 PS2, Line 246: # Test unixtime_micros value predicate copy pasta artifact -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 2 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Tue, 06 Mar 2018 00:45:21 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 8: (7 comments) http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG@6 PS7, Line 6: : KUDU-1704: add java client support for READ_YOUR_WRITES mode > ok, thats fine. really want to make sure we don't drop it though, if we're Yeah, totally makes sense to me. I will certainly pick up the jepsen test with RYW. http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java: http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@817 PS7, Line 817: case OPENING: > nit, comma after "set" Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@818 PS7, Line 818: nner. This kind of a > pet peeve: propagated timestamp or propagation timestamp? are these differe Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1044 PS7, Line 1044: run > nit: "are running" or just "run" Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1047 PS7, Line 1047: // scan mode, from leader replica. In this test writes are > nit: comma after "scan mode" Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1062 PS7, Line 1062: // Similar to testReadYourWritesSyncLeaderReplica, but in this : // test writes are performed in MANUAL_FLUSH (batches) flush modes. : @Test(timeout = 10) : public void testReadYourWritesBa > This text is repeated. Likely add it to the first test and then make the ot Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1086 PS7, Line 1086: > avoid magic numbers (set a final var) Done -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 8 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 06 Mar 2018 00:35:23 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Hello Dan Burkert, David Ribeiro Alves, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8847 to look at the new patch set (#8). Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. KUDU-1704: add java client support for READ_YOUR_WRITES mode Change-Id: I6239521c022147257859e399f55c6f3f945af465 --- M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java 4 files changed, 241 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/8847/8 -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 8 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [docs] Improvements to multi-master migration doc
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9466 ) Change subject: [docs] Improvements to multi-master migration doc .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc File docs/administration.adoc: http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@297 PS2, Line 297: , which should be run as the Kudu UNIX user, typically `kudu` Given the addition of "sudo -u kudu" throughout (and the prior existence of the WARNING up front), this is probably redundant. http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@354 PS2, Line 354: . If your Kudu cluster is secure, in addition to running as the Kudu UNIX user, you must Hmm, breaking it out like this will make it look like a separate step. What if it were made into a WARNING? Would that look better? If not, then merge it back into the previous step, as I think that's still better than this. http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@475 PS2, Line 475: $ kudu fs dump uuid --fs_wal_dir= [--fs_data_dirs=] 2>/dev/null Could you add "sudo -u kudu" to these steps too? http://gerrit.cloudera.org:8080/#/c/9466/2/docs/administration.adoc@534 PS2, Line 534: . Copy the master data to the replacement master with the following command: Need the authentication warning here too. -- To view, visit http://gerrit.cloudera.org:8080/9466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc Gerrit-Change-Number: 9466 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Mar 2018 00:28:47 + Gerrit-HasComments: Yes
[kudu-CR] Make metrics name matching case-insensitive
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 ) Change subject: Make metrics name matching case-insensitive .. Patch Set 2: > Some metrics have mixed case names, e.g. > "handler_latency_kudu_tserver_TabletCopyService_BeginTabletCopySession". > While working on an issue with Mike recently, we wanted to look at all > tablet-copy-related metrics, so we hit /metrics?metrics=copy and were > surprised to see the above metric missing. Gotcha. Could you include that use case in the commit message, so it's more clear what sort of new functionality is enabled by the commit? -- To view, visit http://gerrit.cloudera.org:8080/9462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7 Gerrit-Change-Number: 9462 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Mar 2018 00:19:45 + Gerrit-HasComments: No
[kudu-CR] Make metrics name matching case-insensitive
Hello Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9462 to look at the new patch set (#2). Change subject: Make metrics name matching case-insensitive .. Make metrics name matching case-insensitive Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7 --- M src/kudu/util/metrics-test.cc M src/kudu/util/metrics.cc 2 files changed, 18 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/9462/2 -- To view, visit http://gerrit.cloudera.org:8080/9462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7 Gerrit-Change-Number: 9462 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG@6 PS7, Line 6: : KUDU-1704: add java client support for READ_YOUR_WRITES mode > Unfortunately, did not get a change to spend much time on it yet, but shoul ok, thats fine. really want to make sure we don't drop it though, if we're willing to go the distance in implementing RYW we should also test it properly. super weird corner cases often pop up and jepsen is one of the best tools to let you know that went wrong and why. http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java: http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@418 PS7, Line 418: if (readMode == ReadMode.READ_YOUR_WRITES && : resp.scanTimestamp != AsyncKuduClient.NO_TIMESTAMP) { : client.updateLastPropagatedTimestamp(resp.scanTimestamp); : } else if (resp.propagatedTimestamp != AsyncKuduClient.NO_TIMESTAMP) { : client.updateLastPropagatedTimestamp(resp.propagatedTimestamp); : } > I guess you are saying the comment above is not clear enough. Will update. yeah, that's why I said slightly simplified. I agree that we might want to keep the flow for consistency. the point about the comment was more important though -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 7 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 06 Mar 2018 00:13:19 + Gerrit-HasComments: Yes
[kudu-CR] Make metrics name matching case-insensitive
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9462 ) Change subject: Make metrics name matching case-insensitive .. Patch Set 1: (2 comments) > Code looks fine but what's the motivation for this change? Some metrics have mixed case names, e.g. "handler_latency_kudu_tserver_TabletCopyService_BeginTabletCopySession". While working on an issue with Mike recently, we wanted to look at all tablet-copy-related metrics, so we hit /metrics?metrics=copy and were surprised to see the above metric missing. http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics-test.cc File src/kudu/util/metrics-test.cc: http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics-test.cc@214 PS1, Line 214: // Verify that filtering is case-insensitive. > While you're there, maybe also test that it's indeed a substring match? Done http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics.cc File src/kudu/util/metrics.cc: http://gerrit.cloudera.org:8080/#/c/9462/1/src/kudu/util/metrics.cc@194 PS1, Line 194: string param_uc; > Nit: could be declared inside the loop. Done -- To view, visit http://gerrit.cloudera.org:8080/9462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49d76f969873c532e7cd297bee6fde13c98c68e7 Gerrit-Change-Number: 9462 Gerrit-PatchSet: 1 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Mar 2018 00:12:14 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG@6 PS7, Line 6: : KUDU-1704: add java client support for READ_YOUR_WRITES mode > more generally, how's the jepsen test going? it'd be a great validation of Unfortunately, did not get a change to spend much time on it yet, but should wrap up other things I am working on soon. -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 7 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 06 Mar 2018 00:01:48 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 7: (7 comments) http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java: http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@133 PS7, Line 133: return > nit: s/thus return/thus may return/ Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@200 PS7, Line 200: propagationTimestamp > in the c++ client this has a different name, right? please keep names for m Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@315 PS7, Line 315: unnecessarily wait. > grammar Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@415 PS7, Line 415: updates > s/updates/update Done http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@418 PS7, Line 418: if (readMode == ReadMode.READ_YOUR_WRITES && : resp.scanTimestamp != AsyncKuduClient.NO_TIMESTAMP) { : client.updateLastPropagatedTimestamp(resp.scanTimestamp); : } else if (resp.propagatedTimestamp != AsyncKuduClient.NO_TIMESTAMP) { : client.updateLastPropagatedTimestamp(resp.propagatedTimestamp); : } > this could be slightly simplified (choose a timestamp with the ifs, call cl I guess you are saying the comment above is not clear enough. Will update. So in general, for READ_YOUR_WRITES since we return the chosen snapshot timestamp, we should use it for as the propagation timestamp to avoid bump up the propagation timestamp unnecessarily. Since for RYW scan it is good, as long as the chosen timestamp of next read is greater than the previous one. There is an identical flow in C++ Client https://gerrit.cloudera.org/#/c/8823/20/src/kudu/client/scanner-internal.cc@491 http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1080 PS7, Line 1080: readYourWrites > do other java tests that have concurrency also use raw threads? or do they There is another java test inside this test class use raw threads. https://gerrit.cloudera.org/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@918 http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1150 PS7, Line 1150: // Fail the main thread if ever encounter AssertionError. : if (assertionError != null) { : fail("the test should not throw AssertionError: " + assertionError); : } > hum, not sure what this is doing... Since the JUnit only captures assertion errors in the main thread of the test, and it is not aware of exceptions from within new spawn threads. I added this line to fail the main thread if there is any error in the spawn ones. Will update the comment to be more clear. -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 7 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 23:59:21 + Gerrit-HasComments: Yes
[kudu-CR] [docs] Improvements to multi-master migration doc
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9466 ) Change subject: [docs] Improvements to multi-master migration doc .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/9466/1/docs/administration.adoc File docs/administration.adoc: http://gerrit.cloudera.org:8080/#/c/9466/1/docs/administration.adoc@215 PS1, Line 215: WARNING: The workflow is unsafe for adding new masters to an existing multi-master configuration, : with the one exception that it can be used, with obvious modifications, to migrate from two masters : to a multi-master configuration. Do not use it to add masters to a multi-master configuration with : three or more masters. > The WARNING seems like an odd place to note that 2->3 migrations are okay. Done http://gerrit.cloudera.org:8080/#/c/9466/1/docs/administration.adoc@235 PS1, Line 235: data > FWIW, "data" here was supposed to mean "all on-disk data" rather than just (thumbsup) http://gerrit.cloudera.org:8080/#/c/9466/1/docs/administration.adoc@236 PS1, Line 236: it > Nit: they Done http://gerrit.cloudera.org:8080/#/c/9466/1/docs/administration.adoc@298 PS1, Line 298: following command sequence, which should be run as the Kudu UNIX user, typically `kudu`: > We already have a WARNING at the top of the workflow to run everything as t In my experience, people sometimes don't read those warnings, or they read them but it takes a bit to get to this part of the workflow and they forget, or they copy-paste the commands and then modify them with their dirs but forget to run the command as kudu. Probably adding sudo -u kudu is most helpful, so I will do that. http://gerrit.cloudera.org:8080/#/c/9466/1/docs/administration.adoc@353 PS1, Line 353: If your Kudu cluster is secure, in addition to running as the Kudu UNIX user (typically : `kudu`), you must authenticate as the Kudu service user prior to running this command. > I think this would be more clear if broken up into two separate recommendat Done -- To view, visit http://gerrit.cloudera.org:8080/9466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc Gerrit-Change-Number: 9466 Gerrit-PatchSet: 1 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Mar 2018 23:31:54 + Gerrit-HasComments: Yes
[kudu-CR] [docs] Improvements to multi-master migration doc
Hello Alex Rodoni, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9466 to look at the new patch set (#2). Change subject: [docs] Improvements to multi-master migration doc .. [docs] Improvements to multi-master migration doc - Add extra reminder to run as the Kudu user. - Note that the copy_from_remote command requires authenticating to the remote service as the Kudu user. - Note that the workflow can be used to migrate 2->3 masters by making straightforward adjustments to the procedure. - Move steps for verifying the migration was successful to a new section so they are more noticeable. Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc --- M docs/administration.adoc 1 file changed, 32 insertions(+), 28 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/9466/2 -- To view, visit http://gerrit.cloudera.org:8080/9466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I77ef796f8b35729871ef8ddf2b635989278c2ebc Gerrit-Change-Number: 9466 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [WIP] [tools] Add a tool to recover master data from tablet servers
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9490 to look at the new patch set (#3). Change subject: [WIP] [tools] Add a tool to recover master data from tablet servers .. [WIP] [tools] Add a tool to recover master data from tablet servers This adds a tool that attempts to recover master metadata from tablet servers using ListTablets, writing the metadata to a syscatalog table that a new master process can use to make the cluster operational. It has several limitations. See the comment on MasterRebuilder. Note that it should be possible to fix some limitations by giving more master metadata to tablet servers. The advantage of the tool as-is is that it should work on all versions of Kudu since 1.0. WIP: Needs tests. Change-Id: If29e421d466a531ebad72e281ae27e74e458f8c6 --- M src/kudu/master/catalog_manager.h M src/kudu/tools/CMakeLists.txt A src/kudu/tools/master_rebuilder.cc A src/kudu/tools/master_rebuilder.h M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_remote_replica.cc 8 files changed, 561 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/9490/3 -- To view, visit http://gerrit.cloudera.org:8080/9490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If29e421d466a531ebad72e281ae27e74e458f8c6 Gerrit-Change-Number: 9490 Gerrit-PatchSet: 3 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8847/7//COMMIT_MSG@6 PS7, Line 6: : KUDU-1704: add java client support for READ_YOUR_WRITES mode more generally, how's the jepsen test going? it'd be a great validation of this client's implementation, since it uses it. -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6239521c022147257859e399f55c6f3f945af465 Gerrit-Change-Number: 8847 Gerrit-PatchSet: 7 Gerrit-Owner: Hao HaoGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 23:15:27 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/8847 ) Change subject: KUDU-1704: add java client support for READ_YOUR_WRITES mode .. Patch Set 7: (13 comments) http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java: http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@133 PS7, Line 133: return nit: s/thus return/thus may return/ http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@200 PS7, Line 200: propagationTimestamp in the c++ client this has a different name, right? please keep names for methods/fields as strictly consistent between clients as possible http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@315 PS7, Line 315: unnecessarily wait. grammar http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@415 PS7, Line 415: updates s/updates/update http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@418 PS7, Line 418: if (readMode == ReadMode.READ_YOUR_WRITES && : resp.scanTimestamp != AsyncKuduClient.NO_TIMESTAMP) { : client.updateLastPropagatedTimestamp(resp.scanTimestamp); : } else if (resp.propagatedTimestamp != AsyncKuduClient.NO_TIMESTAMP) { : client.updateLastPropagatedTimestamp(resp.propagatedTimestamp); : } this could be slightly simplified (choose a timestamp with the ifs, call client.updateLastPropagatedTimestamp() only once. more generally though I'm not sure I understand what this is doing, likely worth it to add a comment explaining when and why we get these timestamps and why we chose the one we chose. http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@817 PS7, Line 817: // If the last propagated timestamp is set send it with the scan. nit, comma after "set" http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@818 PS7, Line 818: propagation timestamp pet peeve: propagated timestamp or propagation timestamp? are these different? if not maybe choose one and use it consistently, likely too late for the other client, but still on time for this one. http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1044 PS7, Line 1044: running nit: "are running" or just "run" http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1047 PS7, Line 1047: // scan mode from leader replica. In this test writes are nit: comma after "scan mode" http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1062 PS7, Line 1062: // This is a test that verifies, when multiple clients running : // simultaneously, a client can get read-your-writes and : // read-your-reads session guarantees using READ_YOUR_WRITES : // scan mode from leader replica This text is repeated. Likely add it to the first test and then make the other tests point to it. http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1080 PS7, Line 1080: readYourWrites do other java tests that have concurrency also use raw threads? or do they use executors/tasks and futures? http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1086 PS7, Line 1086: 4 avoid magic numbers (set a final var) http://gerrit.cloudera.org:8080/#/c/8847/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@1150 PS7, Line 1150: // Fail the main thread if ever encounter AssertionError. : if (assertionError != null) { : fail("the test should not throw AssertionError: " + assertionError); : } hum, not sure what this is doing... -- To view, visit http://gerrit.cloudera.org:8080/8847 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id:
[kudu-CR] decimal util: do not produce warnings when building with gcc
Hello Grant Henke, Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9499 to review the following change. Change subject: decimal_util: do not produce warnings when building with gcc .. decimal_util: do not produce warnings when building with gcc When I build with gcc5, I get the following warning: ../../src/kudu/util/decimal_util.cc:31:45: warning: ‘no_sanitize’ attribute directive ignored [-Wattributes] int128_t MaxUnscaledDecimal(int8_t precision) { This is because gcc doesn't recognize the "no_sanitize" attribute, so let's deal with this as we've dealt with other sanitizer suppressions: compile it conditionally using ASAN support as a proxy. Change-Id: Iac9b145a914d551e3a18d62a0984aad34b95f4dd --- M src/kudu/gutil/port.h M src/kudu/util/decimal_util.cc 2 files changed, 17 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/9499/1 -- To view, visit http://gerrit.cloudera.org:8080/9499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iac9b145a914d551e3a18d62a0984aad34b95f4dd Gerrit-Change-Number: 9499 Gerrit-PatchSet: 1 Gerrit-Owner: Adar DemboGerrit-Reviewer: Grant Henke Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9496 ) Change subject: KUDU-721: [Python] Add DECIMAL column type support .. Patch Set 2: Code-Review+1 Let's get another review or two. -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 2 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Mon, 05 Mar 2018 22:09:15 + Gerrit-HasComments: No
[kudu-CR] KUDU-2309: /masters can show the wrong list of masters
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9378 ) Change subject: KUDU-2309: /masters can show the wrong list of masters .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache File www/masters.mustache: http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache@40 PS4, Line 40: {{^has_no_errors}} > mustache doesn't have a way to condition based on the length of the 'errors "An inverted section begins with a caret (hat) and ends with a slash. That is {{^person}} begins a "person" inverted section while {{/person}} ends it. While sections can be used to render text one or more times based on the value of the key, inverted sections may render text once based on the inverse value of the key. That is, they will be rendered if the key doesn't exist, is false, or is an empty list" https://mustache.github.io/mustache.5.html I don't think the 'has_no_errors' field is necessary. The manual says if an array is empty, it will execute the condition under {{^emptyarray}}. So I imagine you could just have a regular section after: {{#errors}} ... ... {{/errors}} {{^errors}} No errors {{/errors}} Or something similar. Alternatively, you could use Javascript to read the JSON and suppress HTML fields accordingly. -- To view, visit http://gerrit.cloudera.org:8080/9378 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26 Gerrit-Change-Number: 9378 Gerrit-PatchSet: 4 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Mar 2018 21:23:41 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for push to asf.py
Dan Burkert has removed a vote on this change. Change subject: python3 support for push_to_asf.py .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I80ea273e6eb8556fa803b2da879537df4f343a11 Gerrit-Change-Number: 9498 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] python3 support for push to asf.py
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9498 ) Change subject: python3 support for push_to_asf.py .. Patch Set 1: Verified+1 unrelated flake -- To view, visit http://gerrit.cloudera.org:8080/9498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I80ea273e6eb8556fa803b2da879537df4f343a11 Gerrit-Change-Number: 9498 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 21:22:53 + Gerrit-HasComments: No
[kudu-CR] python3 support for push to asf.py
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9498 ) Change subject: python3 support for push_to_asf.py .. python3 support for push_to_asf.py This is a fixup for 112869cf22c47543. I missed a few callers of check_output. check_output returns a byte string, but the callers assumed a string. Change-Id: I80ea273e6eb8556fa803b2da879537df4f343a11 Reviewed-on: http://gerrit.cloudera.org:8080/9498 Reviewed-by: Adar DemboTested-by: Dan Burkert --- M build-support/gen_version_info.py M build-support/push_to_asf.py 2 files changed, 3 insertions(+), 3 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Dan Burkert: Verified -- To view, visit http://gerrit.cloudera.org:8080/9498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I80ea273e6eb8556fa803b2da879537df4f343a11 Gerrit-Change-Number: 9498 Gerrit-PatchSet: 2 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9496 ) Change subject: KUDU-721: [Python] Add DECIMAL column type support .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/__init__.py File python/kudu/__init__.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/__init__.py@39 PS1, Line 39: unixtime_micros, bool_ as bool, decimal, > Do you know what's the purpose of these large import statements? Very few ( Without it tests fail with: ``` > builder.add_column('decimal_val', type_=kudu.decimal, precision=5, > scale=2) E AttributeError: 'module' object has no attribute 'decimal' ``` http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/schema.pyx File python/kudu/schema.pyx: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/schema.pyx@367 PS1, Line 367: Clients can specify a scale for decimal columns. > Nit: odd line wrapping here, and on L346 too. Done http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/test_util.py File python/kudu/tests/test_util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/test_util.py@34 PS1, Line 34: def test_to_unscaled_decimal(self): > Since you're inheriting from unittest.TestCase, you should be able to use i Done http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/util.py File python/kudu/tests/util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/util.py@20 PS1, Line 20: from decimal import * > What is this import needed for here? Done http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/util.py File python/kudu/util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/util.py@20 PS1, Line 20: from decimal import * > Can we avoid this? Seems extreme. Done -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Mon, 05 Mar 2018 21:21:07 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Hello a...@phdata.io, Wes McKinney, Kudu Jenkins, Jordan Birdsell, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9496 to look at the new patch set (#2). Change subject: KUDU-721: [Python] Add DECIMAL column type support .. KUDU-721: [Python] Add DECIMAL column type support This patch adds basic support to the Python client to create, read, and write tables with DECIMAL columns. Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 --- M python/kudu/__init__.py M python/kudu/client.pyx M python/kudu/libkudu_client.pxd M python/kudu/schema.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 A python/kudu/tests/test_util.py M python/kudu/tests/util.py M python/kudu/util.py 11 files changed, 368 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/9496/2 -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 2 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io
[kudu-CR] python3 support for push to asf.py
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9498 ) Change subject: python3 support for push_to_asf.py .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I80ea273e6eb8556fa803b2da879537df4f343a11 Gerrit-Change-Number: 9498 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 20:53:00 + Gerrit-HasComments: No
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9496 ) Change subject: KUDU-721: [Python] Add DECIMAL column type support .. Patch Set 1: It doesn't look like cpcloud (Phillip Cloud) has an account. -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Mon, 05 Mar 2018 20:51:52 + Gerrit-HasComments: No
[kudu-CR] python3 support for push to asf.py
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9498 to review the following change. Change subject: python3 support for push_to_asf.py .. python3 support for push_to_asf.py This is a fixup for 112869cf22c47543. I missed a few callers of check_output. check_output returns a byte string, but the callers assumed a string. Change-Id: I80ea273e6eb8556fa803b2da879537df4f343a11 --- M build-support/gen_version_info.py M build-support/push_to_asf.py 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/98/9498/1 -- To view, visit http://gerrit.cloudera.org:8080/9498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I80ea273e6eb8556fa803b2da879537df4f343a11 Gerrit-Change-Number: 9498 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Wes McKinney has posted comments on this change. ( http://gerrit.cloudera.org:8080/9496 ) Change subject: KUDU-721: [Python] Add DECIMAL column type support .. Patch Set 1: Could you add @cpcloud to this review? I will alert him to this CR, not sure if he has an account on gerrit.cloudera.org yet -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Mon, 05 Mar 2018 20:44:20 + Gerrit-HasComments: No
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9496 ) Change subject: KUDU-721: [Python] Add DECIMAL column type support .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/__init__.py File python/kudu/__init__.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/__init__.py@39 PS1, Line 39: unixtime_micros, bool_ as bool, decimal, Do you know what's the purpose of these large import statements? Very few (perhaps none?) of them are referenced here. http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/schema.pyx File python/kudu/schema.pyx: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/schema.pyx@367 PS1, Line 367: Clients can specify a scale for decimal columns. Nit: odd line wrapping here, and on L346 too. http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/test_util.py File python/kudu/tests/test_util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/test_util.py@34 PS1, Line 34: def test_to_unscaled_decimal(self): Since you're inheriting from unittest.TestCase, you should be able to use its assertion methods. See https://docs.python.org/2/library/unittest.html#test-cases for details. http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/util.py File python/kudu/tests/util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/tests/util.py@20 PS1, Line 20: from decimal import * What is this import needed for here? http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/util.py File python/kudu/util.py: http://gerrit.cloudera.org:8080/#/c/9496/1/python/kudu/util.py@20 PS1, Line 20: from decimal import * Can we avoid this? Seems extreme. -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jordan Birdsell Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Wes McKinney Gerrit-Reviewer: a...@phdata.io Gerrit-Comment-Date: Mon, 05 Mar 2018 20:40:44 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for some build tools
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. Patch Set 4: Verified+1 failure appears to be an unrelated flake -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 4 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 20:28:34 + Gerrit-HasComments: No
[kudu-CR] python3 support for some build tools
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. python3 support for some build tools Adds python3 support to the following files while retaining python2 compatibility: - gen_version_info.py - kudu_util.py, - push_to_asf.py Some python build tools are still known not to be python3 compatible, including: - build_source_release.py - dist_test.py - trigger_gerrit.py. These haven't been fixed either because they use urllib, which I can't figure out how to make 2 and 3 compatible, or because I don't have a dist-test compatible system with python 3 available. Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Reviewed-on: http://gerrit.cloudera.org:8080/9492 Reviewed-by: Alexey SerbinReviewed-by: Adar Dembo Tested-by: Dan Burkert --- M build-support/build_source_release.py M build-support/gen_version_info.py M build-support/kudu_util.py M build-support/push_to_asf.py 4 files changed, 84 insertions(+), 74 deletions(-) Approvals: Alexey Serbin: Looks good to me, but someone else must approve Adar Dembo: Looks good to me, approved Dan Burkert: Verified -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 5 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Fix un-defined variable in push to asf.py
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9495 ) Change subject: Fix un-defined variable in push_to_asf.py .. Fix un-defined variable in push_to_asf.py Change-Id: I7b436033826171889b123fe90a2400f2494faf1f Reviewed-on: http://gerrit.cloudera.org:8080/9495 Reviewed-by: Adar DemboTested-by: Kudu Jenkins --- M build-support/push_to_asf.py 1 file changed, 3 insertions(+), 2 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7b436033826171889b123fe90a2400f2494faf1f Gerrit-Change-Number: 9495 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] python3 support for some build tools
Dan Burkert has removed a vote on this change. Change subject: python3 support for some build tools .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 4 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9496 Change subject: KUDU-721: [Python] Add DECIMAL column type support .. KUDU-721: [Python] Add DECIMAL column type support This patch adds basic support to the Python client to create, read, and write tables with DECIMAL columns. Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 --- M python/kudu/__init__.py M python/kudu/client.pyx M python/kudu/libkudu_client.pxd M python/kudu/schema.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 A python/kudu/tests/test_util.py M python/kudu/tests/util.py M python/kudu/util.py 11 files changed, 370 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/9496/1 -- To view, visit http://gerrit.cloudera.org:8080/9496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8e0855100ab1ea891f990931ec94d0b98c0dece1 Gerrit-Change-Number: 9496 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] python3 support for some build tools
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 4 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 19:52:11 + Gerrit-HasComments: No
[kudu-CR] Fix un-defined variable in push to asf.py
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9495 ) Change subject: Fix un-defined variable in push_to_asf.py .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7b436033826171889b123fe90a2400f2494faf1f Gerrit-Change-Number: 9495 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Mar 2018 19:51:21 + Gerrit-HasComments: No
[kudu-CR] Fix un-defined variable in push to asf.py
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9495 ) Change subject: Fix un-defined variable in push_to_asf.py .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9495/1/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9495/1/build-support/push_to_asf.py@91 PS1, Line 91: Expected to URL > to find? Done -- To view, visit http://gerrit.cloudera.org:8080/9495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7b436033826171889b123fe90a2400f2494faf1f Gerrit-Change-Number: 9495 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Mar 2018 19:50:50 + Gerrit-HasComments: Yes
[kudu-CR] Fix un-defined variable in push to asf.py
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9495 to look at the new patch set (#2). Change subject: Fix un-defined variable in push_to_asf.py .. Fix un-defined variable in push_to_asf.py Change-Id: I7b436033826171889b123fe90a2400f2494faf1f --- M build-support/push_to_asf.py 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/9495/2 -- To view, visit http://gerrit.cloudera.org:8080/9495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7b436033826171889b123fe90a2400f2494faf1f Gerrit-Change-Number: 9495 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] python3 support for some build tools
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 4 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 19:50:34 + Gerrit-HasComments: No
[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9050 ) Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9050/3/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: http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1502 PS3, Line 1502: LOG.info("ConnectToCluster got authn token!"); > Did you mean to remove this or make it DEBUG? oops, yea, this was just me debugging a unit test, will remove. -- To view, visit http://gerrit.cloudera.org:8080/9050 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539 Gerrit-Change-Number: 9050 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Mar 2018 19:49:36 + Gerrit-HasComments: Yes
[kudu-CR] Fix un-defined variable in push to asf.py
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9495 ) Change subject: Fix un-defined variable in push_to_asf.py .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9495/1/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9495/1/build-support/push_to_asf.py@91 PS1, Line 91: Expected to URL to find? -- To view, visit http://gerrit.cloudera.org:8080/9495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7b436033826171889b123fe90a2400f2494faf1f Gerrit-Change-Number: 9495 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Mar 2018 19:48:40 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for some build tools
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/3/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/3/build-support/push_to_asf.py@86 PS3, Line 86: sys.exit(1) > This is Python3 compatibility as is: 'rint >>' construct does not work in P Ah, sorry -- it seems I confused the location for this comment. -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 4 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 19:43:25 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for some build tools
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/3/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/3/build-support/push_to_asf.py@86 PS3, Line 86: print("the contributor guide (git remote add gerrit %s)." % GERRIT_URL, file=sys.stderr) > Mind breaking this out into a separate patch? I know it's tiny but I think Done -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 3 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 19:43:03 + Gerrit-HasComments: Yes
[kudu-CR] Fix un-defined variable in push to asf.py
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9495 to review the following change. Change subject: Fix un-defined variable in push_to_asf.py .. Fix un-defined variable in push_to_asf.py Change-Id: I7b436033826171889b123fe90a2400f2494faf1f --- M build-support/push_to_asf.py 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/9495/1 -- To view, visit http://gerrit.cloudera.org:8080/9495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7b436033826171889b123fe90a2400f2494faf1f Gerrit-Change-Number: 9495 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo
[kudu-CR] python3 support for some build tools
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9492 to look at the new patch set (#4). Change subject: python3 support for some build tools .. python3 support for some build tools Adds python3 support to the following files while retaining python2 compatibility: - gen_version_info.py - kudu_util.py, - push_to_asf.py Some python build tools are still known not to be python3 compatible, including: - build_source_release.py - dist_test.py - trigger_gerrit.py. These haven't been fixed either because they use urllib, which I can't figure out how to make 2 and 3 compatible, or because I don't have a dist-test compatible system with python 3 available. Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 --- M build-support/build_source_release.py M build-support/gen_version_info.py M build-support/kudu_util.py M build-support/push_to_asf.py 4 files changed, 84 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/9492/4 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 4 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] python3 support for some build tools
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/3/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/3/build-support/push_to_asf.py@86 PS3, Line 86: print("the contributor guide (git remote add gerrit %s)." % GERRIT_URL, file=sys.stderr) > Mind breaking this out into a separate patch? I know it's tiny but I think This is Python3 compatibility as is: 'rint >>' construct does not work in Python3. Or the idea is to separate build-related and aux stuff? -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 3 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 19:36:35 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for some build tools
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/3/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/3/build-support/push_to_asf.py@86 PS3, Line 86: print("the contributor guide (git remote add gerrit %s)." % GERRIT_URL, file=sys.stderr) Mind breaking this out into a separate patch? I know it's tiny but I think it'd be good if this patch were JUST about Python 3 compatibility issues. -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 3 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 19:33:13 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for some build tools
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for some build tools .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/gen_version_info.py File build-support/gen_version_info.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/gen_version_info.py@23 PS2, Line 23: import hashlib > Some time ago I looked at that as well: Yeah, it appears to work on my system's 2.6 and 2.7. http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py@83 PS2, Line 83: except subprocess.CalledProcessError: : print >>sys.stderr, "No remote named 'gerrit'. Please set one up following " : print >>sys.stderr, "the contributor guide." : sys.exit(1) : if not GERRIT_URL_RE.match(url): : print >>sys.stderr, "Unexpected URL for remote 'gerrit'." : print >>sys.stderr, " Got: ", url : print >>sys.stderr, " Expected to find host '%s' in the URL" % GERRIT_HOST > Does this work in python3? nope, good catch. -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 19:28:46 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for some build tools
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9492 to look at the new patch set (#3). Change subject: python3 support for some build tools .. python3 support for some build tools Adds python3 support to the following files while retaining python2 compatibility: - gen_version_info.py - kudu_util.py, - push_to_asf.py Some python build tools are still known not to be python3 compatible, including: - build_source_release.py - dist_test.py - trigger_gerrit.py. These haven't been fixed either because they use urllib, which I can't figure out how to make 2 and 3 compatible, or because I don't have a dist-test compatible system with python 3 available. Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 --- M build-support/build_source_release.py M build-support/gen_version_info.py M build-support/kudu_util.py M build-support/push_to_asf.py 4 files changed, 85 insertions(+), 74 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/9492/3 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 3 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2309: /masters can show the wrong list of masters
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9378 ) Change subject: KUDU-2309: /masters can show the wrong list of masters .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache File www/masters.mustache: http://gerrit.cloudera.org:8080/#/c/9378/4/www/masters.mustache@40 PS4, Line 40: {{^has_no_errors}} mustache doesn't have a way to condition based on the length of the 'errors' array? That's pretty crappy :( -- To view, visit http://gerrit.cloudera.org:8080/9378 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26 Gerrit-Change-Number: 9378 Gerrit-PatchSet: 4 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Mar 2018 19:18:05 + Gerrit-HasComments: Yes
[kudu-CR] [consensus] fix on RequestForPeer clean-up
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9487 ) Change subject: [consensus] fix on RequestForPeer clean-up .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9487 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I71b3b47bbf9b5011bd11534287f6ae8f760addd5 Gerrit-Change-Number: 9487 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey SerbinGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Mar 2018 18:55:21 + Gerrit-HasComments: No
[kudu-CR] Add log parser script
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8229 ) Change subject: Add log parser script .. Patch Set 3: Code-Review+2 This is useful so let's ship it. We can improve it later. -- To view, visit http://gerrit.cloudera.org:8080/8229 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3bd8bb5d9a3c598ade7bc1bbc8f5a9e24ca618af Gerrit-Change-Number: 8229 Gerrit-PatchSet: 3 Gerrit-Owner: Mike PercyGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Mar 2018 19:05:34 + Gerrit-HasComments: No
[kudu-CR] Add log parser script
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8229 ) Change subject: Add log parser script .. Patch Set 3: Thanks Will! -- To view, visit http://gerrit.cloudera.org:8080/8229 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3bd8bb5d9a3c598ade7bc1bbc8f5a9e24ca618af Gerrit-Change-Number: 8229 Gerrit-PatchSet: 3 Gerrit-Owner: Mike PercyGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Mar 2018 19:06:04 + Gerrit-HasComments: No
[kudu-CR] Add log parser script
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8229 ) Change subject: Add log parser script .. Add log parser script This script collates and filters logs from a cluster, printing messages indicating latency issues and RaftConsensus election activity. It's a work-in-progress but I found it useful for an investigation I was doing (it highlighted some problems I hadn't noticed). It's pretty simple and basically consists of a bunch of regexes it runs on the logs. It's also a little slow and could be optimized, as it currently does an in-memory sort (by timestamp) of any included log entries right before it prints the relevant log lines to stdout. Change-Id: I3bd8bb5d9a3c598ade7bc1bbc8f5a9e24ca618af Reviewed-on: http://gerrit.cloudera.org:8080/8229 Tested-by: Kudu Jenkins Reviewed-by: Will Berkeley--- A src/kudu/scripts/kudu-log-parser.pl 1 file changed, 399 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/8229 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3bd8bb5d9a3c598ade7bc1bbc8f5a9e24ca618af Gerrit-Change-Number: 8229 Gerrit-PatchSet: 4 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [consensus] fix on RequestForPeer clean-up
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9487 ) Change subject: [consensus] fix on RequestForPeer clean-up .. [consensus] fix on RequestForPeer clean-up The consensus queue can switch to the non-leader mode when exiting from the RequestForPeer() method scope, so it's necessary to check for that condition in the scoped clean-up object before updating information on the peer's health. The issue was found while running the raft_consensus_stress-itest for the 3-2-3 re-replication scheme, so I don't think any additional test is needed to cover this change. Change-Id: I71b3b47bbf9b5011bd11534287f6ae8f760addd5 Reviewed-on: http://gerrit.cloudera.org:8080/9487 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy--- M src/kudu/consensus/consensus_queue.cc 1 file changed, 11 insertions(+), 2 deletions(-) Approvals: Kudu Jenkins: Verified Mike Percy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9487 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I71b3b47bbf9b5011bd11534287f6ae8f760addd5 Gerrit-Change-Number: 9487 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration
Jean-Daniel Cryans has posted comments on this change. ( http://gerrit.cloudera.org:8080/9050 ) Change subject: KUDU-2264. java: automatically attempt to re-acquire Kerberos credentials before expiration .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9050/3/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: http://gerrit.cloudera.org:8080/#/c/9050/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1502 PS3, Line 1502: LOG.info("ConnectToCluster got authn token!"); Did you mean to remove this or make it DEBUG? -- To view, visit http://gerrit.cloudera.org:8080/9050 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I514253e0a7f067dbc8ffe4eaf5a7a2c32900b539 Gerrit-Change-Number: 9050 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Mar 2018 18:52:29 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py@83 PS2, Line 83: except subprocess.CalledProcessError: : print >>sys.stderr, "No remote named 'gerrit'. Please set one up following " : print >>sys.stderr, "the contributor guide." : sys.exit(1) : if not GERRIT_URL_RE.match(url): : print >>sys.stderr, "Unexpected URL for remote 'gerrit'." : print >>sys.stderr, " Got: ", url : print >>sys.stderr, " Expected to find host '%s' in the URL" % GERRIT_HOST Does this work in python3? -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:43:49 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/gen_version_info.py File build-support/gen_version_info.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/gen_version_info.py@23 PS2, Line 23: import hashlib > Does it work for python 2.7? Some time ago I looked at that as well: https://gerrit.cloudera.org/#/c/8353 It seemed to me that it was necessary to do something like the following to make it work for python 2.6/2.7: try: from hashlib import sha1 except: from sha import sha as sha1 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:39:40 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py@35 PS2, Line 35: from __future__ import print_function > I was just googling this to understand it a bit more. https://docs.python.o Oh, I didn't bother googling so I assumed that, like any import, "print_function" should be referenced literally in the code somewhere. Makes sense. -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:39:04 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/gen_version_info.py File build-support/gen_version_info.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/gen_version_info.py@23 PS2, Line 23: import hashlib Does it work for python 2.7? -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:37:39 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:33:28 + Gerrit-HasComments: No
[kudu-CR] python3 support for build
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py@35 PS2, Line 35: from __future__ import print_function > Where is this used? I was just googling this to understand it a bit more. https://docs.python.org/2/reference/simple_stmts.html#future -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:31:54 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py@35 PS2, Line 35: from __future__ import print_function > Where is this used? It's necessary to get the 'file=sys.stderr' support for 'print'. -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:31:28 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py File build-support/push_to_asf.py: http://gerrit.cloudera.org:8080/#/c/9492/2/build-support/push_to_asf.py@35 PS2, Line 35: from __future__ import print_function Where is this used? -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:29:55 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2309: /masters can show the wrong list of masters
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9378 to look at the new patch set (#4). Change subject: KUDU-2309: /masters can show the wrong list of masters .. KUDU-2309: /masters can show the wrong list of masters The ListMasters RPC (on which the /masters page is based) returned masters based on the -master_addresses flag. It should return masters based on the consensus configuration. Since we verify the two sets of addresses are the same at startup, this usually makes no difference. However, if a master is replaced by a server with a new UUID, previously we would report the new UUID, even though it may not be part of the previously established quorum. This patch adds an additional check that the UUIDs match, and returns an error along with the registration information if there is a mismatch. Screenshot for down master: https://github.com/wdberkeley/kudu/blob/kudu2309screenshots/www/mastersonedown.png Screenshot for UUID mismatch: https://github.com/wdberkeley/kudu/blob/kudu2309screenshots/www/mastersmismatch.png Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26 --- M src/kudu/master/master.cc M src/kudu/master/master_path_handlers.cc M src/kudu/master/sys_catalog.h M www/masters.mustache 4 files changed, 53 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/9378/4 -- To view, visit http://gerrit.cloudera.org:8080/9378 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I603ebc22e998bac9bd00edc939577ae339587f26 Gerrit-Change-Number: 9378 Gerrit-PatchSet: 4 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] python3 support for build
Hello Kudu Jenkins, Adar Dembo, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9492 to look at the new patch set (#2). Change subject: python3 support for build .. python3 support for build Adds python3 support to gen_version_info.py, kudu_util.py, and push_to_asf.py while maintaining python2 compatibility. Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 --- M build-support/gen_version_info.py M build-support/kudu_util.py M build-support/push_to_asf.py 3 files changed, 39 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/9492/2 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 2 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] maintenance manager: log the reason for scheduling each operation
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9172 ) Change subject: maintenance_manager: log the reason for scheduling each operation .. maintenance_manager: log the reason for scheduling each operation This makes it easier to troubleshoot when the maintenance manager appears to be scheduling the "wrong" operation. Example output from a long run of full_stack_insert_scan-test: ...: Scheduling FlushMRSOp(93ded54b4dfb4e1586ff7fe700184f53): under memory pressure (60.30% used, can flush 641875735 bytes) ...: Scheduling LogGCOp(93ded54b4dfb4e1586ff7fe700184f53): free 386079935 bytes of WAL ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 29857788 bytes on disk ...: Scheduling FlushMRSOp(93ded54b4dfb4e1586ff7fe700184f53): under memory pressure (60.01% used, can flush 637714199 bytes) ...: Scheduling LogGCOp(93ded54b4dfb4e1586ff7fe700184f53): free 394543122 bytes of WAL ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.281697 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.280992 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.280256 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.060532 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.060298 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 56855045 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.054961 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 7202893 bytes on disk ...: Scheduling FlushMRSOp(93ded54b4dfb4e1586ff7fe700184f53): under memory pressure (60.25% used, can flush 633552663 bytes) ...: Scheduling LogGCOp(93ded54b4dfb4e1586ff7fe700184f53): free 394836440 bytes of WAL ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.192819 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.184820 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.184674 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 70881575 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.127476 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 18119656 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.127334 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.111677 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 31714242 bytes on disk ...: Scheduling FlushMRSOp(93ded54b4dfb4e1586ff7fe700184f53): under memory pressure (60.06% used, can flush 624189207 bytes) ...: Scheduling LogGCOp(93ded54b4dfb4e1586ff7fe700184f53): free 377818508 bytes of WAL ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 29069301 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.144540 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.138843 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 36867458 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.138827 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.122173 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 31717417 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.121637 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.121104 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.088980 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.087296 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 54371475 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.055906 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 9063001 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.031908 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 6840843 bytes on disk ...: Scheduling FlushMRSOp(93ded54b4dfb4e1586ff7fe700184f53): under memory pressure (60.08% used, can flush 614825751 bytes) ...: Scheduling LogGCOp(93ded54b4dfb4e1586ff7fe700184f53): free 377913596 bytes of WAL ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53): 28612520 bytes on disk ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.113092 ...: Scheduling CompactRowSetsOp(93ded54b4dfb4e1586ff7fe700184f53): perf score=0.110844 ...: Scheduling UndoDeltaBlockGCOp(93ded54b4dfb4e1586ff7fe700184f53):
[kudu-CR] [python] Ignore pytest cache and environment files
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9491 ) Change subject: [python] Ignore pytest cache and environment files .. [python] Ignore pytest cache and environment files Change-Id: Id77fd8fad9ca0029778dda524d7aae4a339f230d Reviewed-on: http://gerrit.cloudera.org:8080/9491 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo--- M python/.gitignore 1 file changed, 10 insertions(+), 0 deletions(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id77fd8fad9ca0029778dda524d7aae4a339f230d Gerrit-Change-Number: 9491 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [python] Ignore pytest cache and environment files
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9491 ) Change subject: [python] Ignore pytest cache and environment files .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore File python/.gitignore: http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore@31 PS1, Line 31: .pytest_cache/ > Interesting. Are you using Python 3? I am testing with both python2 and python3. It looks like both generate this. Perhaps it's a difference pytest version. -- To view, visit http://gerrit.cloudera.org:8080/9491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id77fd8fad9ca0029778dda524d7aae4a339f230d Gerrit-Change-Number: 9491 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 18:09:48 + Gerrit-HasComments: Yes
[kudu-CR] maintenance manager: log the reason for scheduling each operation
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9172 ) Change subject: maintenance_manager: log the reason for scheduling each operation .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9172 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4dcdb863a7a0b0fc2a72757801d5c057fa725c34 Gerrit-Change-Number: 9172 Gerrit-PatchSet: 2 Gerrit-Owner: Todd LipconGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Mar 2018 18:08:40 + Gerrit-HasComments: No
[kudu-CR] [WIP] [tools] Add a tool to recover master data from tablet servers
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9490 to look at the new patch set (#2). Change subject: [WIP] [tools] Add a tool to recover master data from tablet servers .. [WIP] [tools] Add a tool to recover master data from tablet servers This adds a tool that attempts to recover master metadata from tablet servers using ListTablets, writing the metadata to a syscatalog table that a new master process can use to make the cluster operational. It has several limitations. See the comment on MasterRebuilder. Note that it should be possible to fix some limitations by giving more master metadata to tablet servers. The advantage of the tool as-is is that it should work on all versions of Kudu since 1.0. WIP: Needs tests. Change-Id: If29e421d466a531ebad72e281ae27e74e458f8c6 --- M src/kudu/master/catalog_manager.h M src/kudu/tools/CMakeLists.txt A src/kudu/tools/master_rebuilder.cc A src/kudu/tools/master_rebuilder.h M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_master.cc M src/kudu/tools/tool_action_remote_replica.cc 8 files changed, 561 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/9490/2 -- To view, visit http://gerrit.cloudera.org:8080/9490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If29e421d466a531ebad72e281ae27e74e458f8c6 Gerrit-Change-Number: 9490 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [python] Ignore pytest cache and environment files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9491 ) Change subject: [python] Ignore pytest cache and environment files .. Patch Set 1: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore File python/.gitignore: http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore@31 PS1, Line 31: .pytest_cache/ > I ran `python setup.py test` and this was generated. It could definitely be Interesting. Are you using Python 3? http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore@49 PS1, Line 49: # Environments > When working with PyCharm (the Jetbrains IDE) the venv directory can be cre Makes sense, thanks. -- To view, visit http://gerrit.cloudera.org:8080/9491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id77fd8fad9ca0029778dda524d7aae4a339f230d Gerrit-Change-Number: 9491 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 17:53:54 + Gerrit-HasComments: Yes
[kudu-CR] [python] Ignore pytest cache and environment files
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9491 ) Change subject: [python] Ignore pytest cache and environment files .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore File python/.gitignore: http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore@31 PS1, Line 31: .pytest_cache/ > What tool generates these? Can't say I've seen them when I tested Python ch I ran `python setup.py test` and this was generated. It could definitely be a situation where I am doing something in a "non standard" way. I am far from a python expert and it doesn't look like we have any development oriented docs around building and testing the python client. I will look to add some, but will need so review from someone more educated on python than me. http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore@49 PS1, Line 49: # Environments > Hmm, how do these get created? In my experience, you run "virtualenv foo" a When working with PyCharm (the Jetbrains IDE) the venv directory can be created. However I pulled in the others from githubs gitignore project, expecting it wasn't totally standard. (https://github.com/github/gitignore/blob/master/Python.gitignore) -- To view, visit http://gerrit.cloudera.org:8080/9491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id77fd8fad9ca0029778dda524d7aae4a339f230d Gerrit-Change-Number: 9491 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 17:47:23 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9492 ) Change subject: python3 support for build .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 17:40:54 + Gerrit-HasComments: No
[kudu-CR] [python] Ignore pytest cache and environment files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9491 ) Change subject: [python] Ignore pytest cache and environment files .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore File python/.gitignore: http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore@31 PS1, Line 31: .pytest_cache/ What tool generates these? Can't say I've seen them when I tested Python changes locally. http://gerrit.cloudera.org:8080/#/c/9491/1/python/.gitignore@49 PS1, Line 49: # Environments Hmm, how do these get created? In my experience, you run "virtualenv foo" and "foo" is the virtualenv directory, so there's no standard name. Are these standard names for some other tool? -- To view, visit http://gerrit.cloudera.org:8080/9491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id77fd8fad9ca0029778dda524d7aae4a339f230d Gerrit-Change-Number: 9491 Gerrit-PatchSet: 1 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Mar 2018 17:36:51 + Gerrit-HasComments: Yes
[kudu-CR] python3 support for build
Hello Grant Henke, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9492 to review the following change. Change subject: python3 support for build .. python3 support for build makes gen_version_info.py and kudu_util.py python3 as well as python2 compliant. Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 --- M build-support/gen_version_info.py M build-support/kudu_util.py 2 files changed, 9 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/9492/1 -- To view, visit http://gerrit.cloudera.org:8080/9492 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I547d870a54c7d396d2706d63f65844df01ff7c57 Gerrit-Change-Number: 9492 Gerrit-PatchSet: 1 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Grant Henke
[kudu-CR] [python] Ignore pytest cache and environment files
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9491 Change subject: [python] Ignore pytest cache and environment files .. [python] Ignore pytest cache and environment files Change-Id: Id77fd8fad9ca0029778dda524d7aae4a339f230d --- M python/.gitignore 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/9491/1 -- To view, visit http://gerrit.cloudera.org:8080/9491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id77fd8fad9ca0029778dda524d7aae4a339f230d Gerrit-Change-Number: 9491 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2230: java: sync client exception stack traces should make sense
Hello Alexey Serbin, Jean-Daniel Cryans, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9489 to review the following change. Change subject: KUDU-2230: java: sync client exception stack traces should make sense .. KUDU-2230: java: sync client exception stack traces should make sense Previously the exceptions thrown by the synchronous client always had a stack trace somewhere deep inside our async callback chain. So, when the user eventually caught this exception, it would be very hard to even know what part of their code failed, since their own code would not even show up in the call chain. This patch simply replaces the exception's stack trace with the stack at the "join" point where the exception is transformed back to a KuduException. We lose a bit of info that might be helpful for debugging of internals, but the positive here strongly outweighs the negative. Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 --- M java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java M java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java 2 files changed, 11 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/9489/1 -- To view, visit http://gerrit.cloudera.org:8080/9489 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Gerrit-Change-Number: 9489 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Jean-Daniel Cryans