[kudu-CR] [security-faults-itest] fix test flakiness

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [security-faults-itest] fix test flakiness
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6586/1/src/kudu/integration-tests/security-faults-itest.cc
File src/kudu/integration-tests/security-faults-itest.cc:

PS1, Line 55: #if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
:   : krb_lifetime_seconds_(64),
: #else
:   : krb_lifetime_seconds_(32),
: #endif
Any particular reason not to use 64 universally?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I132426ee37358a228cf3f5d1fef9e9ec08610b16
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [security-faults-itest] fix test flakiness

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [security-faults-itest] fix test flakiness
..

[security-faults-itest] fix test flakiness

When running the security-faults-itest on a slow VM with other
activities in parallel, it might take up to 8 seconds to start a master.
So, bumping the ticket TTL to higher value so Kudu cluster with 3
masters finishes starting up when initially acquired tickets are valid.

Also, updated Raft parameters interval to speed up leader election.

Change-Id: I132426ee37358a228cf3f5d1fef9e9ec08610b16
---
M src/kudu/integration-tests/security-faults-itest.cc
1 file changed, 12 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I132426ee37358a228cf3f5d1fef9e9ec08610b16
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 

[kudu-CR] log block manager: corruptor test utility

[Adar Dembo (Code Review)]

Hello David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: log block manager: corruptor test utility
..

log block manager: corruptor test utility

This patch introduces the LBMCorruptor, a test-only class for adding various
inconsistencies to the LBM's on-disk structures. The bulk of the patch is a
set of new tests that exercise the corruptor and then verify the results via
the generated FsReport.

Additionally, block_manager-stress-test has been modified to inject
non-fatal inconsistencies via the LBMCorruptor in between passes.

Change-Id: I197b693d5a3c1a909e91b772ebfb31e50bae488b
---
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
A src/kudu/fs/log_block_manager-test-util.cc
A src/kudu/fs/log_block_manager-test-util.h
M src/kudu/fs/log_block_manager-test.cc
5 files changed, 825 insertions(+), 39 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I197b693d5a3c1a909e91b772ebfb31e50bae488b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] fs: ensure FS IOC FIEMAP can be used on LBM systems

[Adar Dembo (Code Review)]

Hello David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: fs: ensure FS_IOC_FIEMAP can be used on LBM systems
..

fs: ensure FS_IOC_FIEMAP can be used on LBM systems

This patch generalizes the existing "hole punch test" to include a check
that FS_IOC_FIEMAP works.

I've tested it with ext4 on el6.6 and Ubuntu 16.04. Notably, it doesn't work
with ecryptfs, though neither does hole punching.

Change-Id: Ie027a264473ae5f905f8ad50aa45e810e046a135
---
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/log_block_manager.cc
3 files changed, 26 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie027a264473ae5f905f8ad50aa45e810e046a135
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] fs: generate report during Open

[Adar Dembo (Code Review)]

Hello David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: fs: generate report during Open
..

fs: generate report during Open

This patch modifies the FsManager and block managers to produce a filesystem
report when opened. The report includes various filesystem statistics as
well as all inconsistencies found and repaired.

The heart of the patch is the FsReport nested data structure. Originally
implemented as a protobuf to take advantage of DebugString(), I found it to
be a poor fit once I began customizing the log output, so I rewrote it in
its current form. The report includes fs-wide statistics as well as optional
consistency checks that may or may not be performed by the block managers.
Reports can be merged; the LBM takes advantage of this as it processes data
directories in parallel. The consistency checks have structure, so as to
simplify testing and ToString() customization.

Thanks to the level of detail in the FsReport, the LBM now separates the act
of finding inconsistencies from repairing them. This makes it much easier to
enforce that repairs aren't done on a read-only filesystem, or if there were
any fatal and irreparable errors.

Thorough testing of the inconsistencies in the report is deferred to a
different patch as this one was already quite large.

Other changes of note:
- The LBM treats data files without matching metadata files as incomplete
  containers; previously only the opposite was true.
- The LBM maps all containers by name so they can be looked up during the
  repair process; previously they were stored in a vector.
- The checks performed by "kudu fs check" are in FsReport. I think this
  makes sense, and it's an acknowledgement that they should be performed by
  the block manager in the future. The code in tool_action_fs.cc was
  restructured to write its findings into the report before logging it.

Change-Id: Idcc1d512e2bed103c8d41623a63b4d071532b35e
---
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/fs_report.cc
A src/kudu/fs/fs_report.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/util/pb_util.cc
17 files changed, 1,144 insertions(+), 208 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idcc1d512e2bed103c8d41623a63b4d071532b35e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] env: add RWFile::GetExtentMap for analyzing file extents

[Adar Dembo (Code Review)]

Hello David Ribeiro Alves, Todd Lipcon,

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

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

to review the following change.

Change subject: env: add RWFile::GetExtentMap for analyzing file extents
..

env: add RWFile::GetExtentMap for analyzing file extents

This patch introduces a method to get the extent metadata of a file provided
it resides on an extent-based filesystem (such as ext4 or xfs). Each extent
is an offset and length into the file, and represents a chunk of filesystem
that has been allocated for the file. Gaps between extents are expected to
be unallocated and may represent punched out holes.

On Linux, the extent listing is retrieved via repeated calls to the
FS_IOC_FIEMAP ioctl, though only some of the information returned is
actually used.

I intend to use this in the log block manager for two purposes:
- Discovering whether a container has allocated space beyond its file size.
- Finding all of a container's "unpunched" holes, and repunching them.

Change-Id: I35bd1bdb9e1a839af2ab95ea73b79217c1f4a2b3
---
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/file_cache.cc
4 files changed, 170 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I35bd1bdb9e1a839af2ab95ea73b79217c1f4a2b3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [consensus] fixed typos in consensus flags tagging

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [consensus] fixed typos in consensus flags tagging
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6580/1/src/kudu/consensus/consensus_peers.cc
File src/kudu/consensus/consensus_peers.cc:

Line 54: TAG_FLAG(raft_get_node_instance_timeout_ms, hidden);
> As I understand, consensus_rpc_timeout_ms is already hidden by correspondin
I meant line 50 makes it consensus_rpc_timeout advanced.  If they wanted to 
hide it, then specifying 'advanced' when it's already marked 'hidden' would be 
too much.

But it's necessary to clarify, sure.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I39fb954313368c8f612c986ab13dd47e8c948998
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [consensus] fixed typos in consensus flags tagging

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [consensus] fixed typos in consensus flags tagging
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6580/1/src/kudu/consensus/consensus_peers.cc
File src/kudu/consensus/consensus_peers.cc:

Line 54: TAG_FLAG(raft_get_node_instance_timeout_ms, hidden);
> You sure the intent was to make this flag hidden and not consensus_rpc_time
As I understand, consensus_rpc_timeout_ms is already hidden by corresponding 
TAG_FLAG() macro at line 50.  That's why I think it's a typo.

But it would be nice if David or Todd could confirm that, sure.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I39fb954313368c8f612c986ab13dd47e8c948998
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [consensus] fixed typos in consensus flags tagging

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [consensus] fixed typos in consensus flags tagging
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6580/1/src/kudu/consensus/consensus_peers.cc
File src/kudu/consensus/consensus_peers.cc:

Line 54: TAG_FLAG(raft_get_node_instance_timeout_ms, hidden);
You sure the intent was to make this flag hidden and not 
consensus_rpc_timeout_ms?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I39fb954313368c8f612c986ab13dd47e8c948998
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [consensus] fixed typos in consensus flags tagging

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [consensus] fixed typos in consensus flags tagging
..

[consensus] fixed typos in consensus flags tagging

Change-Id: I39fb954313368c8f612c986ab13dd47e8c948998
---
M src/kudu/consensus/consensus_peers.cc
1 file changed, 6 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I39fb954313368c8f612c986ab13dd47e8c948998
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 

[kudu-CR] KUDU-579 [java client] Scanner fault tolerance

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change.

Change subject: KUDU-579 [java_client] Scanner fault tolerance
..


Patch Set 3:

(17 comments)

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

Line 78:* Ensures the scanner to be fault tolerant, and returns scan 
results in primary
> This javadoc needs to be re-written.
Done


Line 89: return (S) this;
> Remove all the Interface stuff, we don't want this to be private anymore.
Done


Line 91: 
> Make it public.
Done


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

Line 505: // is fault tolerant.
> Shouldn't you only do this for fault tolerant scanners? Also it seems there
Done


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

Line 113:   /**
> We put this above the method signature, below the javadoc.
Done


PS2, Line 115: 
> RemoteTablet?
Done


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

Line 179: // if uuid is empty, it means the leader has been removed from
> I'm not following this. If the leader isn't known then we need to go ask th
I also do not have a good feeling about this. However, without this change, 
ITScannerMultiTablet.testFaultTolerantScannerKillTS would keep on retry RPC 
until timeout and eventually fail. It seems it can never get the correct leader 
information. Any suggestions on this? :)


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

Line 2: // or more contributor license agreements.  See the NOTICE file
> Use a difference license, like KuduSession has, since this file AFAIK isn't
Done


PS2, Line 38: 
> This should be a RecoverableException.
Done


PS2, Line 42: m
> Remove trailing periods from all @param lines
Done


PS2, Line 51: 
> Always start with a lower case.
Done


http://gerrit.cloudera.org:8080/#/c/6566/2/java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
File java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java:

PS2, Line 377: l
> nit
Done


Line 393: }
> Part of this is copied from killTabletLeader(KuduTable), can you do the req
Done


PS2, Line 477: 
> nit
Done


Line 483:   }
> Another refactoring opportunity.
Done


http://gerrit.cloudera.org:8080/#/c/6566/2/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:

Line 53: builder.addHashPartitions(
> nit: now the line is too long.
Done


Line 152:   killTabletLeader(scanner.currentTablet());
> This should be refactor into one helper method and two tests that simple ca
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-579 [java client] Scanner fault tolerance

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-579 [java_client] Scanner fault tolerance
..

KUDU-579 [java_client] Scanner fault tolerance

This patch adds java client support to restart scanners if they
fail in the middle of table scanning. AsyncKuduScanner records
the final primary key retrieved in the previous scan. If a tserver
fails while scanning, the client marks the tserver as failed and
retries the scan elsewhere, providing its last primary key.

Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b
---
M 
java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
A 
java/kudu-client/src/main/java/org/apache/kudu/client/ScannerExpiredException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
8 files changed, 224 insertions(+), 52 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] linked list-test: dump a histogram of Update performance

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: linked_list-test: dump a histogram of Update performance
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6576/1/src/kudu/integration-tests/linked_list-test-util.h
File src/kudu/integration-tests/linked_list-test-util.h:

PS1, Line 312: 100, 3
magic numbers? at least doc


PS1, Line 547: void LinkedListTester::DumpHistogram(const HdrHistogram& h) {
 :   using std::cout;
 :   using std::endl;
 :   cout << "Count: " << h.TotalCount() << endl;
 :   cout << "Mean: " << h.MeanValue() << endl;
 :   cout << "Percentiles:" << endl;
 :   cout << "   0%  (min) = " << h.MinValue() << endl;
 :   cout << "  25%= " << h.ValueAtPercentile(25) << endl;
 :   cout << "  50%  (med) = " << h.ValueAtPercentile(50) << endl;
 :   cout << "  75%= " << h.ValueAtPercentile(75) << endl;
 :   cout << "  95%= " << h.ValueAtPercentile(95) << endl;
 :   cout << "  99%= " << h.ValueAtPercentile(99) << endl;
 :   cout << "  99.9%  = " << h.ValueAtPercentile(99.9) << endl;
 :   cout << "  99.99% = " << h.ValueAtPercentile(99.99) << 
endl;
 :   cout << "  100% (max) = " << h.MaxValue() << endl;
 :   if (h.MaxValue() >= h.highest_trackable_value()) {
 : cout << "*NOTE: some values were greater than highest 
trackable value" << endl;
 :   }
 : }
maybe dumping the histo to string should be on the histogram class and then 
you'd dump it to stdout here?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82d495d55091ee597995c2e5963c573401f624
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] linked list-test: dump a histogram of Update performance

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: linked_list-test: dump a histogram of Update performance
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6576/1/src/kudu/integration-tests/linked_list-test-util.h
File src/kudu/integration-tests/linked_list-test-util.h:

PS1, Line 181: static void DumpHistogram(const HdrHistogram& h);
DumpLatencyHistogram? Also docs


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82d495d55091ee597995c2e5963c573401f624
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

[kudu-CR] tablet: revert batch-check-presence optimization for UPDATE/DELETE

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves,

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

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

to review the following change.

Change subject: tablet: revert batch-check-presence optimization for 
UPDATE/DELETE
..

tablet: revert batch-check-presence optimization for UPDATE/DELETE

4568f2a4cbd999d5a2ab64d285205b8d30997339 implemented an optimization
which does a bulk "check presence" path for all operations in a batch
prior to applying them.

This is beneficial for INSERT, but caused a regression for UPDATE, in
particular in the case where the UPDATEs are affecting rows that are
still present in the MRS. The issue was that the old mutate path would
do:

  for each mutation:
attempt to mutate row in MRS
if it didn't find the row in MRS:
  for each rowset:
attempt to mutate row in that rowset
  check if possibly present (using bloom)
  find the row index
  apply the mutation
if successful, return

With the "bulk presence" optimization, it ended up being:

  bulk presence check:
for each mutation/rowset pair
  try to find if it exists in any non-MRS rowset
check bloom filters
FindRow
if we found it, make a note and stop checking

  for each op:
if we noted a rowset above, apply there
  check bloom
  find the row index
otherwise check MRS

So, in the case that the update applied to a row still in MRS, we used
to avoid doing any bloom reads, but regressed to having to check every
DRS, and only look _last_ in the MRS. Additionally even in the case that
we did find it in an existing DRS, we'd check that DRS's bloom and key
index twice instead of once.

This regression was exposed by some timeouts of the Updater thread in
linked_list-test. In this test, the updater thread "trails" the inserter
thread updating records immediately after they've been inserted.

The fix here is to basically exclude UPDATE/DELETE ops from the "bulk
presence" code path and revert to the old way of performing them. There
is probably a way we could extend the batching optimization to
UPDATE/DELETE as well, but it may require a more complex restructuring,
so I'll defer that to a TODO.

To quantify the regression and this fix, I compared the latency histogram of
1000-row batches of updates from this test:

Before bulk-presence-check patch

  Count: 2454
  Mean: 1740.77
  Percentiles:
 0%  (min) = 1365
25%= 1638
50%  (med) = 1731
75%= 1834
95%= 1997
99%= 2126
99.9%  = 2292
99.99% = 2834
100% (max) = 2835
Total rows inserted: 1929400

After bulk-presence-check-patch (regression)
---
  Mean: 6964.93
  Percentiles:
 0%  (min) = 1578
25%= 4014
50%  (med) = 6560
75%= 9688
95%= 12576 (5-6x slower)
99%= 13672
99.9%  = 14752
99.99% = 14976
100% (max) = 14981
Total rows inserted: 2327900

With this fix in place
---
  Count: 2471
  Mean: 1842.69
  Percentiles:
 0%  (min) = 1405
25%= 1748
50%  (med) = 1835
75%= 1926
95%= 2084
99%= 2216
99.9%  = 2350
99.99% = 2696
100% (max) = 2697
  Total rows inserted: 2471550

The numbers aren't entirely stable across multiple runs of the benchmark
because there's a lot going on, but it seems the regression is largely
fixed.

Change-Id: I78e1e0a0293d737074e1309d2ef450c5439db2c6
---
M src/kudu/tablet/row_op.h
M src/kudu/tablet/tablet.cc
2 files changed, 70 insertions(+), 36 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I78e1e0a0293d737074e1309d2ef450c5439db2c6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 

[kudu-CR] linked list-test: dump a histogram of Update performance

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves,

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

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

to review the following change.

Change subject: linked_list-test: dump a histogram of Update performance
..

linked_list-test: dump a histogram of Update performance

This makes the updater thread in linked_list-test update a histogram of
its latencies before exiting.

In order to get accurate latency readings, I had to switch it back to
MANUAL_FLUSH mode.

This was useful in diagnosing an UPDATE performance regression in a
recent patch.

Change-Id: I6d82d495d55091ee597995c2e5963c573401f624
---
M src/kudu/integration-tests/linked_list-test-util.h
1 file changed, 36 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6d82d495d55091ee597995c2e5963c573401f624
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 

[kudu-CR] Allow to get the raw data from a KuduScanBatch

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Allow to get the raw data from a KuduScanBatch
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6574/1/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

Line 118: 
> Todd: do you mean that we should start versioning wire_protocol and that we
Sort of the other way around...

Given the client implementation currently only speaks one version of "wire 
protocol", we know that given a particular client shared object, it will be 
exactly that format in memory. So, it's enough for the user of the API (i.e. 
Impala) to basically CHECK() that the shared library it's running against 
matches the same one it was built against. We don't need to actually check 
anything about the server side, since the client-side and server-side here are 
tightly coupled.

So one solution would be for impala to CHECK something about the returned 
string from GetKuduVersion() or whatever. But that seems a little fragile in 
that it would have to get updated every time we bumped version number, etc. So 
I'm suggesting we pass it as a parameter here, and have the other side CHECK if 
it doesn't match.

Another solution would be to say that, if we change the in-memory format, we'll 
rename/remove these methods, so that a program linked against the old client 
(expecting the old format) would get a link error if it attempted to run 
against a new client.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c9ad5aa7c5f45a87827352597a404241912342f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Allow to get the raw data from a KuduScanBatch

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Allow to get the raw data from a KuduScanBatch
..


Patch Set 1: Verified+1

Unrelated flake.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c9ad5aa7c5f45a87827352597a404241912342f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Allow to get the raw data from a KuduScanBatch

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Allow to get the raw data from a KuduScanBatch
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6574/1/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

Line 118: 
> I think rather than documenting, we should explicitly document that it's ad
Todd: do you mean that we should start versioning wire_protocol and that we 
should allow to get the version number on the KuduScanBatch?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c9ad5aa7c5f45a87827352597a404241912342f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Allow to get the raw data from a KuduScanBatch

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Allow to get the raw data from a KuduScanBatch
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6574/1/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

Line 118: 
> We should probably document something about the data format. And say someth
I think rather than documenting, we should explicitly document that it's 
advanced/undocumented/unstable/etc with all the warning labels. Anyone who 
wants to consume this should tightly couple themselves to a particular version 
of the client and be advanced enough to read the source code, IMO.

As a safeguard, though, perhaps we should add a parameter like a version number 
and CHECK from the .cc file that the parameter matches? That way if we 
accidentally ran against the wrong client.so, we'd get a CHECK crash instead of 
a surprising result?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c9ad5aa7c5f45a87827352597a404241912342f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Allow to get the raw data from a KuduScanBatch

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Allow to get the raw data from a KuduScanBatch
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6574/1/src/kudu/client/scan_batch.h
File src/kudu/client/scan_batch.h:

Line 118: 
We should probably document something about the data format. And say something 
about the compatibility contract of that format. Are we allowed to change the 
format? Or are we locked into it once exposed in this way?


Line 124:  private:
Nit: separate from indirect_data() with an empty line.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c9ad5aa7c5f45a87827352597a404241912342f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Allow to get the raw data from a KuduScanBatch

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Allow to get the raw data from a KuduScanBatch
..

Allow to get the raw data from a KuduScanBatch

This allows to fetch both the direct and the indirect raw data from
a KuduScanBatch. Exposing this opens the door for Impala to do
whole batch memcpy, instead of row by row. Ideally there would be
no memcpying at all, followup patches will allow for that.

Change-Id: I3c9ad5aa7c5f45a87827352597a404241912342f
---
M src/kudu/client/client-test.cc
M src/kudu/client/scan_batch.cc
M src/kudu/client/scan_batch.h
M src/kudu/client/schema.h
M src/kudu/common/schema.h
5 files changed, 62 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c9ad5aa7c5f45a87827352597a404241912342f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 

[kudu-CR] [python] Fix flaky test test connect timeouts

[Jean-Daniel Cryans (Code Review)]

Hello Todd Lipcon,

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

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

to review the following change.

Change subject: [python] Fix flaky test test_connect_timeouts
..

[python] Fix flaky test test_connect_timeouts

It only tests passing the timeout, but it's low enough that on our poor
jenkins workers it might actually time out connecting to the cluster.

Change-Id: I9df059724ae77c404bccc594e4fa01ac75d905a5
---
M python/kudu/tests/test_client.py
1 file changed, 2 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9df059724ae77c404bccc594e4fa01ac75d905a5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](branch-1.2.x) KUDU-1607. Unpin tablet flush after failed bootstrap

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1607. Unpin tablet flush after failed bootstrap
..


Patch Set 1:

Argh I think this is dependent on 
https://github.com/apache/kudu/commit/4114ba9bf1cdfbd03eef2a13d6e51b6094527997 
so I'll let Mike be the judge of if this is good or not.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id274c6ee1da75bc6f92ab91c0a01edaa009b8962
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-1865 (part 2): pre-allocate OutboundTransfer for inbound calls

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change.

Change subject: WIP: KUDU-1865 (part 2): pre-allocate OutboundTransfer for 
inbound calls
..


Patch Set 2:

Just rebased this. Sorry for the noise.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2ce44d0a00e2dafd1a9858d758ea83ac7923fc0
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-1865 (part 2): pre-allocate OutboundTransfer for inbound calls

[Sailesh Mukil (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP: KUDU-1865 (part 2): pre-allocate OutboundTransfer for 
inbound calls
..

WIP: KUDU-1865 (part 2): pre-allocate OutboundTransfer for inbound calls

This allocates the OutboundTransfer object on the reactor when a call is
first received, to avoid a cross-thread alloc-free pair per RPC.

Conflicts:
src/kudu/rpc/inbound_call.cc
src/kudu/rpc/transfer.h

Change-Id: If2ce44d0a00e2dafd1a9858d758ea83ac7923fc0
---
M src/kudu/rpc/connection.cc
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/inbound_call.h
M src/kudu/rpc/transfer.cc
M src/kudu/rpc/transfer.h
5 files changed, 56 insertions(+), 30 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If2ce44d0a00e2dafd1a9858d758ea83ac7923fc0
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil 

[kudu-CR](branch-1.3.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log 
index
..


KUDU-1933. consensus: Avoid and repair integer overflow in log index

We observed a crash on a long-running master server that looked like the
following:

  F0308 00:25:53.568773  7655 log_index.cc:171] Check failed: log_index > 0 
(-2147483648 vs. 0)

It turns out that this was caused due to integer overflow on the log
index field. This patch fixes this type of truncation in a couple of
places (LogReader and MakeOpId()) and adds a couple of new tests that
fail without both of those fixes.

This patch also adds "repair" logic in log replay during tablet
bootstrap that "reverts" integer overflow when it is detected while
rewriting the log entry.

Finally, this patch includes some test helper fixes to avoid log index
integer truncation in future tests.

Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f
Reviewed-on: http://gerrit.cloudera.org:8080/6376
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
(cherry picked from commit 8363b74506f8513e2fa9dbf772e30d0abce4e444)
Reviewed-on: http://gerrit.cloudera.org:8080/6544
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Jean-Daniel Cryans 
---
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_reader.h
M src/kudu/consensus/opid_util.cc
M src/kudu/consensus/opid_util.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
11 files changed, 236 insertions(+), 27 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved; Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR](branch-1.3.x) KUDU-1933. consensus: Avoid and repair integer overflow in log index

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1933. consensus: Avoid and repair integer overflow in log 
index
..


Patch Set 1: Code-Review+2 Verified+1

Unrelated flake.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I284edbde51dc50fb2f98acc83cdcc3891d37863f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-HasComments: No

[kudu-CR] [mini-cluster] allow more time for masters start up

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [mini-cluster] allow more time for masters start up
..


[mini-cluster] allow more time for masters start up

In case of sanitizer-enabled builds running on slow VMs it might take
more than 5 seconds to start Kudu master.

This patch is to fix master_cert_authority-itest flakiness.

Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
Reviewed-on: http://gerrit.cloudera.org:8080/6571
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
2 files changed, 6 insertions(+), 5 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [mini-cluster] allow more time for masters start up

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [mini-cluster] allow more time for masters start up
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [mini-cluster] allow more time for masters start up

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [mini-cluster] allow more time for masters start up
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6571/1/src/kudu/integration-tests/mini_cluster.cc
File src/kudu/integration-tests/mini_cluster.cc:

Line 302:   static const MonoDelta kDeadlineDelta =
> Maybe define this alongside kRegistrationWaitTimeSeconds?
Good ideas!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [mini-cluster] allow more time for masters start up

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [mini-cluster] allow more time for masters start up
..

[mini-cluster] allow more time for masters start up

In case of sanitizer-enabled builds running on slow VMs it might take
more than 5 seconds to start Kudu master.

This patch is to fix master_cert_authority-itest flakiness.

Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
---
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
2 files changed, 6 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [mini-cluster] allow more time for masters start up

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [mini-cluster] allow more time for masters start up
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6571/1/src/kudu/integration-tests/mini_cluster.cc
File src/kudu/integration-tests/mini_cluster.cc:

Line 302:   static const MonoDelta kDeadlineDelta =
Maybe define this alongside kRegistrationWaitTimeSeconds?

Also, why not a 30s timeout in the non-sanitizer case? A cluster can be under 
excessive load for other reasons.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR](branch-1.2.x) KUDU-1607. Unpin tablet flush after failed bootstrap

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has uploaded a new change for review.

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

Change subject: KUDU-1607. Unpin tablet flush after failed bootstrap
..

KUDU-1607. Unpin tablet flush after failed bootstrap

We have heard reports that, in certain scenarios, a failed tablet is
unable to be deleted. After some investigation, I determined that this
is because we neglect to unpin the Tablet when tablet bootstrap fails.

When a table is being deleted, we delete each tablet superblock by
calling TabletMetadata::DeleteSuperBlock(). This method double-checks
that no orphaned blocks remain referenced to ensure we don't leak disk
space. That requires TabletMetadata::DeleteTabletData() to be called
first, which invokes Flush() twice to ensure that no orphaned blocks are
referenced on disk upon returning. However, if the tablet is pinned,
Flush() silently becomes a no-op (except for a log message that is
printed) and so DeleteSuperBlock() also fails, resulting in the tablet
not being fully deleted and the following warning message being written
to the log file:

W0317 13:05:19.324373 13242 ts_tablet_manager.cc:634] Invalid argument: Unable 
to delete on-disk data from tablet d1b857ddaa0743c79c9bcbd9b2fda174: The 
metadata for tablet d1b857ddaa0743c79c9bcbd9b2fda174 still references orphaned 
blocks. Call DeleteTabletData() first

This patch makes the following changes:

1. Always unpin the tablet after pinning it.
2. Add a new itest that fails without that change, reproducing the
   above warning message.
3. Add a little more test infra to MiniCluster (and the MiniClusterBase
   interface) to make it easier to use the helper functions in
   cluster_itest_util.h in MiniCluster-based itests.

Change-Id: Id274c6ee1da75bc6f92ab91c0a01edaa009b8962
Reviewed-on: http://gerrit.cloudera.org:8080/6422
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
(cherry picked from commit 0450f77f69c74cc6dec08ad36bb89ac12c17608f)
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/mini_cluster_base.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
10 files changed, 224 insertions(+), 48 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id274c6ee1da75bc6f92ab91c0a01edaa009b8962
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 

[kudu-CR](branch-1.3.x) KUDU-1607. Unpin tablet flush after failed bootstrap

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: KUDU-1607. Unpin tablet flush after failed bootstrap
..


KUDU-1607. Unpin tablet flush after failed bootstrap

We have heard reports that, in certain scenarios, a failed tablet is
unable to be deleted. After some investigation, I determined that this
is because we neglect to unpin the Tablet when tablet bootstrap fails.

When a table is being deleted, we delete each tablet superblock by
calling TabletMetadata::DeleteSuperBlock(). This method double-checks
that no orphaned blocks remain referenced to ensure we don't leak disk
space. That requires TabletMetadata::DeleteTabletData() to be called
first, which invokes Flush() twice to ensure that no orphaned blocks are
referenced on disk upon returning. However, if the tablet is pinned,
Flush() silently becomes a no-op (except for a log message that is
printed) and so DeleteSuperBlock() also fails, resulting in the tablet
not being fully deleted and the following warning message being written
to the log file:

W0317 13:05:19.324373 13242 ts_tablet_manager.cc:634] Invalid argument: Unable 
to delete on-disk data from tablet d1b857ddaa0743c79c9bcbd9b2fda174: The 
metadata for tablet d1b857ddaa0743c79c9bcbd9b2fda174 still references orphaned 
blocks. Call DeleteTabletData() first

This patch makes the following changes:

1. Always unpin the tablet after pinning it.
2. Add a new itest that fails without that change, reproducing the
   above warning message.
3. Add a little more test infra to MiniCluster (and the MiniClusterBase
   interface) to make it easier to use the helper functions in
   cluster_itest_util.h in MiniCluster-based itests.

Change-Id: Id274c6ee1da75bc6f92ab91c0a01edaa009b8962
Reviewed-on: http://gerrit.cloudera.org:8080/6422
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
(cherry picked from commit 0450f77f69c74cc6dec08ad36bb89ac12c17608f)
Reviewed-on: http://gerrit.cloudera.org:8080/6545
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/mini_cluster_base.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
10 files changed, 224 insertions(+), 48 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id274c6ee1da75bc6f92ab91c0a01edaa009b8962
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR](branch-1.3.x) KUDU-1607. Unpin tablet flush after failed bootstrap

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1607. Unpin tablet flush after failed bootstrap
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id274c6ee1da75bc6f92ab91c0a01edaa009b8962
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [mini-cluster] allow more time for masters start up

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [mini-cluster] allow more time for masters start up
..

[mini-cluster] allow more time for masters start up

In case of sanitizer-enabled builds running on slow VMs it might take
more than 5 seconds to start Kudu master.

This patch is to fix master_cert_authority-itest flakiness.

Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
---
M src/kudu/integration-tests/mini_cluster.cc
1 file changed, 10 insertions(+), 5 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic3dd9aa9c1620d69b6ae4a877912e5d63e4d5a85
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 

[kudu-CR] [kudu-jepsen] install Kudu packages into local repo

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [kudu-jepsen] install Kudu packages into local repo
..


[kudu-jepsen] install Kudu packages into local repo

Install Kudu Java packages into the local maven repository prior to
running kudu-jepsen tests.

Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
Reviewed-on: http://gerrit.cloudera.org:8080/6256
Reviewed-by: David Ribeiro Alves 
Tested-by: Kudu Jenkins
---
M src/kudu/scripts/jepsen.sh
1 file changed, 9 insertions(+), 9 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [kudu-jepsen] install Kudu packages into local repo

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [kudu-jepsen] install Kudu packages into local repo
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [kudu-jepsen] install Kudu packages into local repo

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [kudu-jepsen] install Kudu packages into local repo
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6256/2/src/kudu/scripts/jepsen.sh
File src/kudu/scripts/jepsen.sh:

PS2, Line 126: compile test-compile
> this is no longer needed (install already does this)
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [kudu-jepsen] install Kudu packages into local repo

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [kudu-jepsen] install Kudu packages into local repo
..

[kudu-jepsen] install Kudu packages into local repo

Install Kudu Java packages into the local maven repository prior to
running kudu-jepsen tests.

Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
---
M src/kudu/scripts/jepsen.sh
1 file changed, 9 insertions(+), 9 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [kudu-jepsen] install Kudu packages into local repo

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [kudu-jepsen] install Kudu packages into local repo
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6256/2/src/kudu/scripts/jepsen.sh
File src/kudu/scripts/jepsen.sh:

PS2, Line 126: compile test-compile
this is no longer needed (install already does this)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [kudu-jepsen] install Kudu packages into local repo

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [kudu-jepsen] install Kudu packages into local repo
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6256/1/src/kudu/scripts/jepsen.sh
File src/kudu/scripts/jepsen.sh:

PS1, Line 126: mvn $MVN_FLAGS -Pjepsen clean compile test-compile
 : 
 : echo
 : echo "Installing Kudu Java packages into local mvn repository"
 : echo 
 : mvn $MVN_FLAGS -DskipTests install
> why not do the install in the first mvn call?
I thought it's worth separating build and install targets.  To me it does not 
seem natural to mix those.

But since it seems it would re-do the dependent targets again, it makes more 
sense to put them into one line.

Done.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [kudu-jepsen] install Kudu packages into local repo

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [kudu-jepsen] install Kudu packages into local repo
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6256/1/src/kudu/scripts/jepsen.sh
File src/kudu/scripts/jepsen.sh:

PS1, Line 126: mvn $MVN_FLAGS -Pjepsen clean compile test-compile
 : 
 : echo
 : echo "Installing Kudu Java packages into local mvn repository"
 : echo 
 : mvn $MVN_FLAGS -DskipTests install
why not do the install in the first mvn call?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I056fb9c5791fbd4850df7a87d8d6f3014ec3284f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] WIP: KUDU-1713: add a client Partitioner API

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: WIP: KUDU-1713: add a client Partitioner API
..


Patch Set 2:

> Adar, did you have any more feedback? We'll wanna start consuming
 > this API pretty soon.

Oh, I thought it was still a WIP.

Todd, is this patch ready for review? If so, could you refresh it and remove 
the WIP comments?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic08a078c75f15ef4200219b5260cfb99d79b72cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [kudu-jepsen] clean up on require/import lists

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [kudu-jepsen] clean up on require/import lists
..


[kudu-jepsen] clean up on require/import lists

Change-Id: If3b25dd802e115dc4b0f0bc8aecef5fb2326438a
Reviewed-on: http://gerrit.cloudera.org:8080/6570
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
5 files changed, 5 insertions(+), 19 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If3b25dd802e115dc4b0f0bc8aecef5fb2326438a
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [kudu-jepsen] clean up on require/import lists

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [kudu-jepsen] clean up on require/import lists
..


Patch Set 2: Code-Review+2

Re-based the patch, carrying over David's +2.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If3b25dd802e115dc4b0f0bc8aecef5fb2326438a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [kudu-jepsen] clean up on require/import lists

[Alexey Serbin (Code Review)]

Hello David Ribeiro Alves, Kudu Jenkins,

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

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

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

Change subject: [kudu-jepsen] clean up on require/import lists
..

[kudu-jepsen] clean up on require/import lists

Change-Id: If3b25dd802e115dc4b0f0bc8aecef5fb2326438a
---
M java/kudu-jepsen/src/main/clojure/jepsen/kudu.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/client.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/nemesis.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/table.clj
5 files changed, 5 insertions(+), 19 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If3b25dd802e115dc4b0f0bc8aecef5fb2326438a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [docs] Add security guide

[Ambreen Kazi (Code Review)]

Ambreen Kazi has posted comments on this change.

Change subject: [docs] Add security guide
..


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/6479/4/docs/security.adoc
File docs/security.adoc:

PS4, Line 31: will explain
describes


PS4, Line 66: requiring
: certificates be manually deployed on every node.
requiring you to manually deploy certificates on every node.


PS4, Line 168: turned off configuring the
 : `--redact` flag
by setting --redact to false?


PS4, Line 220: yet
Remove 'yet'.


PS4, Line 226: yet 
Remove.


PS4, Line 229:  yet
Remove'.


PS4, Line 242: The 
Remove 'the'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabf60804975dc105243626be48d3a141c9a4dab5
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] [kudu-jepsen] updated client model for Jepsen tests

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [kudu-jepsen] updated client model for Jepsen tests
..


[kudu-jepsen] updated client model for Jepsen tests

Use a single Kudu client object across all test actors.  This is
to achieve automatic timestamp propagation between all operations.

Prior to this patch, every test actor operated in the context of a
separate Kudu client.  That did not make much sense for running
consistency tests: by design, in Kudu the consistency for operations
performed by multiple clients can be achieved only if passing timestamp
between them.

Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
Reviewed-on: http://gerrit.cloudera.org:8080/6565
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
1 file changed, 16 insertions(+), 17 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP [java client] fixed NPE in master RPC error handler

[Alexey Serbin (Code Review)]

Alexey Serbin has abandoned this change.

Change subject: WIP [java client] fixed NPE in master RPC error handler
..


Abandoned

Partially addressed by bee8e875e21acb1d36f104f8388fc9ed6c963821, and in the 
long run this should be fixed by other means avoiding the 'masterTable' hack.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ib923212db1e0931ed63b31a39f03335c701f91cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [kudu-jepsen] updated client model for Jepsen tests

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: [kudu-jepsen] updated client model for Jepsen tests
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1424. Add getters to PartialRow

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1424. Add getters to PartialRow
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6554/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java:

PS1, Line 149: A
nit: here and everywhere below, start with a lower case.


http://gerrit.cloudera.org:8080/#/c/6554/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java:

Line 82: // This is the expected exception
nit: Here and elsewhere, inline comments should end with a period.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c751eda9e8d6da5dd6ddd2ec798259bc037fb7d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP: KUDU-1713: add a client Partitioner API

[Matthew Jacobs (Code Review)]

Matthew Jacobs has posted comments on this change.

Change subject: WIP: KUDU-1713: add a client Partitioner API
..


Patch Set 2:

Adar, did you have any more feedback? We'll wanna start consuming this API 
pretty soon.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic08a078c75f15ef4200219b5260cfb99d79b72cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [docs] Add security guide

[Dan Burkert (Code Review)]

Hello Hao Hao, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: [docs] Add security guide
..

[docs] Add security guide

Change-Id: Iabf60804975dc105243626be48d3a141c9a4dab5
---
A docs/security.adoc
1 file changed, 243 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iabf60804975dc105243626be48d3a141c9a4dab5
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [docs] Add security guide

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [docs] Add security guide
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6479/3/docs/security.adoc
File docs/security.adoc:

PS3, Line 113: are
> is
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabf60804975dc105243626be48d3a141c9a4dab5
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-579 [java client] Scanner fault tolerance

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-579 [java_client] Scanner fault tolerance
..


Patch Set 2:

(18 comments)

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

Line 78:* Return scan results in primary key sorted order.
This javadoc needs to be re-written.


Line 89:   @InterfaceAudience.Private
Remove all the Interface stuff, we don't want this to be private anymore.


Line 91:   S setFaultTolerant() {
Make it public.


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

Line 505: if (e instanceof ScannerExpiredException) {
Shouldn't you only do this for fault tolerant scanners? Also it seems there's a 
type mismatch now that we're returning a Deferred. This change is a bit 
suspicious.


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

Line 113:   @VisibleForTesting
We put this above the method signature, below the javadoc.


PS2, Line 115: tabletSlice
RemoteTablet?


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

Line 179: // if uuid is empty, it means the leader has been removed from
I'm not following this. If the leader isn't known then we need to go ask the 
master. Using a different replication selection policy would probably violate 
what the user is asking for.


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

Line 2:  * Copyright (C) 2010-2012  The Async HBase Authors.  All rights 
reserved.
Use a difference license, like KuduSession has, since this file AFAIK isn't 
copied from asynchbase.


PS2, Line 38: KuduException
This should be a RecoverableException.


PS2, Line 42: .
Remove trailing periods from all @param lines


PS2, Line 51: T
Always start with a lower case.


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

PS2, Line 515: TABLET_NOT_RUNNING
I'm surprised that we've been missing this for so long, but it seems like it 
was indeed a problem.


http://gerrit.cloudera.org:8080/#/c/6566/2/java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
File java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java:

PS2, Line 377: .
nit


Line 393:   private static LocatedTablet.Replica getLeaderRpcPort(RemoteTablet 
curTablet)
Part of this is copied from killTabletLeader(KuduTable), can you do the 
required refactoring instead?


PS2, Line 477: .
nit


Line 483: miniCluster.killTabletServerOnPort(port);
Another refactoring opportunity.


http://gerrit.cloudera.org:8080/#/c/6566/2/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:

Line 53: 
builder.addHashPartitions(Lists.newArrayList(schema.getColumnByIndex(0).getName()),
 TABLET_COUNT);
nit: now the line is too long.


Line 152:   public void testFaultTolerantScannerRestartTS() throws Exception {
This should be refactor into one helper method and two tests that simple call 
the method to tell it to either kill or restart.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [docs] Add security guide

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change.

Change subject: [docs] Add security guide
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6479/3/docs/security.adoc
File docs/security.adoc:

PS3, Line 113: are
is


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iabf60804975dc105243626be48d3a141c9a4dab5
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] [kudu-jepsen] updated client model for Jepsen tests

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [kudu-jepsen] updated client model for Jepsen tests
..

[kudu-jepsen] updated client model for Jepsen tests

Use a single Kudu client object across all test actors.  This is
to achieve automatic timestamp propagation between all operations.

Prior to this patch, every test actor operated in the context of a
separate Kudu client.  That did not make much sense for running
consistency tests: by design, in Kudu the consistency for operations
performed by multiple clients can be achieved only if passing timestamp
between them.

Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
---
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
1 file changed, 16 insertions(+), 17 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [kudu-jepsen] updated client model for Jepsen tests

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [kudu-jepsen] updated client model for Jepsen tests
..


Patch Set 1:

(1 comment)

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

PS1, Line 10: write and scan
: operations.
> What is write-write and scan-scan?
Oh, I see what you mean.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [kudu-jepsen] updated client model for Jepsen tests

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [kudu-jepsen] updated client model for Jepsen tests
..

[kudu-jepsen] updated client model for Jepsen tests

Use a single Kudu client object across all test actors.  This is
to achieve automatic timestamp propagation between all operations.

Prior to this change, every writer and reader operated in the context
of a separate client, which does not make much sense while performing
consistency testing.

Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
---
M java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
1 file changed, 16 insertions(+), 17 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [kudu-jepsen] updated client model for Jepsen tests

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [kudu-jepsen] updated client model for Jepsen tests
..


Patch Set 1:

(4 comments)

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

PS1, Line 10: write and scan
: operations.
> all operations.  (that is this also does write-write and scan-scan)
What is write-write and scan-scan?


http://gerrit.cloudera.org:8080/#/c/6565/1/java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj
File java/kudu-jepsen/src/main/clojure/jepsen/kudu/register.clj:

PS1, Line 40: state-atom
> why state-atom and not "client-atom"?
Because I wanted to express the idea of keeping the state there.  Probably, 
client-atom is better.


PS1, Line 45: newclient (kc/sync-client (:master-addresses test))
> you're always creating a new client even if you don't need it. could you ju
true -- code-wise it seemed the simplest way of doing that.


PS1, Line 47: compare-and-set! state-atom nil newclient
> I mean doing:
good idea.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If67328230cc821129babcb5fa40fcbbb503eea39
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes