[kudu-CR] build: stop generating md5 file

2018-03-05 Thread Dan Burkert (Code Review)
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 Lipcon 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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 Dembo 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Lipcon 
Gerrit-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

2018-03-05 Thread Hao Hao (Code Review)
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 Hao 
Gerrit-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

2018-03-05 Thread Hao Hao (Code Review)
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 Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] build: stop generating md5 file

2018-03-05 Thread Todd Lipcon (Code Review)
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 Lipcon 
Gerrit-Reviewer: Dan Burkert 


[kudu-CR] dist test: fix the ability to pass flags to the 'run' option

2018-03-05 Thread Adar Dembo (Code Review)
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 Lipcon 
Gerrit-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

2018-03-05 Thread Todd Lipcon (Code Review)
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 Lipcon 
Gerrit-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

2018-03-05 Thread Andrew Wong (Code Review)
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 Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] Make metrics name matching case-insensitive

2018-03-05 Thread Andrew Wong (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Todd Lipcon (Code Review)
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 Lipcon 
Gerrit-Reviewer: Adar Dembo 


[kudu-CR] dist test: fix the ability to pass flags to the 'run' option

2018-03-05 Thread Todd Lipcon (Code Review)
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 Lipcon 
Gerrit-Reviewer: Adar Dembo 


[kudu-CR] [docs] Improvements to multi-master migration doc

2018-03-05 Thread Andrew Wong (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread David Ribeiro Alves (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Hao Hao (Code Review)
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 Hao 
Gerrit-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

2018-03-05 Thread Hao Hao (Code Review)
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 Hao 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 


[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode

2018-03-05 Thread David Ribeiro Alves (Code Review)
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 Hao 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Hao Hao (Code Review)
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 Hao 
Gerrit-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

2018-03-05 Thread Hao Hao (Code Review)
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 Hao 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] KUDU-1704: add java client support for READ YOUR WRITES mode

2018-03-05 Thread David Ribeiro Alves (Code Review)
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 Hao 
Gerrit-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

2018-03-05 Thread David Ribeiro Alves (Code Review)
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

2018-03-05 Thread Adar Dembo (Code Review)
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 Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support

2018-03-05 Thread Adar Dembo (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Sailesh Mukil (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] python3 support for push to asf.py

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Dembo 
Tested-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

2018-03-05 Thread Grant Henke (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-Reviewer: Adar Dembo 


[kudu-CR] KUDU-721: [Python] Add DECIMAL column type support

2018-03-05 Thread Wes McKinney (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Serbin 
Reviewed-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Dembo 
Tested-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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

2018-03-05 Thread Adar Dembo (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] python3 support for some build tools

2018-03-05 Thread Alexey Serbin (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Todd Lipcon (Code Review)
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 Lipcon 
Gerrit-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

2018-03-05 Thread Todd Lipcon (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Alexey Serbin (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-Reviewer: Adar Dembo 


[kudu-CR] python3 support for some build tools

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Alexey Serbin (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Todd Lipcon (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Mike Percy (Code Review)
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 Serbin 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Percy 
Gerrit-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

2018-03-05 Thread Mike Percy (Code Review)
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 Percy 
Gerrit-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

2018-03-05 Thread Mike Percy (Code Review)
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

2018-03-05 Thread Alexey Serbin (Code Review)
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

2018-03-05 Thread Jean-Daniel Cryans (Code Review)
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 Lipcon 
Gerrit-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

2018-03-05 Thread Alexey Serbin (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Alexey Serbin (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Alexey Serbin (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Berkeley 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] maintenance manager: log the reason for scheduling each operation

2018-03-05 Thread Todd Lipcon (Code Review)
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

2018-03-05 Thread Grant Henke (Code Review)
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

2018-03-05 Thread Grant Henke (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Lipcon 
Gerrit-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

2018-03-05 Thread Will Berkeley (Code Review)
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 Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot


[kudu-CR] [python] Ignore pytest cache and environment files

2018-03-05 Thread Adar Dembo (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Grant Henke (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Burkert 
Gerrit-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

2018-03-05 Thread Adar Dembo (Code Review)
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 Henke 
Gerrit-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

2018-03-05 Thread Dan Burkert (Code Review)
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 Burkert 
Gerrit-Reviewer: Grant Henke 


[kudu-CR] [python] Ignore pytest cache and environment files

2018-03-05 Thread Grant Henke (Code Review)
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

2018-03-05 Thread Todd Lipcon (Code Review)
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 Lipcon 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans