date:20170912

Mike Percy has posted comments on this change.

Change subject: Add tablet state summary metrics
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7980/8/src/kudu/tserver/ts_tablet_manager_metrics.h
File src/kudu/tserver/ts_tablet_manager_metrics.h:

PS8, Line 56:   TSTabletManager* ts_tablet_manager;
: 
:   FunctionGaugeDetacher metric_detacher;
Don't shoot me, but shouldn't these two members be private? Aren't they never 
meant to be accessed by other classes?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2135 (part 1): add persistent disk states

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2135 (part 1): add persistent disk states
..

KUDU-2135 (part 1): add persistent disk states

This patch introduces a new format for PathInstanceMetadataFiles that
includes a health state and path for each data directory, and a
timestamp that the path instance file was last updated.

These additions will be necessary to ensure that failed disks will not
be used and that data is where it is expected be (which will be
increasingly important as we begin to supporting removal, addition, and
replacement of disks).

The expected usage of the new format would enforce startup restrictions
on the server (e.g. to never use a previously failed disk, to not use
data if it is on a different disk than expected, etc.) and tooling does
not yet exist to work around these restrictions. As such, this patch
will upgrade and maintain the new path sets as necessary, persisting
disk health when disks fail, but will not apply the mentioned
restrictions at startup.

Tests are added to data_dirs-test to ensure the path instance files get
updated and upgraded appropriately.

Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/block_manager_util.h
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
7 files changed, 473 insertions(+), 76 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Add tablet state summary metrics

2017-09-12 Thread Will Berkeley (Code Review)

Will Berkeley has posted comments on this change.

Change subject: Add tablet state summary metrics
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7980/7/src/kudu/tserver/ts_tablet_manager_metrics.h
File src/kudu/tserver/ts_tablet_manager_metrics.h:

Line 47:   int num_not_initialized_;
> nit: don't suffix public struct fields with _ per https://google.github.io/
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] Add tablet state summary metrics

2017-09-12 Thread Will Berkeley (Code Review)

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: Add tablet state summary metrics
..

Add tablet state summary metrics

This patch adds metrics for the number of tablets in each state
(i.e. the number of tablets for each value of TabletStatePB). To
avoid contention in the tablet manager, the frequency with which
the state numbers are computed is limited. Requests that come
too soon for an update will receive cached counts. The maximum
frequency of updates is controlled by a new flag.

Change-Id: I9fda2061d341586f0aa343787af59984a627080a
---
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
A src/kudu/tserver/ts_tablet_manager_metrics.cc
A src/kudu/tserver/ts_tablet_manager_metrics.h
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
8 files changed, 379 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley

[kudu-CR] KUDU-2135 (part 2): check integrity of new on-disk format

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2135 (part 2): check integrity of new on-disk format
..

KUDU-2135 (part 2): check integrity of new on-disk format

CheckIntegrity() is updated to ensure that disks that have previously
failed are not used and that servers will only open data where it's
previously known to be.

Rather than comparing all instances against an agreed-upon set of UUIDs,
the single path set with the largest timestamp is used as the main one
to compare against (if only old path sets are available, the first
healthy one is used). If no instances are healthy, CheckIntegrity() will
fail, as all disks are failed.

Unit testing is done for the new iteration of CheckIntegrity().

Some notes:
- Disk failures during FS layout creation are not tolerated. In these
cases, there is presumably no data on the server anyway, so operators
should easily be able to fix their cluster.
- If there are any unhealthy instances when upgrading the path sets (i.e.
adding disk states, paths, timestamp), a complete mapping of UUIDs to
paths will not be available, and CheckIntegrity() will fail.
- The main path set's disk states are updated to reflect the failure of
the instances. Since data on a failed disk is not used, disks
that are successfully read from but are already marked FAILED by the
main path set are not marked HEALTHY.
- In the case of a server restart where all healthy disks fail to start
up and some known failed disks start working again, the server will
successfully start up with the bad disks

This patch is a part of a series of patches to handle disk failures. To
see how this fits, see 2.6 in:
https://docs.google.com/document/d/1zZk-vb_ETKUuePcZ9ZqoSK2oPvAAaEV1sjDXes8Pxgk/edit?usp=sharing

Change-Id: Ifddf0817fe1a82044077f5544c400c88de20769f
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/fs.proto
5 files changed, 328 insertions(+), 117 deletions(-)

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifddf0817fe1a82044077f5544c400c88de20769f
Gerrit-PatchSet: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2135 (part 1): add persistent disk states

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-2135 (part 1): add persistent disk states
..

KUDU-2135 (part 1): add persistent disk states

This patch introduces a new format for PathInstanceMetadataFiles that
includes a disk health and path for each data directory, and a timestamp
that the path instance file was last updated.

These additions will be necessary to ensure failed disks will not be
used and that data is where expect it to be (which will be increasingly
important as we begin to supporting removal/addition/replacement of
disks).

The expected usage of the new format would enforce startup restrictions
on the server (e.g. to not use a failed disk, to not use data if it is
on a different disk than expected, etc.) and tooling does not yet exist
to work around these restrictions. As such, this patch will upgrade and
maintain the new path sets as necessary, persisting disk health when
disks fail, but will not apply the mentioned restrictions at startup.

A test is added to data_dirs-test to ensure the path instance files get
updated/upgraded appropriately.

Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/block_manager_util.h
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
7 files changed, 470 insertions(+), 76 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] docs: clarify steps for removing master from multi-master deployment

Mike Percy has posted comments on this change.

Change subject: docs: clarify steps for removing master from multi-master 
deployment
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8032/2/docs/administration.adoc
File docs/administration.adoc:

PS2, Line 382: WARNING: dropping the number of masters below the number of 
masters currently needed for a Raft
 : majority can incur data loss.
> Please double check this with Mike.
You are technically correct (the best kind of correct) but there are nuances we 
might consider explaining here.

Data loss is kind of a funny concept in this case because we're really talking 
about *metadata* loss, which can actually be catastrophic: for example it can 
cause the master to end up dropping whole tables, I think, depending on how far 
behind the remaining node was, like if it went offline before you created a new 
table or partition. However, I'm not sure whether we've ever tested this 
scenario and I'm pretty confident that we don't have an *automated* test for it 
either.

This is particularly scary if you had one master that was down or partitioned 
for a long time, and you end up removing the other two, and then this stale 
remaining master comes back online and is all we have left.

In general, if you remove more than a majority at once and then do some kind of 
manual repair, we don't give you any durability guarantees at all.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4196dbb2f8a185e868a6906c7cf917d79c404c0d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures

Mike Percy has posted comments on this change.

Change subject: KUDU-2125: Tablet copy client does not retry on failures
..


Patch Set 10:

(5 comments)

Just one thing I'm wondering about, in the tablet copy client.

http://gerrit.cloudera.org:8080/#/c/8016/10/src/kudu/integration-tests/cluster_itest_util.cc
File src/kudu/integration-tests/cluster_itest_util.cc:

PS10, Line 856: std::remove_if
fancy


PS10, Line 858:  != 
Neat: I didn't know you could check for equality between a thing and an 
optional of a thing.


http://gerrit.cloudera.org:8080/#/c/8016/4/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
File src/kudu/integration-tests/tablet_copy_client_session-itest.cc:

PS4, Line 305: or Turns out it was caught because of some existing calls to HasFailure() in e
I'm surprised at this. I've had weird issues trying to do this before and now I 
always use CHECK_OK outside of the main thread. Since the docs claim otherwise, 
do you have any evidence that this is safe to do?


http://gerrit.cloudera.org:8080/#/c/8016/10/src/kudu/tserver/tablet_copy_client.cc
File src/kudu/tserver/tablet_copy_client.cc:

PS10, Line 702: >=
why >= and not == ?


Line 747:   // Polynomial backoff with 50% jitter.
Have you used this before? I plotted y=10x^2 and it seems pretty good -- well 
behaved and no need for a cap.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Kudu Consistency Blog Post Pt1

Alexey Serbin has posted comments on this change.

Change subject: Kudu Consistency Blog Post Pt1
..


Patch Set 12:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7019/12/_posts/2017-08-21-kudu-consistency-pt1.md
File _posts/2017-08-21-kudu-consistency-pt1.md:

PS12, Line 54:  
> nit: Dynamo[3]-inspired? Dynamo-inspired[3]?
another option might be: ... systems inspired by _Dynamo_[3], ...


PS12, Line 104:  so
Consider starting a new sentence from here for better readability:

... feature).  So, when discussing ...


PS12, Line 119: wall time
wall clock time ?


PS12, Line 152: programatically
typo: programmatically


Line 171: [[3]](www.allthingsdistributed.com/files/amazon-dynamo-sosp2007.pdf): 
Giuseppe DeCandia, Deniz Hastorun, Madan Jampani, Gunavardhan Kakulapati, 
Avinash Lakshman, Alex Pilchin, Swaminathan Sivasubramanian, Peter Vosshall, 
and Werner Vogels. 2007. Dynamo: amazon's highly available key-value store. In 
Proceedings of twenty-first ACM SIGOPS symposium on Operating systems 
principles (SOSP '07). ACM, New York, NY, USA
nit: consider adding the protocol prefix 'http://'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icaec0c8ace20651a65901a8e1786e785265540d1
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2135 (part 1): add persistent disk states

Andrew Wong has uploaded a new change for review.

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

Change subject: KUDU-2135 (part 1): add persistent disk states
..

KUDU-2135 (part 1): add persistent disk states

This patch introduces a new format for PathInstanceMetadataFiles that
includes a disk health and path for each data directory, and a timestamp
that the path instance file was last updated.

These additions will be necessary to ensure failed disks will not be
used and that data is where expect it to be (which will be increasingly
important as we begin to supporting removal/addition/replacement of
disks).

The expected usage of the new format would enforce startup restrictions
on the server (e.g. to not use a failed disk, to not use data if it is
on a different disk than expected, etc.) and tooling does not yet exist
to work around these restrictions. As such, this patch will upgrade and
maintain the new path sets as necessary, persisting disk health when
disks fail, but will not apply the mentioned restrictions at startup.

A test is added to data_dirs-test to ensure the path instance files get
updated/upgraded appropriately.

Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/block_manager_util.h
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
7 files changed, 467 insertions(+), 76 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia7ea6418f6190d7d0c6d5a1aaca4ca592c49ce41
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong

[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession

Mike Percy has posted comments on this change.

Change subject: KUDU-2124. Don't hold session lock while initializing a 
TabletCopySession
..


Patch Set 8: -Code-Review

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7985/8/src/kudu/integration-tests/cluster_itest_util.cc
File src/kudu/integration-tests/cluster_itest_util.cc:

Line 1019: *error_code = error.code();
I'm not sure if you did this or I did this, but if error_code is passed in as 
nullptr this will result in a segfault, which is bad because we default it to 
nullptr in the header file.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession

Mike Percy has posted comments on this change.

Change subject: KUDU-2124. Don't hold session lock while initializing a 
TabletCopySession
..


Patch Set 8: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7985/1/src/kudu/tserver/tablet_copy_source_session.cc
File src/kudu/tserver/tablet_copy_source_session.cc:

Line 166:   if (!reader) {
> Hmm, I'm not seeing where the log_ field in TabletReplica is getting reset?
Ah, you're right.


http://gerrit.cloudera.org:8080/#/c/7985/3/src/kudu/tserver/tablet_copy_source_session.cc
File src/kudu/tserver/tablet_copy_source_session.cc:

Line 73: TAG_FLAG(tablet_copy_session_inject_latency_on_init_ms, hidden);
> Done
Unsafe implies hidden, so explicit hidden isn't actually needed. But not a big 
deal.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add tablet state summary metrics

Mike Percy has posted comments on this change.

Change subject: Add tablet state summary metrics
..


Patch Set 7:

(1 comment)

looks good, just one nit on something that changed since the last review

http://gerrit.cloudera.org:8080/#/c/7980/7/src/kudu/tserver/ts_tablet_manager_metrics.h
File src/kudu/tserver/ts_tablet_manager_metrics.h:

Line 47:   int num_not_initialized_;
nit: don't suffix public struct fields with _ per 
https://google.github.io/styleguide/cppguide.html#Variable_Names


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fda2061d341586f0aa343787af59984a627080a
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries.

Mike Percy has posted comments on this change.

Change subject: KUDU-1788. Increase Raft RPC timeout to 30sec to avoid 
fruitless retries.
..


Patch Set 1:

(1 comment)

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

Line 51: DEFINE_int32(consensus_rpc_timeout_ms, 3,
This doesn't appear to prevent stacking, does it? I don't see a mechanism for 
the heartbeater not to trigger Peer::SignalRequest() while a consensus request 
is outstanding. Is that what we want?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f47dc006dc3dfb1659a224172e1905b6bf3d2a4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1807 (part 3): remove GetTableSchema.create table done

Adar Dembo has posted comments on this change.

Change subject: KUDU-1807 (part 3): remove GetTableSchema.create_table_done
..


Patch Set 1: Verified+1

NTP synchronization error in one of the dist-test slaves, apparently.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5395bd73edd9cf9d18c4cb6db4f8fb39e5a41830
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1807 (part 1): deprecate GetTableSchema.create table done

Hello Dan Burkert, Todd Lipcon,

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

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

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

Change subject: KUDU-1807 (part 1): deprecate GetTableSchema.create_table_done
..

KUDU-1807 (part 1): deprecate GetTableSchema.create_table_done

Improving IsCreateInProgress performance in the master is very challenging
due to the myriad of locks in play. A simpler alternative would be to get it
off any critical paths. It's currently called by the IsCreateTableDone and
GetTableSchema RPCs. The former makes sense but the latter does not: if we
need to know whether a table has been created, well, that's what the
IsCreateTableDone RPC is for.

So, does anyone use the "is table done creating" aspect of GetTableSchema?
The C++ client doesn't, but the Java client does. As luck would have it, the
Java client is already written to fall back to IsCreateTableDone if
GetTableSchema says the table is still being created, which means we can
safely modify the master to always return false in this field without
breaking old Java clients.

Future patches will change the behavior of the Java client and remove the
create_table_done field altogether. For now, the existence of this
deprecation patch helps to ensure that the Java client does, in fact, work
properly if the field's value is always false.

Change-Id: Ia33bbb2abaabb97db1613d442a7d065710048cc2
---
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
3 files changed, 3 insertions(+), 16 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia33bbb2abaabb97db1613d442a7d065710048cc2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1807 (part 2): ban GetTableSchema for table createdness in clients

Adar Dembo has posted comments on this change.

Change subject: KUDU-1807 (part 2): ban GetTableSchema for table createdness in 
clients
..


Patch Set 1:

(9 comments)

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

PS1, Line 23: Meanwhile
> I read this to mean that this patch is both modifying the master as describ
I'll try to clarify while still providing context to understand the motivation 
for this patch.


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

Line 524:* Gets a table's schema
> Missing period
Done


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

Line 36:   IsCreateTableDoneRequest(KuduTable table, String name) {
> Doesn't identifying the table by name open the client up to a race conditio
I suppose so, but IsAlterTableDoneRequest behaves this way, as do both 
IsAlterTableDone and IsCreateTableDone on the C++ side. Moreover, in the 
standalone case (i.e. if a user just makes those client calls), the table name 
is all you have anyway.

So I'd prefer to keep them consistent.


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

Line 99:   public boolean isCreateTableDone(String name) throws KuduException {
> Why is this not a joinAndHandleException wrapper around an async variant?
I was copying isAlterTableDone without thinking about it too hard. In order to 
actually wait for the creation to finish, joining the deferred is insufficient; 
we need a loop that sleeps and retries too.


Line 101: while (totalSleepTime < getDefaultAdminOperationTimeoutMs()) {
> This implementation appears to be waiting for the table to be created if it
I don't want to change the method name (again, to be consistent with 
IsAlterTableDone), but I will reword the Javadocs to make this behavior more 
clear.


Line 121:   LOG.debug("Create not done, sleep " + 
(AsyncKuduClient.SLEEP_TIME - elapsed) +
> This log message doesn't make a lot of sense in isolation, and restructure 
I think it's OK that it doesn't make much sense since it's debug-level 
(something we'd normally enable just for isolated testing).

I will restructure it (and the version in IsAlterTableDone) though.


Line 132: return false;
> Shouldn't this throw a timeout exception?
That's not the behavior of IsAlterTableDone, and I was trying to be consistent 
with it.


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

Line 940: if (e.getMessage().contains("does not exist")) {
> I think this could be more precise by asserting on the returned Status (Not
Done


Line 954:   Insert insert = table.newInsert();
> Shouldn't this work even without waiting for isCreateTableDone?  I would ha
Yes, we discussed this yesterday. Will make the change.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fa07dc34a97f1c9da06ec9129d6d4590b7aac6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1807 (part 2): ban GetTableSchema for table createdness in clients

Hello Dan Burkert, Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: KUDU-1807 (part 2): ban GetTableSchema for table createdness in 
clients
..

KUDU-1807 (part 2): ban GetTableSchema for table createdness in clients

This patch modifies new Java clients to no longer use the create_table_done
field in GetTableSchema RPCs. After the CreateTable RPC returns, the client
will enter an IsCreateTableDone loop, and when that's done, it'll issue a
GetTableSchema to fetch the schema and build a KuduTable.

There are a couple other changes worth noting:
- openTable no longer waits for table creation before returning. Doing that
  now would require at least one IsCreateTableDone RPC, which basically
  defeats the purpose of these changes. Moreover, the C++ client doesn't do
  this, so I don't see why the Java client should.
- I removed the 'tablesNotServed' logic which had no useful effect because,
  as I recently learned, there's retry logic to deal with TABLET_NOT_RUNNING
  master errors embedded more deeply in the Java client.
- I removed the master permit acquisition from the "is create table done"
  loop, since these loops should no longer produce a thundering herds (as I
  suppose they could have due to 'tablesNotServed').
- Even though createTable waits for table creation to finish, I added an
  isCreateTableDone method to help synchronize the use case where multiple
  clients or threads all try to create and then open a table concurrently.

Change-Id: I54fa07dc34a97f1c9da06ec9129d6d4590b7aac6
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/DeadlineTracker.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableSchemaRequest.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/GetTableSchemaResponse.java
M 
java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneRequest.java
A 
java/kudu-client/src/main/java/org/apache/kudu/client/IsCreateTableDoneResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestDeadlineTracker.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestFlexiblePartitioning.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
12 files changed, 398 insertions(+), 253 deletions(-)


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

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

[kudu-CR] KUDU-1807 (part 3): remove GetTableSchema.create table done

Hello Dan Burkert, Todd Lipcon,

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

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

to review the following change.

Change subject: KUDU-1807 (part 3): remove GetTableSchema.create_table_done
..

KUDU-1807 (part 3): remove GetTableSchema.create_table_done

Since this field was optional, and since the Java client is no longer using
it, let's remove it entirely. The protobuf 'reserved field' functionality
ensures that the field number isn't accidentally reused in the future.

Change-Id: I5395bd73edd9cf9d18c4cb6db4f8fb39e5a41830
---
M src/kudu/master/master.proto
1 file changed, 3 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5395bd73edd9cf9d18c4cb6db4f8fb39e5a41830
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2055 [part 2]: Add util to construct sorted disjoint interval

2017-09-12 Thread Hao Hao (Code Review)

Hao Hao has uploaded a new change for review.

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

Change subject: KUDU-2055 [part 2]: Add util to construct sorted disjoint 
interval
..

KUDU-2055 [part 2]: Add util to construct sorted disjoint interval

This patch adds an utility to construct a sorted disjoint interval set
given a set of intervals. The operation to construct such one is
O(nlgn + n) where 'n' is the number of intervals.

For example, given the input interval set:
   |--1---| |-2-|
   |--3--||---4--||5|

The output sorted disjoint interval set is:
   |--1--|  |-2-|

It also adds unit test to verify given overlap, duplicate, invalid
intervals, the implementation works as expected.

Change-Id: I61a813c047be4882f246eaf404598e7e18fcac87
---
M src/kudu/util/CMakeLists.txt
A src/kudu/util/sorted_disjoint_interval-test.cc
A src/kudu/util/sorted_disjoint_interval.h
3 files changed, 261 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I61a813c047be4882f246eaf404598e7e18fcac87
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao

[kudu-CR](gh-pages) Kudu Consistency Blog Post Pt1

2017-09-12 Thread Will Berkeley (Code Review)

Will Berkeley has posted comments on this change.

Change subject: Kudu Consistency Blog Post Pt1
..


Patch Set 12:

(3 comments)

Just a couple of things + Andrew's nits

http://gerrit.cloudera.org:8080/#/c/7019/12/_posts/2017-08-21-kudu-consistency-pt1.md
File _posts/2017-08-21-kudu-consistency-pt1.md:

PS12, Line 35:  prevent
> nit: consider "that prevent"
+1


PS12, Line 57: availability
This is a touch confusing because availability is being maximized by minimizing 
insertion errors + insert latency.


PS12, Line 60: ,
Remove


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icaec0c8ace20651a65901a8e1786e785265540d1
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: Yes

[kudu-CR] docs: clarify steps for removing master from multi-master deployment

Adar Dembo has posted comments on this change.

Change subject: docs: clarify steps for removing master from multi-master 
deployment
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8032/1/docs/administration.adoc
File docs/administration.adoc:

Line 374:  Removing masters from a Multi-Master Deployment
> Done.
I don't know; I think you should ask Mike for clarification. I don't think ksck 
can tell you which master is most 'up-to-date'.


http://gerrit.cloudera.org:8080/#/c/8032/2/docs/administration.adoc
File docs/administration.adoc:

Line 379: WARNING: in planning the new multi-master configuration, keep in mind 
that the number of masters
Nit: other WARNING text begins with a capital letter. Below too.


PS2, Line 382: WARNING: dropping the number of masters below the number of 
masters currently needed for a Raft
 : majority can incur data loss.
Please double check this with Mike.


PS2, Line 392: masters
> nit: master nodes?
I don't really understand why this instruction is worth including. Yes, it 
makes sense to ensure that the masters that you're removing aren't going to 
come back. But I think that's sort of implicit in these instructions, 
especially in the "Remove the directories..." step.

If you look at the other master workflows, none of them call this out 
explicitly, so at the very least, this workflow ought to be consistent with 
them.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4196dbb2f8a185e868a6906c7cf917d79c404c0d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession

Adar Dembo has posted comments on this change.

Change subject: KUDU-2124. Don't hold session lock while initializing a 
TabletCopySession
..


Patch Set 8: Code-Review+1

Mike should review this.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If8bd295a59ade8c89fdf1853dd64c4bceca8da91
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures

Adar Dembo has posted comments on this change.

Change subject: KUDU-2125: Tablet copy client does not retry on failures
..


Patch Set 10: Code-Review+1

Mike should review this.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures

Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-2125: Tablet copy client does not retry on failures
..

KUDU-2125: Tablet copy client does not retry on failures

The tablet copy client would fail the tablet copy after encountering an
RPC error. This commit adds retry logic for tablet copy operations when
encountering SERVER_TOO_BUSY or UNAVAILABLE errors, which are retriable.

Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/tablet_copy_client_session-itest.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_client.h
5 files changed, 133 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/8016/10
-- 
To view, visit http://gerrit.cloudera.org:8080/8016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures

Dan Burkert has posted comments on this change.

Change subject: KUDU-2125: Tablet copy client does not retry on failures
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8016/8/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
File src/kudu/integration-tests/tablet_copy_client_session-itest.cc:

Line 320:   ASSERT_FALSE(HasFailure());
> I see, thanks.  Maybe, remove it then (or the idea is to catch some other e
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] docs: clarify steps for removing master from multi-master deployment

Alexey Serbin has posted comments on this change.

Change subject: docs: clarify steps for removing master from multi-master 
deployment
..


Patch Set 2: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8032/1/docs/administration.adoc
File docs/administration.adoc:

PS1, Line 376:  has
> I think "been" is fine here; I'm using it here as a verb, as in:
SGTM


http://gerrit.cloudera.org:8080/#/c/8032/2/docs/administration.adoc
File docs/administration.adoc:

PS2, Line 392: masters
nit: master nodes?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4196dbb2f8a185e868a6906c7cf917d79c404c0d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures

Alexey Serbin has posted comments on this change.

Change subject: KUDU-2125: Tablet copy client does not retry on failures
..


Patch Set 8: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8016/8/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
File src/kudu/integration-tests/tablet_copy_client_session-itest.cc:

Line 320:   ASSERT_FALSE(HasFailure());
> It still fails even without this statement.
I see, thanks.  Maybe, remove it then (or the idea is to catch some other 
errors)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Update website for 1.5.0 release

Dan Burkert has submitted this change and it was merged.

Change subject: Update website for 1.5.0 release
..


Update website for 1.5.0 release

Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a
Reviewed-on: http://gerrit.cloudera.org:8080/8013
Reviewed-by: Todd Lipcon 
Tested-by: Dan Burkert 
---
M apidocs/allclasses-frame.html
M apidocs/allclasses-noframe.html
M apidocs/constant-values.html
M apidocs/deprecated-list.html
M apidocs/help-doc.html
M apidocs/index-all.html
M apidocs/index.html
M apidocs/org/apache/kudu/ColumnSchema.html
M apidocs/org/apache/kudu/Schema.html
M apidocs/org/apache/kudu/Type.html
D apidocs/org/apache/kudu/annotations/InterfaceAudience.html
D apidocs/org/apache/kudu/annotations/InterfaceStability.html
D apidocs/org/apache/kudu/annotations/class-use/InterfaceAudience.html
D apidocs/org/apache/kudu/annotations/class-use/InterfaceStability.html
D apidocs/org/apache/kudu/annotations/package-frame.html
D apidocs/org/apache/kudu/annotations/package-summary.html
D apidocs/org/apache/kudu/annotations/package-tree.html
D apidocs/org/apache/kudu/annotations/package-use.html
M apidocs/org/apache/kudu/class-use/ColumnSchema.html
M apidocs/org/apache/kudu/class-use/Schema.html
M apidocs/org/apache/kudu/class-use/Type.html
M apidocs/org/apache/kudu/client/AbstractKuduScannerBuilder.html
M apidocs/org/apache/kudu/client/AlterTableOptions.html
M apidocs/org/apache/kudu/client/AlterTableResponse.html
M apidocs/org/apache/kudu/client/AsyncKuduClient.AsyncKuduClientBuilder.html
M apidocs/org/apache/kudu/client/AsyncKuduClient.html
M apidocs/org/apache/kudu/client/AsyncKuduScanner.AsyncKuduScannerBuilder.html
M apidocs/org/apache/kudu/client/AsyncKuduScanner.ReadMode.html
M apidocs/org/apache/kudu/client/AsyncKuduScanner.html
M apidocs/org/apache/kudu/client/AsyncKuduSession.html
M apidocs/org/apache/kudu/client/ColumnRangePredicate.html
M apidocs/org/apache/kudu/client/CreateTableOptions.html
M apidocs/org/apache/kudu/client/Delete.html
M apidocs/org/apache/kudu/client/DeleteTableResponse.html
M apidocs/org/apache/kudu/client/ExternalConsistencyMode.html
M apidocs/org/apache/kudu/client/HasFailedRpcException.html
M apidocs/org/apache/kudu/client/Insert.html
M apidocs/org/apache/kudu/client/IsAlterTableDoneResponse.html
M apidocs/org/apache/kudu/client/KuduClient.KuduClientBuilder.html
M apidocs/org/apache/kudu/client/KuduClient.html
M apidocs/org/apache/kudu/client/KuduException.html
M apidocs/org/apache/kudu/client/KuduPredicate.ComparisonOp.html
M apidocs/org/apache/kudu/client/KuduPredicate.html
M apidocs/org/apache/kudu/client/KuduScanToken.KuduScanTokenBuilder.html
M apidocs/org/apache/kudu/client/KuduScanToken.html
M apidocs/org/apache/kudu/client/KuduScanner.KuduScannerBuilder.html
M apidocs/org/apache/kudu/client/KuduScanner.html
M apidocs/org/apache/kudu/client/KuduSession.html
M apidocs/org/apache/kudu/client/KuduTable.html
M apidocs/org/apache/kudu/client/ListTablesResponse.html
M apidocs/org/apache/kudu/client/ListTabletServersResponse.html
M apidocs/org/apache/kudu/client/LocatedTablet.Replica.html
M apidocs/org/apache/kudu/client/LocatedTablet.html
M apidocs/org/apache/kudu/client/Operation.html
M apidocs/org/apache/kudu/client/OperationResponse.html
M apidocs/org/apache/kudu/client/PartialRow.html
M apidocs/org/apache/kudu/client/PleaseThrottleException.html
M apidocs/org/apache/kudu/client/RangePartitionBound.html
M apidocs/org/apache/kudu/client/ReplicaSelection.html
M apidocs/org/apache/kudu/client/RowError.html
M apidocs/org/apache/kudu/client/RowErrorsAndOverflowStatus.html
M apidocs/org/apache/kudu/client/RowResult.html
M apidocs/org/apache/kudu/client/RowResultIterator.html
M apidocs/org/apache/kudu/client/SessionConfiguration.FlushMode.html
M apidocs/org/apache/kudu/client/SessionConfiguration.html
M apidocs/org/apache/kudu/client/Statistics.Statistic.html
M apidocs/org/apache/kudu/client/Statistics.html
M apidocs/org/apache/kudu/client/Status.html
M apidocs/org/apache/kudu/client/Update.html
M apidocs/org/apache/kudu/client/Upsert.html
M apidocs/org/apache/kudu/client/class-use/AbstractKuduScannerBuilder.html
M apidocs/org/apache/kudu/client/class-use/AlterTableOptions.html
M apidocs/org/apache/kudu/client/class-use/AlterTableResponse.html
M 
apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.AsyncKuduClientBuilder.html
M apidocs/org/apache/kudu/client/class-use/AsyncKuduClient.html
M 
apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.AsyncKuduScannerBuilder.html
M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.ReadMode.html
M apidocs/org/apache/kudu/client/class-use/AsyncKuduScanner.html
M apidocs/org/apache/kudu/client/class-use/AsyncKuduSession.html
M apidocs/org/apache/kudu/client/class-use/ColumnRangePredicate.html
M apidocs/org/apache/kudu/client/class-use/CreateTableOptions.html
M apidocs/org/apache/kudu/client/class-use/Delete.html
M apidocs/org/apache/kudu/client/class-u

[kudu-CR](gh-pages) Update website for 1.5.0 release

Dan Burkert has posted comments on this change.

Change subject: Update website for 1.5.0 release
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Update website for 1.5.0 release

2017-09-12 Thread Todd Lipcon (Code Review)

Todd Lipcon has posted comments on this change.

Change subject: Update website for 1.5.0 release
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Update website for 1.5.0 release

Dan Burkert has posted comments on this change.

Change subject: Update website for 1.5.0 release
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8013/1/releases/index.md
File releases/index.md:

Line 12: * [Kudu 1.5.0](1.5.0/)** was released on September 8, 2017.
> Are you missing a '**' here for proper bold rendering?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8885cd6ecfb904e95370e566a32a579f6c85a4a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures

Dan Burkert has posted comments on this change.

Change subject: KUDU-2125: Tablet copy client does not retry on failures
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8016/8/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
File src/kudu/integration-tests/tablet_copy_client_session-itest.cc:

Line 320:   ASSERT_FALSE(HasFailure());
> I'm curious why it's necessary to add this assert.  What happens if there i
It still fails even without this statement.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2125: Tablet copy client does not retry on failures

Alexey Serbin has posted comments on this change.

Change subject: KUDU-2125: Tablet copy client does not retry on failures
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8016/8/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
File src/kudu/integration-tests/tablet_copy_client_session-itest.cc:

Line 320:   ASSERT_FALSE(HasFailure());
> NO_FATALS, through some unknowable black magic, only triggers when there ar
I'm curious why it's necessary to add this assert.  What happens if there is a 
failure at least in one thread above and this ASSERT_FALSE(HasFailure()) is not 
present?  Does the test pass in that case or there is some other misreporting?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c8454fc600a841bd15306a2b3b06ddf53130be6
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1788. Increase Raft RPC timeout to 30sec to avoid fruitless retries.

2017-09-12 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1788. Increase Raft RPC timeout to 30sec to avoid 
fruitless retries.
..


Patch Set 1:

does this have any nasty interaction with failure detection for other 
situations than the one described in the jira. It seems like it might 
potentially have since the previous timeout was less than 3x heartbeats and the 
new one isn't.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f47dc006dc3dfb1659a224172e1905b6bf3d2a4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Update website for 1.5.0 release