[kudu-CR](branch-1.6.x) Don't perform compactions when clean time has not been advanced

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9482 )

Change subject: Don't perform compactions when clean time has not been advanced
..

Don't perform compactions when clean time has not been advanced

Due to KUDU-2233 we might not advance safe time, and thus clean
time, at bootstrap causing possible corruption or crashes.

This patch adds a check to make sure that clean time has been
advanced at all before performing a compaction.

This is temporary fix until we have a more strict check like the
one proposed in https://gerrit.cloudera.org/#/c/8887/.

Change-Id: Ia74abdf7d806efc4239dc9cff4a5da28621d331a
Reviewed-on: http://gerrit.cloudera.org:8080/9436
Reviewed-by: Todd Lipcon 
Tested-by: David Ribeiro Alves 
(cherry picked from commit 00815045fc3295c12023fd7ae7ad220645a10c3a)
Reviewed-on: http://gerrit.cloudera.org:8080/9482
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/tablet/tablet.cc
1 file changed, 7 insertions(+), 0 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  David Ribeiro Alves: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/9482
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia74abdf7d806efc4239dc9cff4a5da28621d331a
Gerrit-Change-Number: 9482
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.6.x) Optionally advance safe time with non-write operations

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9481 )

Change subject: Optionally advance safe time with non-write operations
..

Optionally advance safe time with non-write operations

We currently have a problem with safe time advancement when
there are no write messages in the WAL. Because of KUDU-2233,
NO_OP and CHANGE_CONFIG messages are not guaranteed to have
monotonic timestamps, which forced us to ignore them in the
past. This can cause compactions to execute with a timestamp
that is in the beginning of time, causing corruption or crashes.

This patch enables us to advance safe time for all messages
if a flag a set.

Once KUDU-2233 is solved we can flip the flag or remove it
altogether.

Change-Id: I99de2284ba1626581c6e0770c823fd1ba714c1d7
Reviewed-on: http://gerrit.cloudera.org:8080/9396
Tested-by: David Ribeiro Alves 
Reviewed-by: Todd Lipcon 
(cherry picked from commit 611627f2c2e1137f50373954090fa9f7a76badef)
Reviewed-on: http://gerrit.cloudera.org:8080/9481
Reviewed-by: David Ribeiro Alves 
Tested-by: Alexey Serbin 
---
M src/kudu/tablet/tablet_bootstrap.cc
1 file changed, 12 insertions(+), 6 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Alexey Serbin: Verified

--
To view, visit http://gerrit.cloudera.org:8080/9481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I99de2284ba1626581c6e0770c823fd1ba714c1d7
Gerrit-Change-Number: 9481
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.6.x) Optionally advance safe time with non-write operations

[Alexey Serbin (Code Review)]

Alexey Serbin has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/9481 )

Change subject: Optionally advance safe time with non-write operations
..


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I99de2284ba1626581c6e0770c823fd1ba714c1d7
Gerrit-Change-Number: 9481
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.6.x) Optionally advance safe time with non-write operations

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9481 )

Change subject: Optionally advance safe time with non-write operations
..


Patch Set 1: Verified+1

Unrelated flake due to unsyncnronizeed clock:

Bad status: Service unavailable: Couldn't start the single master: Couldn't 
start master: Cannot initialize clock: Error reading clock. Clock considered 
unsynchronized


--
To view, visit http://gerrit.cloudera.org:8080/9481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I99de2284ba1626581c6e0770c823fd1ba714c1d7
Gerrit-Change-Number: 9481
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 06:20:50 +
Gerrit-HasComments: No

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

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 19: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 19
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Sat, 03 Mar 2018 04:06:08 +
Gerrit-HasComments: No

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

[Hao Hao (Code Review)]

Hello Tidy Bot, Alex Rodoni, Mike Percy, Dan Burkert, David Ribeiro Alves, Kudu 
Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8823

to look at the new patch set (#19).

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..

KUDU-1704: add c++ client support for READ_YOUR_WRITES mode

Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_configuration.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
10 files changed, 404 insertions(+), 94 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/23/8823/19
--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 19
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

[kudu-CR](branch-1.6.x) Don't perform compactions when clean time has not been advanced

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9482 )

Change subject: Don't perform compactions when clean time has not been advanced
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9482
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia74abdf7d806efc4239dc9cff4a5da28621d331a
Gerrit-Change-Number: 9482
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 04:04:02 +
Gerrit-HasComments: No

[kudu-CR](branch-1.6.x) Optionally advance safe time with non-write operations

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9481 )

Change subject: Optionally advance safe time with non-write operations
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I99de2284ba1626581c6e0770c823fd1ba714c1d7
Gerrit-Change-Number: 9481
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 04:04:25 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2312: Scan predicate application ordering is non-deterministic

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9440 )

Change subject: KUDU-2312: Scan predicate application ordering is 
non-deterministic
..


Patch Set 4:

Whoops sorry I thought I responded to your previous comment but looks like it 
got lost.

> If we changed this to be a std::stable_sort instead of tie-breaking with 
> index, would we end up retaining the client's predicate order, you think?

No, we put the predicates into an hash map, and at that point they lose all 
ordering (ScanSpec::predicates_).  The /scans page is also populated from this 
unorderd map, so it doesn't reflect the true ordering either.  I don't think 
it's wise to guarantee an ordering externally.  Ideally we'd be using our own 
internal stats to do the predicate evaluation ordering on a per-cfile basis, so 
there wouldn't even be a consistent global ordering.


--
To view, visit http://gerrit.cloudera.org:8080/9440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b2cabecd8626cad7e11fbdd492af7276e08348
Gerrit-Change-Number: 9440
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 02:22:14 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2312: Scan predicate application ordering is non-deterministic

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9440 )

Change subject: KUDU-2312: Scan predicate application ordering is 
non-deterministic
..


Patch Set 4:

I did a little research and it actually appears that Impala is ordering 
predicates based on selectivity. See 
https://gist.github.com/89344b31e455a72a831761b5c6ef75f7

That said, I checked a version of kudu without this patch and the scans 
dashboard shows that the predicates were being evaluated in the same order 
(field5 followed by field4) regardless of what Impala said in the explain. I 
also checked an RPC trace and can see that the actual RPC is showing the 
predicates in the order [field4, field5] regardless of what the explain plan 
says. So it seems like our client is likely dropping the order somewhere during 
the scan optimization phase.

If we changed this to be a std::stable_sort instead of tie-breaking with index, 
would we end up retaining the client's predicate order, you think? or would it 
be a bigger change? Certainly seems like it would be valuable to take advantage 
of Impala's stats knowledge in our order of application.


--
To view, visit http://gerrit.cloudera.org:8080/9440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b2cabecd8626cad7e11fbdd492af7276e08348
Gerrit-Change-Number: 9440
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 01:52:37 +
Gerrit-HasComments: No

[kudu-CR] build: enable sharding within cmake/ctest

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9470

to look at the new patch set (#4).

Change subject: build: enable sharding within cmake/ctest
..

build: enable sharding within cmake/ctest

This changes test sharding to be set up via cmake rather than specialized code
in the dist-test wrapper:

* There is a new argument for the ADD_KUDU_TEST() CMake function. When
  set, we generate separate CTest executions for each shard of the
  test. Thus, 'ctest -j' can now parallelize the multiple shards of
  longer-running tests.

* The specialized sharding logic is now gone from dist_test.py. Instead,
  it is now able to parse more of the output from 'ctest -N' and grabs
  the sharding-related environment variables directly from there and
  passes them down into the test environment.

  A few other changes sprung out of this:
- 'dist-test.py loop' no longer understands shards and thus always
  submits a non-sharded test. Its functionality is now available
  with sharding in the 'run' subcommand, which is an improved
  version of the old 'run-all'.
- the 'run' command can now pass through the '-R' regex filter
  parameter to ctest. For example, 'dist_test.py run -R consensus'
  will run all tests with consensus in the name
- the 'run' command can also now loop tests with the '-n' flag.
  Thus we preserve the ability to loop a sharded test suite.
- the 'run' command can also now tack on extra arguments to the end
  of all tests that it submits, which is handy for looping a sharded
  test suite with --stress-cpu-threads or other common test flags.

* The setting of the GTEST_OUTPUT environment variable is moved from
  build-and-test.sh into run_test.sh since it's necessary to ensure
  that the different shards output to separate XML files.

* Flaky-test tracking is currently still done by the test binary name
  and not the specific shard, though we could easily switch that in
  the future.

Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/jenkins/build-and-test.sh
M build-support/run-test.sh
M build-support/run_dist_test.py
M src/kudu/cfile/CMakeLists.txt
M src/kudu/client/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tools/CMakeLists.txt
11 files changed, 223 insertions(+), 139 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/9470/4
--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] dist test: add a cache for ldd results

[Todd Lipcon (Code Review)]

Hello Adar Dembo,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9483

to review the following change.


Change subject: dist_test: add a cache for ldd results
..

dist_test: add a cache for ldd results

With ctest-based sharding we ended up running 'ldd' many times on the
same executable, which isn't necessary. This adds a simple cache for the
results of ldd.

Change-Id: I350ce78d61d9f4428424350ea6b41d4e61e3a4ae
---
M build-support/dist_test.py
1 file changed, 7 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/9483/1
--
To view, visit http://gerrit.cloudera.org:8080/9483
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I350ce78d61d9f4428424350ea6b41d4e61e3a4ae
Gerrit-Change-Number: 9483
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR](branch-1.6.x) Don't perform compactions when clean time has not been advanced

[Alexey Serbin (Code Review)]

Hello David Ribeiro Alves, Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9482

to review the following change.


Change subject: Don't perform compactions when clean time has not been advanced
..

Don't perform compactions when clean time has not been advanced

Due to KUDU-2233 we might not advance safe time, and thus clean
time, at bootstrap causing possible corruption or crashes.

This patch adds a check to make sure that clean time has been
advanced at all before performing a compaction.

This is temporary fix until we have a more strict check like the
one proposed in https://gerrit.cloudera.org/#/c/8887/.

Change-Id: Ia74abdf7d806efc4239dc9cff4a5da28621d331a
Reviewed-on: http://gerrit.cloudera.org:8080/9436
Reviewed-by: Todd Lipcon 
Tested-by: David Ribeiro Alves 
(cherry picked from commit 00815045fc3295c12023fd7ae7ad220645a10c3a)
---
M src/kudu/tablet/tablet.cc
1 file changed, 7 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/9482/1
--
To view, visit http://gerrit.cloudera.org:8080/9482
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia74abdf7d806efc4239dc9cff4a5da28621d331a
Gerrit-Change-Number: 9482
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.6.x) Optionally advance safe time with non-write operations

[Alexey Serbin (Code Review)]

Hello David Ribeiro Alves, Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9481

to review the following change.


Change subject: Optionally advance safe time with non-write operations
..

Optionally advance safe time with non-write operations

We currently have a problem with safe time advancement when
there are no write messages in the WAL. Because of KUDU-2233,
NO_OP and CHANGE_CONFIG messages are not guaranteed to have
monotonic timestamps, which forced us to ignore them in the
past. This can cause compactions to execute with a timestamp
that is in the beginning of time, causing corruption or crashes.

This patch enables us to advance safe time for all messages
if a flag a set.

Once KUDU-2233 is solved we can flip the flag or remove it
altogether.

Change-Id: I99de2284ba1626581c6e0770c823fd1ba714c1d7
Reviewed-on: http://gerrit.cloudera.org:8080/9396
Tested-by: David Ribeiro Alves 
Reviewed-by: Todd Lipcon 
(cherry picked from commit 611627f2c2e1137f50373954090fa9f7a76badef)
---
M src/kudu/tablet/tablet_bootstrap.cc
1 file changed, 12 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/9481/1
--
To view, visit http://gerrit.cloudera.org:8080/9481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.6.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I99de2284ba1626581c6e0770c823fd1ba714c1d7
Gerrit-Change-Number: 9481
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] build: enable sharding within cmake/ctest

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9470 )

Change subject: build: enable sharding within cmake/ctest
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py
File build-support/dist_test.py:

http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@442
PS2, Line 442: 
> hmm, how about we take a page from ctest and have "dist_test.py loop -n 100
We talked about this offline because I wanted clarification. What you're 
suggesting is two-fold:
1. Augment 'run-all' with an argument to match test names by regex, and another 
argument to loop. This is effectively the new (and only) way to run, merging 
the functionality of both existing commands. The regex can be used to select a 
particular shard, a subset of shards, a subset of shards, or all shards. It can 
also be used to select a particular test or subset of tests (orthogonally).
2. Leave 'loop' as a vestigial, deprecated command that only operates on test 
binaries and isn't aware of sharding. If you need to shard a test to prevent it 
from timing out, you have to use 'run-all'.

+1



--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 01:08:30 +
Gerrit-HasComments: Yes

[kudu-CR] debug-util-test: add status message when stacks fail

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9471 )

Change subject: debug-util-test: add status message when stacks fail
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f1da4658c2fbb8e4a049edc392b15f7bf14c8fa
Gerrit-Change-Number: 9471
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:57:16 +
Gerrit-HasComments: No

[kudu-CR] build: use thin static libraries on Linux

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9472 )

Change subject: build: use thin static libraries on Linux
..


Patch Set 2: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If662cea380e06eaddf45e52617e38e55e4613773
Gerrit-Change-Number: 9472
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:57:01 +
Gerrit-HasComments: No

[kudu-CR] build: enable sharding within cmake/ctest

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9470 )

Change subject: build: enable sharding within cmake/ctest
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py
File build-support/dist_test.py:

http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@442
PS2, Line 442:
> I see. I've often used loop to loop an entire suite. For example, I'd loop
hmm, how about we take a page from ctest and have "dist_test.py loop -n 100 -R 
" which uses 'ctest -R " to find the matching tests? Then I think 
we'd get the behavior you want but still allow "loop build/latest/bin/foo-test"?



--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:51:57 +
Gerrit-HasComments: Yes

[kudu-CR] build: use thin static libraries on Linux

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9472

to look at the new patch set (#2).

Change subject: build: use thin static libraries on Linux
..

build: use thin static libraries on Linux

Thin static libraries just contain pointers to .o files rather than copying all
of the data. This reduces the amount of IO during the build as well as the 
required
disk space.

For example, comparing a clean build ('ninja clean kudu') before and after this
change, we can see that the build is about 40% faster and about 200x
less disk space in the lib/ directory.

Thick (original):
  real0m24.784s
  user0m58.643s
  sys 0m37.701s

  todd@ve0518:/data/1/todd/kudu$ du -sm build/release/lib/
  594 build/release/lib/

Thin:
  real0m17.422s
  user0m56.092s
  sys 0m34.131s
  todd@ve0518:/data/1/todd/kudu$ du -sm build/release/lib/
  3   build/release/lib/

For comparison, a release build with -DKUDU_LINK=dynamic:

  real0m24.008s
  user2m3.212s
  sys 1m5.243s
  todd@ve0518:/data/1/todd/kudu/build/release-shared$ du -sm lib/
  256 lib/

Of course the dynamic build has a much smaller resulting 'kudu' binary, and if
we are compiling many binaries (eg all the tests), the dynamic build is still
faster and smaller than a thin-static build.

Change-Id: If662cea380e06eaddf45e52617e38e55e4613773
---
M CMakeLists.txt
1 file changed, 9 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/9472/2
--
To view, visit http://gerrit.cloudera.org:8080/9472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If662cea380e06eaddf45e52617e38e55e4613773
Gerrit-Change-Number: 9472
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] debug-util-test: add status message when stacks fail

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9471

to look at the new patch set (#2).

Change subject: debug-util-test: add status message when stacks fail
..

debug-util-test: add status message when stacks fail

The TestSnapshot test case is flaky, and it seems to be because one of the
threads sometimes fails to take a stack trace. This patch just adds the Status
to the log so that we can diagnose the flake next time it happens.

Change-Id: I4f1da4658c2fbb8e4a049edc392b15f7bf14c8fa
---
M src/kudu/util/debug-util-test.cc
1 file changed, 3 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/9471/2
--
To view, visit http://gerrit.cloudera.org:8080/9471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f1da4658c2fbb8e4a049edc392b15f7bf14c8fa
Gerrit-Change-Number: 9471
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] build: enable sharding within cmake/ctest

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9470 )

Change subject: build: enable sharding within cmake/ctest
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py
File build-support/dist_test.py:

http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@442
PS2, Line 442:
> yea, though with looping a test you specify the whole command line yourself
I see. I've often used loop to loop an entire suite. For example, I'd loop 
raft_consensus-itest 1000 times and wait for 8000 test instances to finish 
running. Without sharding, I'd get 1000 mega-instances that may individually 
time out, right?

What if we parsed argv for the test name(s), then looked for them in the output 
of ctest -N? I guess it'd get pretty annoying if we allowed both sharded and 
non-sharded test names on the command line.


http://gerrit.cloudera.org:8080/#/c/9470/3/build-support/dist_test.py
File build-support/dist_test.py:

http://gerrit.cloudera.org:8080/#/c/9470/3/build-support/dist_test.py@449
PS3, Line 449: argv=(
Is naming the argument actually necessary? Won't the first argument get 
marshalled into 'argv', the second (which doesn't exist here) into 'env', etc?



--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:41:00 +
Gerrit-HasComments: Yes

[kudu-CR] build: enable sharding within cmake/ctest

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9470

to look at the new patch set (#3).

Change subject: build: enable sharding within cmake/ctest
..

build: enable sharding within cmake/ctest

This changes test sharding to be set up via cmake rather than specialized code
in the dist-test wrapper:

* There is a new argument for the ADD_KUDU_TEST() CMake function. When
  set, we generate separate CTest executions for each shard of the
  test. Thus, 'ctest -j' can now parallelize the multiple shards of
  longer-running tests.

* The specialized sharding logic is now gone from dist_test.py. Instead,
  it is now able to parse more of the output from 'ctest -N' and grabs
  the sharding-related environment variables directly from there and
  passes them down into the test environment.

* The setting of the GTEST_OUTPUT environment variable is moved from
  build-and-test.sh into run_test.sh since it's necessary to ensure
  that the different shards output to separate XML files.

* Flaky-test tracking is currently still done by the test binary name
  and not the specific shard, though we could easily switch that in
  the future.

Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/jenkins/build-and-test.sh
M build-support/run-test.sh
M build-support/run_dist_test.py
M src/kudu/cfile/CMakeLists.txt
M src/kudu/client/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tools/CMakeLists.txt
11 files changed, 171 insertions(+), 126 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/9470/3
--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] build: enable sharding within cmake/ctest

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9470 )

Change subject: build: enable sharding within cmake/ctest
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/9470/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9470/2//COMMIT_MSG@26
PS2, Line 26: * Flaky-test tracking is currently still done by the test binary 
name
:   and not the specific shard, though we could easily switch that 
in
:   the future.
> This gets weird though because the number of shards may change over time, w
yea, that's why I didn't change for now.. on the other hand it can be useful to 
know which sub-shard is actually flaky. I suppose longest term it woudl be nice 
to even track each test-case separately but that woudl require more mechanics


http://gerrit.cloudera.org:8080/#/c/9470/2/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/9470/2/CMakeLists.txt@678
PS2, Line 678: If a test suite is long enough
 : #   to require a bumped timeout, consider enabling sharding 
of the
 : #   test by adding it to the NUM_SHARDS_BY_TEST dictionary 
in dist_test.py.
> Update.
Done


http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py
File build-support/dist_test.py:

http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@56
PS2, Line 56: TEST_COMMAND_RE = re.compile('Test command: (.+)$')
: TEST_ENV_RE = re.compile('^\d+:  (\S+)=(.+)')
: LDD_RE = re.compile(r'^\s+.+? => (\S+) \(0x.+\)')
> Would be nice to include an example string for each of these.
Done


http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@322
PS2, Line 322:   name=execution.test_name,
> Note that prior to your change the name of the test also included the total
it seems like this 'name' actually never ends up displayed anywhere best I can 
tell. It probably corresponds to some ancient version of isolate. I'll just 
remove it.


http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@442
PS2, Line 442:   execution = dict(argv=(["run-test.sh", options.cmd] + 
options.args), env={})
> This isn't right, I think it should be:
yea, though with looping a test you specify the whole command line yourself and 
typically filter to a single test. In other words, I almost always used the 
loop mode along with --disable-sharding. I couldn't figure out the best way to 
modify this for the new scheme. But you're right I also broke the existing 
functionality.



--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:32:50 +
Gerrit-HasComments: Yes

[kudu-CR] build: use thin static libraries on Linux

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9472 )

Change subject: build: use thin static libraries on Linux
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9472/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9472/1//COMMIT_MSG@7
PS1, Line 7: build: use thin static libraries on Linux
Neat trick; how did you discover it?


http://gerrit.cloudera.org:8080/#/c/9472/1//COMMIT_MSG@14
PS1, Line 14: 40% faster
Out of curiosity, how does this compare to a shared object build?


http://gerrit.cloudera.org:8080/#/c/9472/1/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/9472/1/CMakeLists.txt@1158
PS1, Line 1158: endif ()
$ git grep "endif (" | wc -l
17
$ git grep "endif(" | wc -l
149


http://gerrit.cloudera.org:8080/#/c/9472/1/CMakeLists.txt@1159
PS1, Line 1159:
Nit: extra empty line here.



--
To view, visit http://gerrit.cloudera.org:8080/9472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If662cea380e06eaddf45e52617e38e55e4613773
Gerrit-Change-Number: 9472
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:29:19 +
Gerrit-HasComments: Yes

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

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 18: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:28:29 +
Gerrit-HasComments: No

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

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 18: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:27:58 +
Gerrit-HasComments: No

[kudu-CR] debug-util-test: add status message when stacks fail

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9471 )

Change subject: debug-util-test: add status message when stacks fail
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9471/1/src/kudu/util/debug-util-test.cc
File src/kudu/util/debug-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/9471/1/src/kudu/util/debug-util-test.cc@189
PS1, Line 189:   for (auto& info : group) {
Nit: could be cref, right?



--
To view, visit http://gerrit.cloudera.org:8080/9471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4f1da4658c2fbb8e4a049edc392b15f7bf14c8fa
Gerrit-Change-Number: 9471
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:20:27 +
Gerrit-HasComments: Yes

[kudu-CR] remove old Cyrus-SASL references from thirdparty

[Adar Dembo (Code Review)]

Adar Dembo has abandoned this change. ( http://gerrit.cloudera.org:8080/9474 )

Change subject: remove old Cyrus-SASL references from thirdparty
..


Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/9474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I6b36dc09197945688ed779897760cd212195aa31
Gerrit-Change-Number: 9474
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] remove libcurl from Kudu dependencies

[Adar Dembo (Code Review)]

Adar Dembo has abandoned this change. ( http://gerrit.cloudera.org:8080/9473 )

Change subject: remove libcurl from Kudu dependencies
..


Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/9473
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I0e2a0d10e2ec75bbc365d20ba4a80a6abb2726e0
Gerrit-Change-Number: 9473
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] test result server: better handling for non-ASCII in logs

[Adar Dembo (Code Review)]

Adar Dembo has abandoned this change. ( http://gerrit.cloudera.org:8080/9478 )

Change subject: test_result_server: better handling for non-ASCII in logs
..


Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/9478
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Iac6da45ffe6c473dc985e60a4471ba708bae4610
Gerrit-Change-Number: 9478
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] build: only use -fPIC when absolutely necessary

[Adar Dembo (Code Review)]

Adar Dembo has abandoned this change. ( http://gerrit.cloudera.org:8080/9476 )

Change subject: build: only use -fPIC when absolutely necessary
..


Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/9476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I9e5c5b2d6e973a43e042709158e2d42f75c3739c
Gerrit-Change-Number: 9476
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Replace ASSERT STATUS OK with ASSERT OK

[Adar Dembo (Code Review)]

Adar Dembo has abandoned this change. ( http://gerrit.cloudera.org:8080/9475 )

Change subject: Replace ASSERT_STATUS_OK with ASSERT_OK
..


Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/9475
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I4327f87fb98b186a8f83cf211c38fbddd08c3b05
Gerrit-Change-Number: 9475
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] client: bump default timeouts

[Adar Dembo (Code Review)]

Adar Dembo has abandoned this change. ( http://gerrit.cloudera.org:8080/9477 )

Change subject: client: bump default timeouts
..


Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/9477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ie2be96f0095d2da76d943eab383b2d53d72ebe5d
Gerrit-Change-Number: 9477
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: exported kudu client static archives

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9479

to review the following change.


Change subject: WIP: exported kudu client static archives
..

WIP: exported kudu client static archives

This is an attempt to produce a static libkudu_client.a deliverable. I
think; I haven't looked at it since early 2015.

DONT_BUILD

Change-Id: I233972163aabfe6bb36aba02616b3c03fc4041e0
---
M CMakeLists.txt
M src/kudu/cfile/CMakeLists.txt
M src/kudu/client/CMakeLists.txt
M src/kudu/common/CMakeLists.txt
M src/kudu/consensus/CMakeLists.txt
M src/kudu/fs/CMakeLists.txt
M src/kudu/gutil/CMakeLists.txt
M src/kudu/master/CMakeLists.txt
M src/kudu/rpc/CMakeLists.txt
M src/kudu/server/CMakeLists.txt
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/util/CMakeLists.txt
13 files changed, 98 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/9479/1
--
To view, visit http://gerrit.cloudera.org:8080/9479
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I233972163aabfe6bb36aba02616b3c03fc4041e0
Gerrit-Change-Number: 9479
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Replace ASSERT STATUS OK with ASSERT OK

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9475

to review the following change.


Change subject: Replace ASSERT_STATUS_OK with ASSERT_OK
..

Replace ASSERT_STATUS_OK with ASSERT_OK

Used the following command line:

  for f in $(git grep -l ASSERT_STATUS_OK) do
sed -i s/ASSERT_STATUS_OK/ASSERT_OK/g $f
  done

ASSERT_STATUS_OK_FAST comes along for the ride.

Change-Id: I4327f87fb98b186a8f83cf211c38fbddd08c3b05
---
M src/kudu/benchmarks/tpch/rpc_line_item_dao-test.cc
M src/kudu/cfile/bloomfile-test-base.h
M src/kudu/cfile/bloomfile-test.cc
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/compression-test.cc
M src/kudu/cfile/encoding-test.cc
M src/kudu/cfile/index-test.cc
M src/kudu/cfile/mt-bloomfile-test.cc
M src/kudu/client/client-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log_anchor_registry-test.cc
M src/kudu/consensus/mt-log-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/fs_manager-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/create-table-stress-test.cc
M src/kudu/integration-tests/external_mini_cluster-test.cc
M src/kudu/integration-tests/linked_list-test.cc
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/integration-tests/master_replication-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/master-test-util.h
M src/kudu/master/master-test.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/rpc/mt-rpc-test.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpc_stub-test.cc
M src/kudu/rpc/sasl_rpc-test.cc
M src/kudu/server/hybrid_clock-test.cc
M src/kudu/server/logical_clock-test.cc
M src/kudu/server/webserver-test.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction_policy-test.cc
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/delta_compaction-test.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mt-diskrowset-test.cc
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/rowset_tree-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-schema-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tserver/remote_bootstrap_service-test.cc
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/twitter-demo/parser-test.cc
M src/kudu/util/env-test.cc
M src/kudu/util/failure_detector-test.cc
M src/kudu/util/memenv/memenv-test.cc
M src/kudu/util/metrics-test.cc
M src/kudu/util/mt-metrics-test.cc
M src/kudu/util/net/dns_resolver-test.cc
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/os-util-test.cc
M src/kudu/util/pb_util-test.cc
M src/kudu/util/pstack_watcher-test.cc
M src/kudu/util/resettable_heartbeater-test.cc
M src/kudu/util/subprocess-test.cc
M src/kudu/util/sync_point-test.cc
M src/kudu/util/test_macros.h
M src/kudu/util/thread-test.cc
M src/kudu/util/threadpool-test.cc
M src/kudu/util/user-test.cc
86 files changed, 1,236 insertions(+), 1,242 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/9475/1
--
To view, visit http://gerrit.cloudera.org:8080/9475
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4327f87fb98b186a8f83cf211c38fbddd08c3b05
Gerrit-Change-Number: 9475
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] build: only use -fPIC when absolutely necessary

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9476

to review the following change.


Change subject: build: only use -fPIC when absolutely necessary
..

build: only use -fPIC when absolutely necessary

In general, position independent code is only needed when building shared
objects, but we also need it when KUDU_EXPORTED_CLIENT=1, because that's a
mixed static/shared build.

We'll continue to build thirdparty dependencies with -fPIC unconditionally,
however, because maintaining two trees (one with -fPIC, one without) would
be expensive and annoying.

Change-Id: I9e5c5b2d6e973a43e042709158e2d42f75c3739c
---
M CMakeLists.txt
1 file changed, 5 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/9476/1
--
To view, visit http://gerrit.cloudera.org:8080/9476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9e5c5b2d6e973a43e042709158e2d42f75c3739c
Gerrit-Change-Number: 9476
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] remove old Cyrus-SASL references from thirdparty

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9474

to review the following change.


Change subject: remove old Cyrus-SASL references from thirdparty
..

remove old Cyrus-SASL references from thirdparty

This has been a host-based library for a while now.

Change-Id: I6b36dc09197945688ed779897760cd212195aa31
---
M thirdparty/.gitignore
M thirdparty/build-thirdparty.sh
M thirdparty/vars.sh
3 files changed, 1 insertion(+), 9 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/9474/1
--
To view, visit http://gerrit.cloudera.org:8080/9474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6b36dc09197945688ed779897760cd212195aa31
Gerrit-Change-Number: 9474
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] remove libcurl from Kudu dependencies

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9473

to review the following change.


Change subject: remove libcurl from Kudu dependencies
..

remove libcurl from Kudu dependencies

It was needed for twitter-demo before thirdparty included curl, but is no
longer necessary.

Change-Id: I0e2a0d10e2ec75bbc365d20ba4a80a6abb2726e0
---
M README.adoc
M docs/quickstart.adoc
M src/kudu/twitter-demo/CMakeLists.txt
3 files changed, 3 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/9473/1
--
To view, visit http://gerrit.cloudera.org:8080/9473
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0e2a0d10e2ec75bbc365d20ba4a80a6abb2726e0
Gerrit-Change-Number: 9473
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] client: bump default timeouts

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9477

to review the following change.


Change subject: client: bump default timeouts
..

client: bump default timeouts

Commit 2c8aa9e introduced some new test flakiness, probably due to the
default RPC timeout being so low (2s). Let's bump it to 5s, and double
the default admin operation timeout to provide enough ceiling.

Change-Id: Ie2be96f0095d2da76d943eab383b2d53d72ebe5d
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client.h
M src/kudu/client/client_builder-internal.cc
M src/kudu/client/table-internal.cc
4 files changed, 10 insertions(+), 10 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/9477/1
--
To view, visit http://gerrit.cloudera.org:8080/9477
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie2be96f0095d2da76d943eab383b2d53d72ebe5d
Gerrit-Change-Number: 9477
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] test result server: better handling for non-ASCII in logs

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9478

to review the following change.


Change subject: test_result_server: better handling for non-ASCII in logs
..

test_result_server: better handling for non-ASCII in logs

I was trying to diagnose a test failure whose log contained gdb stack
traces, and got this exception:

Traceback (most recent call last):
  File 
"/home/todd/flaky-test-server-env/lib/python2.6/site-packages/cherrypy/_cprequest.py",
 line 670, in respond
response.body = self.handler()
  File 
"/home/todd/flaky-test-server-env/lib/python2.6/site-packages/cherrypy/lib/encoding.py",
 line 217, in __call__
self.body = self.oldhandler(*args, **kwargs)
  File 
"/home/todd/flaky-test-server-env/lib/python2.6/site-packages/cherrypy/_cpdispatch.py",
 line 61, in __call__
return self.callable(*self.args, **self.kwargs)
  File "./test_result_server.py", line 174, in diagnose
return self.render_container(template.render(summary=summary, 
log_text=log_text))
  File "/home/todd/flaky-test-server-env/li
b/python2.6/site-packages/jinja2/environment.py", line 969, in render
return self.environment.handle_exception(exc_info, True)
  File 
"/home/todd/flaky-test-server-env/lib/python2.6/site-packages/jinja2/environment.py",
 line 742, in handle_exception
reraise(exc_type, exc_value, tb)
  File "", line 5, in top-level template code
UnicodeDecodeError: 'ascii' codec can't decode byte 0xd0 in position 3871577: 
ordinal not in range(128)

The docs for jinja2 say that it expects to receive unicode types, and I
think we're passing in strings. This should fix that.

Change-Id: Iac6da45ffe6c473dc985e60a4471ba708bae4610
---
M build-support/test_result_server.py
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/9478/1
--
To view, visit http://gerrit.cloudera.org:8080/9478
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iac6da45ffe6c473dc985e60a4471ba708bae4610
Gerrit-Change-Number: 9478
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] debug-util-test: add status message when stacks fail

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9471


Change subject: debug-util-test: add status message when stacks fail
..

debug-util-test: add status message when stacks fail

The TestSnapshot test case is flaky, and it seems to be because one of the
threads sometimes fails to take a stack trace. This patch just adds the Status
to the log so that we can diagnose the flake next time it happens.

Change-Id: I4f1da4658c2fbb8e4a049edc392b15f7bf14c8fa
---
M src/kudu/util/debug-util-test.cc
1 file changed, 2 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/9471/1
--
To view, visit http://gerrit.cloudera.org:8080/9471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4f1da4658c2fbb8e4a049edc392b15f7bf14c8fa
Gerrit-Change-Number: 9471
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 

[kudu-CR] build: use thin static libraries on Linux

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9472


Change subject: build: use thin static libraries on Linux
..

build: use thin static libraries on Linux

Thin static libraries just contain pointers to .o files rather than copying all
of the data. This reduces the amount of IO during the build as well as the 
required
disk space.

For example, comparing a clean build ('ninja clean kudu') before and after this
change, we can see that the build is about 40% faster and about 200x
less disk space in the lib/ directory.

Thick (original):
  real0m24.784s
  user0m58.643s
  sys 0m37.701s

  todd@ve0518:/data/1/todd/kudu$ du -sm build/release/lib/
  594 build/release/lib/

Thin:
  real0m17.422s
  user0m56.092s
  sys 0m34.131s
  todd@ve0518:/data/1/todd/kudu$ du -sm build/release/lib/
  3   build/release/lib/

Change-Id: If662cea380e06eaddf45e52617e38e55e4613773
---
M CMakeLists.txt
1 file changed, 10 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/72/9472/1
--
To view, visit http://gerrit.cloudera.org:8080/9472
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If662cea380e06eaddf45e52617e38e55e4613773
Gerrit-Change-Number: 9472
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 

[kudu-CR] docs: update docs for update dirs tool

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9110 )

Change subject: docs: update docs for update_dirs tool
..


Patch Set 3: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/9110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c139326aee35f72495a1cb1aaf8df1d58776cb
Gerrit-Change-Number: 9110
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:07:55 +
Gerrit-HasComments: No

[kudu-CR] build: enable sharding within cmake/ctest

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9470 )

Change subject: build: enable sharding within cmake/ctest
..


Patch Set 2:

(5 comments)

Makes sense, I'm much happier to see the sharding definitions colocated with  
test definitions.

http://gerrit.cloudera.org:8080/#/c/9470/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9470/2//COMMIT_MSG@26
PS2, Line 26: * Flaky-test tracking is currently still done by the test binary 
name
:   and not the specific shard, though we could easily switch that 
in
:   the future.
This gets weird though because the number of shards may change over time, which 
would muck up the sharded test's history.


http://gerrit.cloudera.org:8080/#/c/9470/2/CMakeLists.txt
File CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/9470/2/CMakeLists.txt@678
PS2, Line 678: If a test suite is long enough
 : #   to require a bumped timeout, consider enabling sharding 
of the
 : #   test by adding it to the NUM_SHARDS_BY_TEST dictionary 
in dist_test.py.
Update.


http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py
File build-support/dist_test.py:

http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@56
PS2, Line 56: TEST_COMMAND_RE = re.compile('Test command: (.+)$')
: TEST_ENV_RE = re.compile('^\d+:  (\S+)=(.+)')
: LDD_RE = re.compile(r'^\s+.+? => (\S+) \(0x.+\)')
Would be nice to include an example string for each of these.


http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@322
PS2, Line 322:   name=execution.test_name,
Note that prior to your change the name of the test also included the total 
number of shards. Now it doesn't anymore. Does that matter, or was it purely 
cosmetic?


http://gerrit.cloudera.org:8080/#/c/9470/2/build-support/dist_test.py@442
PS2, Line 442:   execution = dict(argv=(["run-test.sh", options.cmd] + 
options.args), env={})
This isn't right, I think it should be:

  execution = TestExecution([...] + options.args)

Something like that? Actually, this should still call get_test_executions() in 
some capacity, right? Otherwise looping a sharded test will ignore the sharding.



--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:06:56 +
Gerrit-HasComments: Yes

[kudu-CR] docs: add warning to wait between FS rebuilds

[Alex Rodoni (Code Review)]

Alex Rodoni has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9279 )

Change subject: docs: add warning to wait between FS rebuilds
..


Patch Set 4: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/9279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5659c2ba05a0a6b9905213c5df6ba0dcb371f312
Gerrit-Change-Number: 9279
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:05:19 +
Gerrit-HasComments: No

[kudu-CR] python: fix build on Python 2.6

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9468 )

Change subject: python: fix build on Python 2.6
..

python: fix build on Python 2.6

We appear to have been bitten by a bug [1] in setuptools_scm. Here's the
recommended workaround.

For the life of me, I couldn't isolate this to any particular package
version change. I tried "last known good" versions of pip, setuptools,
setuptools_scm, pytest, and pytest-runner. In every case the exception
persisted when building on CentOS 6.6 with Python 2.6.6, but not on Ubuntu
16.04 with Python 2.7.12. My best guess is that it's due to the Python
version, but I don't know that for sure.

1. https://github.com/pypa/setuptools_scm/issues/209

Change-Id: I0cf7f64363b308e06ac9bcdebcfc3832ff7b65b9
Reviewed-on: http://gerrit.cloudera.org:8080/9468
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M python/requirements.txt
1 file changed, 8 insertions(+), 0 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Todd Lipcon: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/9468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0cf7f64363b308e06ac9bcdebcfc3832ff7b65b9
Gerrit-Change-Number: 9468
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] build: enable sharding within cmake/ctest

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9470

to look at the new patch set (#2).

Change subject: build: enable sharding within cmake/ctest
..

build: enable sharding within cmake/ctest

This changes test sharding to be set up via cmake rather than specialized code
in the dist-test wrapper:

* There is a new argument for the ADD_KUDU_TEST() CMake function. When
  set, we generate separate CTest executions for each shard of the
  test. Thus, 'ctest -j' can now parallelize the multiple shards of
  longer-running tests.

* The specialized sharding logic is now gone from dist_test.py. Instead,
  it is now able to parse more of the output from 'ctest -N' and grabs
  the sharding-related environment variables directly from there and
  passes them down into the test environment.

* The setting of the GTEST_OUTPUT environment variable is moved from
  build-and-test.sh into run_test.sh since it's necessary to ensure
  that the different shards output to separate XML files.

* Flaky-test tracking is currently still done by the test binary name
  and not the specific shard, though we could easily switch that in
  the future.

Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/jenkins/build-and-test.sh
M build-support/run-test.sh
M build-support/run_dist_test.py
M src/kudu/cfile/CMakeLists.txt
M src/kudu/client/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tools/CMakeLists.txt
11 files changed, 163 insertions(+), 125 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/9470/2
--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

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

[Hao Hao (Code Review)]

Hello Tidy Bot, Alex Rodoni, Mike Percy, Dan Burkert, David Ribeiro Alves, Kudu 
Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8823

to look at the new patch set (#18).

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..

KUDU-1704: add c++ client support for READ_YOUR_WRITES mode

Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/scan_configuration.cc
M src/kudu/client/scan_configuration.h
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/delete_table-itest.cc
10 files changed, 403 insertions(+), 94 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/23/8823/18
--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 18
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot

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

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 17:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8823/17/src/kudu/client/scan_configuration.cc
File src/kudu/client/scan_configuration.cc:

http://gerrit.cloudera.org:8080/#/c/8823/17/src/kudu/client/scan_configuration.cc@45
PS17, Line 45: ScanConfiguration::ScanConfiguration(KuduTable* table)
> lower_bound_propagation_timestamp_ needs to be initialized, I'm surprised t
Ah, thanks a lot for catching this!


http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc@186
PS10, Line 186:   }
> This sounds reasonable to me, and I second distilling this  discussion into
Cool, makes sense to me too. Will add it to a short comment.



--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 17
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 23:41:44 +
Gerrit-HasComments: Yes

[kudu-CR] python: fix build on Python 2.6

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9468 )

Change subject: python: fix build on Python 2.6
..


Patch Set 1: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/9468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cf7f64363b308e06ac9bcdebcfc3832ff7b65b9
Gerrit-Change-Number: 9468
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 02 Mar 2018 23:29:21 +
Gerrit-HasComments: No

[kudu-CR] build: enable sharding within cmake/ctest

[Todd Lipcon (Code Review)]

Hello Adar Dembo,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9470

to review the following change.


Change subject: build: enable sharding within cmake/ctest
..

build: enable sharding within cmake/ctest

This changes test sharding to be set up via cmake rather than specialized code
in the dist-test wrapper:

* There is a new argument for the ADD_KUDU_TEST() CMake function. When
  set, we generate separate CTest executions for each shard of the
  test. Thus, 'ctest -j' can now parallelize the multiple shards of
  longer-running tests.

* The specialized sharding logic is now gone from dist_test.py. Instead,
  it is now able to parse more of the output from 'ctest -N' and grabs
  the sharding-related environment variables directly from there and
  passes them down into the test environment.

* The setting of the GTEST_OUTPUT environment variable is moved from
  build-and-test.sh into run_test.sh since it's necessary to ensure
  that the different shards output to separate XML files.

* Flaky-test tracking is currently still done by the test binary name
  and not the specific shard, though we could easily switch that in
  the future.

Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
---
M CMakeLists.txt
M build-support/dist_test.py
M build-support/jenkins/build-and-test.sh
M build-support/run-test.sh
M build-support/run_dist_test.py
M src/kudu/cfile/CMakeLists.txt
M src/kudu/client/CMakeLists.txt
M src/kudu/hms/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tools/CMakeLists.txt
11 files changed, 162 insertions(+), 112 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/9470/1
--
To view, visit http://gerrit.cloudera.org:8080/9470
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I20ddbdd73a64fda3fe32fca98ee541aa4cead4b3
Gerrit-Change-Number: 9470
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 

[kudu-CR] [java] fix the regression test for KUDU-2267/KUDU-2319

[Hao Hao (Code Review)]

Hao Hao has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9388 )

Change subject: [java] fix the regression test for KUDU-2267/KUDU-2319
..

[java] fix the regression test for KUDU-2267/KUDU-2319

In the previous commit 0f0e42144, a regression test was added to ensure
a client with valid authn token but without valid Kerberos credentials
should be able to connect to all the masters. However, the test was not
correct. This patch fixes that.

Change-Id: I5e827586fe549f6a0c927ce8a4f8eca954bfe690
Reviewed-on: http://gerrit.cloudera.org:8080/9388
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
2 files changed, 22 insertions(+), 8 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/9388
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5e827586fe549f6a0c927ce8a4f8eca954bfe690
Gerrit-Change-Number: 9388
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

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

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc@186
PS10, Line 186: 
client->data_->UpdateLatestObservedTimestamp(message.propagated_timestamp());
> RYW with scan tokens is a bit of a fuzzy concept, since RYW is by definitio
This sounds reasonable to me, and I second distilling this  discussion into a 
short comment in the code.



--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 23:05:16 +
Gerrit-HasComments: Yes

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

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc@186
PS10, Line 186: 
client->data_->UpdateLatestObservedTimestamp(message.propagated_timestamp());
> Right, so KuduScanner::Open is called after this, and at that point it grab
RYW with scan tokens is a bit of a fuzzy concept, since RYW is by definition 
local and scan tokens are by definition multi client.

If I understand the suggestion, the point is that to get RYW in the point of 
view of the scan token emitter, we could safely ignore the scan token's 
receiver client's internal last propagated timestamp, which might be more 
recent.

I think things get murky here a bit, since RYW with scan tokens are murky. I 
don't think it would necessarily be wrong to do that, but I think it makes 
reasoning about timestamps and how they get propagated a bit harder if we have 
two ways to choose the lower bound for the RYW scan. Moreover, in practice, I 
suspect it's very rarely the case that an active client is permanently being 
written to and read from (using scan tokens).

My suggestion is to keep the path simple and the same as for the local scans. 
Maybe note somewhere that is a possible optimization if we ever notice scan 
token RYW reads stalling.



--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 23:01:02 +
Gerrit-HasComments: Yes

[kudu-CR] python: fix build on Python 2.6

[Adar Dembo (Code Review)]

Hello Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/9468

to review the following change.


Change subject: python: fix build on Python 2.6
..

python: fix build on Python 2.6

We appear to have been bitten by a bug [1] in setuptools_scm. Here's the
recommended workaround.

For the life of me, I couldn't isolate this to any particular package
version change. I tried "last known good" versions of pip, setuptools,
setuptools_scm, pytest, and pytest-runner. In every case the exception
persisted when building on CentOS 6.6 with Python 2.6.6, but not on Ubuntu
16.04 with Python 2.7.12. My best guess is that it's due to the Python
version, but I don't know that for sure.

1. https://github.com/pypa/setuptools_scm/issues/209

Change-Id: I0cf7f64363b308e06ac9bcdebcfc3832ff7b65b9
---
M python/requirements.txt
1 file changed, 8 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/9468/1
--
To view, visit http://gerrit.cloudera.org:8080/9468
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0cf7f64363b308e06ac9bcdebcfc3832ff7b65b9
Gerrit-Change-Number: 9468
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon 

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

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 17:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8823/17/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/17/src/kudu/client/scanner-internal.cc@495
PS17, Line 495: CHECK(last_response_.has_snap_timestamp());
> I think it would be simpler to only have the single branch like before, but
Disregard; I didn't see the difference between propagated_timestamp and 
snapshot_timestamp



--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 17
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 20:56:36 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2319 follower masters should be able to verify authn tokens

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9373 )

Change subject: KUDU-2319 follower masters should be able to verify authn tokens
..


Patch Set 12: Verified+1

Unrelated flake in DeleteTableITest.TestNoDeleteTombstonedTablets


--
To view, visit http://gerrit.cloudera.org:8080/9373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Gerrit-Change-Number: 9373
Gerrit-PatchSet: 12
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 20:22:32 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2319 follower masters should be able to verify authn tokens

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9373 )

Change subject: KUDU-2319 follower masters should be able to verify authn tokens
..

KUDU-2319 follower masters should be able to verify authn tokens

This patch addresses the issue that follower masters could not accept
authn tokens to authenticate clients because they didn't have keys for
token signature verification.

Added a couple of tests:
  * SecurityMasterAuthTest::FollowerTokenVerificationKeys
  * ConnectToFollowerMasterTest::AuthnTokenVerifierHaveKeys

The former is more a unit test; the latter is an integration test.

Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Reviewed-on: http://gerrit.cloudera.org:8080/9373
Reviewed-by: Dan Burkert 
Tested-by: Alexey Serbin 
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/security-itest.cc
R src/kudu/integration-tests/security-master-auth-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
5 files changed, 233 insertions(+), 24 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Alexey Serbin: Verified

--
To view, visit http://gerrit.cloudera.org:8080/9373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Gerrit-Change-Number: 9373
Gerrit-PatchSet: 13
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-2319 follower masters should be able to verify authn tokens

[Alexey Serbin (Code Review)]

Alexey Serbin has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/9373 )

Change subject: KUDU-2319 follower masters should be able to verify authn tokens
..


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Gerrit-Change-Number: 9373
Gerrit-PatchSet: 12
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-2319 follower masters should be able to verify authn tokens

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9373 )

Change subject: KUDU-2319 follower masters should be able to verify authn tokens
..


Patch Set 12: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc@1034
PS11, Line 1034:   return s;
   :   }
> I updated this code to be easier to follow.
looks great


http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc@1042
PS11, Line 1042:
   :
   :   LOG_WITH_PREFIX(INFO) << kDescription
   : <<
> The return status from this methods affects whether the PrepareFollower() m
oh I see, thanks for the explanation.



--
To view, visit http://gerrit.cloudera.org:8080/9373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Gerrit-Change-Number: 9373
Gerrit-PatchSet: 12
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 19:54:50 +
Gerrit-HasComments: Yes

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

[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 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. 
Maybe just change this to:

The workflow is unsafe for adding new masters to an existing configuration that 
already has three or more masters. Do not use it for that purpose.

And then separately explain 2->3?


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 
"Kudu data blocks". But I understand that not everyone may read it that way, so 
your clarification makes sense.


http://gerrit.cloudera.org:8080/#/c/9466/1/docs/administration.adoc@236
PS1, Line 236: it
Nit: they


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 the 
'kudu' user, so why note it on individual steps?

Alternatively, we could add "sudo -u kudu" to the beginning of every shell line 
if you think that would help.


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 recommendations.



--
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-Comment-Date: Fri, 02 Mar 2018 19:48:39 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2319 follower masters should be able to verify authn tokens

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9373 )

Change subject: KUDU-2319 follower masters should be able to verify authn tokens
..


Patch Set 11:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc@1034
PS11, Line 1034: } else {
   : if (!key
> this can be un-nested one level with an 'else if', which I think will make
I updated this code to be easier to follow.


http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc@1042
PS11, Line 1042: ) in this case would lead to
   :   // waiting till next cycle of TSK rotation. Returning 
non-OK status will
   :   // make the periodic task of 
CatalogManagerBgTasks::Run() to retry
   :   // fetching the keys.
> I'm not seeing how the return status from this method is effecting CatalogM
The return status from this methods affects whether the PrepareFollower() 
method updates its output parameter 'last_tspk_run'.  Yes, the 
CatalogManagerBgTasks::Run() runs with its own interval, but 
PrepareFollowerTokenVerifier() will not be called until enough time has passed 
to expect a new TSK to appear.



--
To view, visit http://gerrit.cloudera.org:8080/9373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Gerrit-Change-Number: 9373
Gerrit-PatchSet: 11
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 19:39:27 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2319 follower masters should be able to verify authn tokens

[Alexey Serbin (Code Review)]

Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Hao Hao,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9373

to look at the new patch set (#12).

Change subject: KUDU-2319 follower masters should be able to verify authn tokens
..

KUDU-2319 follower masters should be able to verify authn tokens

This patch addresses the issue that follower masters could not accept
authn tokens to authenticate clients because they didn't have keys for
token signature verification.

Added a couple of tests:
  * SecurityMasterAuthTest::FollowerTokenVerificationKeys
  * ConnectToFollowerMasterTest::AuthnTokenVerifierHaveKeys

The former is more a unit test; the latter is an integration test.

Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/security-itest.cc
R src/kudu/integration-tests/security-master-auth-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
5 files changed, 233 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/9373/12
--
To view, visit http://gerrit.cloudera.org:8080/9373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Gerrit-Change-Number: 9373
Gerrit-PatchSet: 12
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

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

[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 1:

(2 comments)

Code looks fine but what's the motivation for this change?

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?


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.



--
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: Fri, 02 Mar 2018 19:38:17 +
Gerrit-HasComments: Yes

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

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8823 )

Change subject: KUDU-1704: add c++ client support for READ_YOUR_WRITES mode
..


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8823/17/src/kudu/client/scan_configuration.cc
File src/kudu/client/scan_configuration.cc:

http://gerrit.cloudera.org:8080/#/c/8823/17/src/kudu/client/scan_configuration.cc@45
PS17, Line 45: ScanConfiguration::ScanConfiguration(KuduTable* table)
lower_bound_propagation_timestamp_ needs to be initialized, I'm surprised this 
didn't cause issues?


http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc@186
PS10, Line 186: 
client->data_->UpdateLatestObservedTimestamp(message.propagated_timestamp());
> Hmm, currently SetScanLowerBoundTimestampRaw() is called when we open a kud
Right, so KuduScanner::Open is called after this, and at that point it grabs 
the client's LatestObservedTimestamp and sets it as the scan config's 
propagation timestamp.

That's correct, however I think it's stricter than necessary: we could get RYW 
by setting the scan config's propagation timestamp to the ScanToken's 
propagated timestamp, which may be older than the client's latest observed 
timestamp (e.g. if the client has other writes/scans happening concurrently).  
It may not matter in practice, though?


http://gerrit.cloudera.org:8080/#/c/8823/10/src/kudu/client/scan_token-internal.cc@263
PS10, Line 263:   
pb.set_propagated_timestamp(client->GetLatestObservedTimestamp());
> Hmm,  you mean use the previous scanner's propagated timestamp? I do not th
yeah sorry I don't think this question made any sense.


http://gerrit.cloudera.org:8080/#/c/8823/17/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:

http://gerrit.cloudera.org:8080/#/c/8823/17/src/kudu/client/scanner-internal.cc@495
PS17, Line 495: }
I think it would be simpler to only have the single branch like before, but 
check that in RYW the field is set:

  CHECK(configuration_.read_mode() != KuduScanner::READ_YOUR_WRITES ||
last_response_.has_snap_timestamp());
  if (last_response_.has_propagated_timestamp()) {
table_->client()->data_->UpdateLatestObservedTimestamp(
last_response_.propagated_timestamp());
  }



--
To view, visit http://gerrit.cloudera.org:8080/8823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34214245a78aed172a28fbdb395ff5bccd0fc0e1
Gerrit-Change-Number: 8823
Gerrit-PatchSet: 10
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alex Rodoni 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 19:23:12 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2259: add real user to AuthenticationCredentialsPB

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9374 )

Change subject: KUDU-2259: add real user to AuthenticationCredentialsPB
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java:

http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@156
PS2, Line 156: authentica
> the hasAuthnToke() check takes care of that.
Not sure I understand: the condition for this 'if' closure is 
'pb.hasAuthnToken()', which comes from input bytes 'authnData'.  Meanwhile, 
'authnToken' is a member of the class which might be null at this point, no?


http://gerrit.cloudera.org:8080/#/c/9374/1/src/kudu/client/client.proto
File src/kudu/client/client.proto:

http://gerrit.cloudera.org:8080/#/c/9374/1/src/kudu/client/client.proto@116
PS1, Line 116: root
> Is that in the case of external PKI?  With internal-PKI I don't think we ev
Yes, the idea was to use the sequence for the certificate chain, if we had 
that.  The chain is needed for validation.

As of now, there is no multiple root CA certificates.



--
To view, visit http://gerrit.cloudera.org:8080/9374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d2d901d42501ecfc0f6372f68cf7335eb188b45
Gerrit-Change-Number: 9374
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 02 Mar 2018 19:05:46 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2259: add real user to AuthenticationCredentialsPB

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9374 )

Change subject: KUDU-2259: add real user to AuthenticationCredentialsPB
..


Patch Set 2:

(4 comments)

I'm still unsure about some of the changes in this PR from a 
design-perspective, and I think there's a non-zero chance that it may have 
unintended side-effects, so I think we should hold off on getting it in for the 
immediately upcoming release.

My design hesitations are around the fact that it potentially duplicates the 
real-user field in AuthenticationCredentialsPB when the token is set: the new 
'realUser' field and the signed token could have different users.  Should we 
only set one or the other?  I'm also somewhat struggling to come up with a 
coherent high-level explanation for this change.

http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java:

http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@61
PS2, Line 61:   private String realUser;
> probably worth a comment.
I think this is a typo, it should indeed never be null.


http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@117
PS2, Line 117: public synchronized byte[] exportAuthenticationCredentials() {
> It seems the previous version returned null if either authn token or certs
Yes, that's true.  Even though the implementation has changed to never return 
null, I didn't want to change the actual interface, since changing the 
guarantee later would be more difficult.


http://gerrit.cloudera.org:8080/#/c/9374/2/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java@156
PS2, Line 156: authnToken
> What if authnToken was null?
the hasAuthnToke() check takes care of that.


http://gerrit.cloudera.org:8080/#/c/9374/1/src/kudu/client/client.proto
File src/kudu/client/client.proto:

http://gerrit.cloudera.org:8080/#/c/9374/1/src/kudu/client/client.proto@116
PS1, Line 116: root
> nit: IIRC, some time ago there was an idea to put the whole certificate cha
Is that in the case of external PKI?  With internal-PKI I don't think we ever 
really have a 'chain', or at least there are no intermediates.



--
To view, visit http://gerrit.cloudera.org:8080/9374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d2d901d42501ecfc0f6372f68cf7335eb188b45
Gerrit-Change-Number: 9374
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 02 Mar 2018 18:43:08 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2312: Scan predicate application ordering is non-deterministic

[Dan Burkert (Code Review)]

Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/9440

to look at the new patch set (#4).

Change subject: KUDU-2312: Scan predicate application ordering is 
non-deterministic
..

KUDU-2312: Scan predicate application ordering is non-deterministic

This changes the scan predicate evaluation ordering so that it primarily
orders by selectivity (as before), but breaks ties by column index.

Change-Id: I99b2cabecd8626cad7e11fbdd492af7276e08348
---
M docs/release_notes.adoc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/generic_iterators.h
M src/kudu/tablet/cfile_set.h
5 files changed, 117 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/9440/4
--
To view, visit http://gerrit.cloudera.org:8080/9440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99b2cabecd8626cad7e11fbdd492af7276e08348
Gerrit-Change-Number: 9440
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2312: Scan predicate application ordering is non-deterministic

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9440 )

Change subject: KUDU-2312: Scan predicate application ordering is 
non-deterministic
..


Patch Set 3:

(2 comments)

Release note added.

http://gerrit.cloudera.org:8080/#/c/9440/1/src/kudu/common/generic_iterators-test.cc
File src/kudu/common/generic_iterators-test.cc:

http://gerrit.cloudera.org:8080/#/c/9440/1/src/kudu/common/generic_iterators-test.cc@402
PS1, Line 402: ASSER
> ASSERT_EQ does but not ASSERT_TRUE
ah sorry I misunderstood.


http://gerrit.cloudera.org:8080/#/c/9440/3/src/kudu/common/generic_iterators.cc
File src/kudu/common/generic_iterators.cc:

http://gerrit.cloudera.org:8080/#/c/9440/3/src/kudu/common/generic_iterators.cc@46
PS3, Line 46: using std::all_of;
> warning: using decl 'all_of' is unused [misc-unused-using-decls]
Done



--
To view, visit http://gerrit.cloudera.org:8080/9440
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I99b2cabecd8626cad7e11fbdd492af7276e08348
Gerrit-Change-Number: 9440
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 02 Mar 2018 18:06:38 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2319 follower masters should be able to verify authn tokens

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9373 )

Change subject: KUDU-2319 follower masters should be able to verify authn tokens
..


Patch Set 11:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc@1034
PS11, Line 1034: } else {
   : if (!key
this can be un-nested one level with an 'else if', which I think will make it 
easier to follow as well.


http://gerrit.cloudera.org:8080/#/c/9373/11/src/kudu/master/catalog_manager.cc@1042
PS11, Line 1042: ) in this case would lead to
   :   // waiting till next cycle of TSK rotation. Returning 
non-OK status will
   :   // make the periodic task of 
CatalogManagerBgTasks::Run() to retry
   :   // fetching the keys.
I'm not seeing how the return status from this method is effecting 
CatalogManagerBgTasks::Run() in this way.  From what I can see it's simply 
logging a warning if it's a non-OK success status.  Either way it appears to go 
back through the loop 1 second later (line 569)?



--
To view, visit http://gerrit.cloudera.org:8080/9373
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7
Gerrit-Change-Number: 9373
Gerrit-PatchSet: 11
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 02 Mar 2018 17:52:06 +
Gerrit-HasComments: Yes