[kudu-CR] docs: design for handling permanent master failures

2016-08-02 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: docs: design for handling permanent master failures
..


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3393/3/docs/design-docs/master-perm-failure-1.0.md
File docs/design-docs/master-perm-failure-1.0.md:

Line 52: 3. Copy the master's entire data/WAL directory from **X** to **Y**.
> hrm, this is odd -- I thought in step 2, X died. how are we going to copy i
To be honest I didn't delve into the various ways in which this condition (that 
X is "dead" but the data is salvageable) could be satisfied. Here are some 
possibilities:
1. X is super old and we'd like to decommission it. It'll be considered "dead" 
after the copy.
2. X has a bad DIMM that causes faulst rarely. Maybe we'll rip out the bad 
DIMM, boot, do the copy, then decommission it.
3. Some other piece of X's hardware is gone, in which case yes, we may move the 
disk.

Do you think these are too contrived? Should I just rewrite this to dispel any 
notion that today's Kudu can recover from some kinds of permanent failure?


Line 114: 2. Find new master machines, creating DNS cnames for all of them. 
Create a DNS
> how will this work in the context of a management tool like CM? wouldn't th
I haven't given much thought to CM since it's out of scope for the Kudu 
_project_, but yeah, we may need that. Is there a similar concept in HDFS?


Line 136: 2. Implement new command line tool to rewrite cmeta files.
> can we combine these two? something that leads you through the process?
I'd rather have both: a command line tool that can perform each (specific) task 
on its own, and a script that ties them together.

Now that I've implemented this, though, it's proving difficult to combine since 
different pieces of work happen on different machines:
1. On each new master, run new "format" command to create FS.
2. On each new master, run kudu-fs_dump "list_uuid" to get the FS's UUID.
3. On the old master, run new "cmeta rewrite" command with the new UUIDs, 
hostports, and existing UUID/hostport.
4. On each new master, run new "tablet copy" command to fetch the master tablet.

I guess it can be done with a shell script that uses ssh to get to each 
machine. That won't work in every environment, though.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f05c319c89cf37e2d71fdc4b7ec951b2932a2b2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] remote bootstrap client: mild API changes

2016-08-02 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: remote_bootstrap_client: mild API changes
..


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/2709/

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

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

[kudu-CR] remote bootstrap client: mild API changes

2016-08-02 Thread Adar Dembo (Code Review) 
Hello Kudu Jenkins,

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

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

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

Change subject: remote_bootstrap_client: mild API changes
..

remote_bootstrap_client: mild API changes

1. I removed the uuid argument from the constructor; it's always the uuid of
   the FsManager.
2. I made the TabletStatusListener argument to FetchAll() optional. I think
   that was the original intent (because the TabletMetadata OUT parameter in
   Start() is optional), but a CHECK_NOTNULL() got added at some point.
3. I removed the peer uuid argument from Start(). It was only used for
   logging, so I added an equivalent field to the response protocol.

Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
---
M src/kudu/integration-tests/remote_bootstrap-itest.cc
M src/kudu/tserver/remote_bootstrap.proto
M src/kudu/tserver/remote_bootstrap_client-test.cc
M src/kudu/tserver/remote_bootstrap_client.cc
M src/kudu/tserver/remote_bootstrap_client.h
M src/kudu/tserver/remote_bootstrap_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
7 files changed, 49 insertions(+), 52 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] c++ client: remove unnecessary code

2016-08-02 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: c++ client: remove unnecessary code
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3809/3/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

Line 63: using master::GetTableSchemaRequestPB;
> Nit: is it needed after the change?
Yeah, these are still needed by the new code.


Line 64: using master::GetTableSchemaResponsePB;
> Ditto.
See above.


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

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

[kudu-CR] [util] fixed build on MacOS X

2016-08-02 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: [util] fixed build on MacOS X
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3836/1/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:

Line 209: dl
> At my laptop the error looks like
It probably means -ldl can resolve to something on macOS, so kudu_test_main can 
link properly. However, Kudu's main CMakeLists.txt file does NOT create dl and 
dl_exported targets on macOS, so when the dl dependency sneaks into 
ADD_EXPORTABLY_LIBRARY(), -ldl_exported cannot resolve.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I894adf2d5f961eea567a32c6bb85af8998ce4adb
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] [util] fixed build on MacOS X

2016-08-02 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: [util] fixed build on MacOS X
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3836/1/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:

Line 209: dl
weird... Adar asked if I needed to put this here, and I figured I didn't 
because we link 'dl' in the target_link_libraries(kudu_test_main...) stuff down 
aroudn line 257. Why does it work there and not here?


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

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

[kudu-CR] [KuduScanBatch::const iterator] a minor clean-up

2016-08-02 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: [KuduScanBatch::const_iterator] a minor clean-up
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/2708/

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

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

[kudu-CR] [KuduScanBatch::const iterator] a minor clean-up

2016-08-02 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: [KuduScanBatch::const_iterator] a minor clean-up
..


Patch Set 1:

(3 comments)

Thank you for review.  Will post an updated version soon.

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

Line 208:   return (idx_ == other.idx_) && (batch_ == other.batch_);
> nit: tabs
oops

In my .vimrc I have

set expandtab
set shiftwidth=2
set tabstop=2

I'm curious how it appeared.


Line 211:   return !this->operator==(other);
> is there a difference between this and the more obvious-looking "!(*this ==
Sure, "!(*this == other)" is much more readable, thanks.


Line 221:   const KuduScanBatch* batch_;
> maybe we should make this const (const KuduScanBatch* const batch_;) to enc
Great point -- will change.


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

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

[kudu-CR] c++ client: remove unnecessary code

2016-08-02 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change.

Change subject: c++ client: remove unnecessary code
..


Patch Set 3: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3809/3/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

Line 63: using master::GetTableSchemaRequestPB;
Nit: is it needed after the change?


Line 64: using master::GetTableSchemaResponsePB;
Ditto.


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

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

[kudu-CR] remote bootstrap client: mild API changes

2016-08-02 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: remote_bootstrap_client: mild API changes
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3811/3//COMMIT_MSG
Commit Message:

Line 15:logging, so I added an equivalent field to the response protocol.
> Can I just drop the peer uuid argument altogether then? It's only used for 
hrm.. ok, now I see your point. guess I'm OK with the slightly-ugly RPC change 
then


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

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

[kudu-CR] [KuduScanBatch::const iterator] a minor clean-up

2016-08-02 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: [KuduScanBatch::const_iterator] a minor clean-up
..


Patch Set 1:

(3 comments)

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

Line 208:   return (idx_ == other.idx_) && (batch_ == other.batch_);
nit: tabs


Line 211:   return !this->operator==(other);
is there a difference between this and the more obvious-looking "!(*this == 
other)"?


Line 221:   const KuduScanBatch* batch_;
maybe we should make this const (const KuduScanBatch* const batch_;) to 
encourage the optimizer to be able to inline out the equality checks?


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

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

[kudu-CR] make election timeout jitter more aggressive

2016-08-02 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: make election timeout jitter more aggressive
..


Patch Set 1:

Thinking about this more, the 20s clamp is probably OK.  It means we could 
theoretically not make progress is RTT are > 20s, but we already can't make 
progress in that situation anyway since the election timeout is 1.5s.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c9dad820c2b7d4bc4b9e791b78222559cdf63c8
Gerrit-PatchSet: 1
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] [KuduScanBatch::const iterator] a minor clean-up

2016-08-02 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: [KuduScanBatch::const_iterator] a minor clean-up
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2706/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3eeba46e5bc45c9dd7b96abad60173381d6c3e39
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [KuduScanBatch::const iterator] a minor clean-up

2016-08-02 Thread Alexey Serbin (Code Review) 
Alexey Serbin has uploaded a new change for review.

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

Change subject: [KuduScanBatch::const_iterator] a minor clean-up
..

[KuduScanBatch::const_iterator] a minor clean-up

More 'standardized' signature for the prefix increment operator.
Added postfix increment operator.  Changed the behavior of the
equality/non-equality operators: along with current row index,
also check whether iterators are built upon the same batch.
Added units tests to cover the modified code.

Change-Id: I3eeba46e5bc45c9dd7b96abad60173381d6c3e39
---
M src/kudu/client/client-test.cc
M src/kudu/client/scan_batch.h
2 files changed, 122 insertions(+), 6 deletions(-)


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

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

[kudu-CR] remote bootstrap client: mild API changes

2016-08-02 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: remote_bootstrap_client: mild API changes
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3811/3//COMMIT_MSG
Commit Message:

Line 15:logging, so I added an equivalent field to the response protocol.
> I'm not super keen on changing the RPC protocol to avoid the extra paramete
Can I just drop the peer uuid argument altogether then? It's only used for 
logging.

If not dropped, a user who wants to remote bootstrap from the command line 
needs to provide it...just so it'll appear in some log messages.


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

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

[kudu-CR] make election timeout jitter more aggressive

2016-08-02 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: make election timeout jitter more aggressive
..


Patch Set 1:

(1 comment)

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

Line 7: make election timeout jitter more aggressive
> I agree with Todd, backoff cap avoids insane exponential backoffs. It seems
I don't think I'm following.  How can you add more jitter without increasing 
the average timeout?  We are, after al,l bounded by a minimum timeout of 0.  
The jitter *must* increase as a function of the number of retries, otherwise 
you risk a situation where the cluster can't make progress due to RTT being 
greater than the jitter.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c9dad820c2b7d4bc4b9e791b78222559cdf63c8
Gerrit-PatchSet: 1
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] c++ client: remove unnecessary code

2016-08-02 Thread Adar Dembo (Code Review) 
Hello Todd Lipcon, Kudu Jenkins,

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

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

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

Change subject: c++ client: remove unnecessary code
..

c++ client: remove unnecessary code

1. GetTableSchema() was implemented using its own RPC instead of a much
   simpler call to SyncLeaderMasterRpc(). An RPC is attractive for
   asynchronous use, but since it's never used that way, let's just switch
   over to SyncLeaderMasterRpc().
2. KuduTable::Open() was issuing both GetTableSchema() and
   GetTableLocations() RPCs. It's not clear why we're doing the latter; the
   response is completely ignored. So let's stop doing that.

My main motivation is to remove fragile error-handling and retry logic which
has been duplicated all over the client.

Change-Id: Idda2cc15bc6224df992bfe2eec287c0222adced0
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client.cc
M src/kudu/client/table-internal.cc
M src/kudu/client/table-internal.h
4 files changed, 34 insertions(+), 304 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idda2cc15bc6224df992bfe2eec287c0222adced0
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-526: use on-disk cmeta when loading existing master state

2016-08-02 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-526: use on-disk cmeta when loading existing master state
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/2704/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b4c6d8b6adf696973445a6f9d1314ba9de27e70
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] remote bootstrap client: mild API changes

2016-08-02 Thread Adar Dembo (Code Review) 
Hello Kudu Jenkins,

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

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

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

Change subject: remote_bootstrap_client: mild API changes
..

remote_bootstrap_client: mild API changes

1. I removed the uuid argument from the constructor; it's always the uuid of
   the FsManager.
2. I made the TabletStatusListener argument to FetchAll() optional. I think
   that was the original intent (because the TabletMetadata OUT parameter in
   Start() is optional), but a CHECK_NOTNULL() got added at some point.
3. I removed the peer uuid argument from Start(). It was only used for
   logging, so I added an equivalent field to the response protocol.

Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
---
M src/kudu/integration-tests/remote_bootstrap-itest.cc
M src/kudu/tserver/remote_bootstrap.proto
M src/kudu/tserver/remote_bootstrap_client-test.cc
M src/kudu/tserver/remote_bootstrap_client.cc
M src/kudu/tserver/remote_bootstrap_client.h
M src/kudu/tserver/remote_bootstrap_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
7 files changed, 48 insertions(+), 52 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] master: additional leader lock assertions in catalog manager

2016-08-02 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: master: additional leader lock assertions in catalog manager
..


Patch Set 5:

Build Started http://104.196.14.100/job/kudu-gerrit/2705/

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

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

[kudu-CR] c++ client: remove unnecessary code

2016-08-02 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: c++ client: remove unnecessary code
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/2703/

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

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

[kudu-CR] master: additional leader lock assertions in catalog manager

2016-08-02 Thread Adar Dembo (Code Review) 
Hello Kudu Jenkins,

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

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

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

Change subject: master: additional leader lock assertions in catalog manager
..

master: additional leader lock assertions in catalog manager

I went through the catalog manager entry points and added leader lock
assertions where necessary, updating tests as needed.

I also snuck in a couple cluster fixes:
1. MiniCluster::leader_mini_master() was unsafe because it didn't pass the
   (now held) leader lock back to the caller. It's only used in a few places
   though, so I removed it outright rather than fix it.
2. WaitForTabletServerCount() has been updated for both kinds of clusters.
   The new version waits for the correct count on every master, a necessary
   change now that tservers heartbeat to every master. Without this, we may
   stop waiting when the master that has seen all tservers was a follower
   and fail a subsequent CreateTable. The new version also ignore masters
   that have been shut down. This isn't essential, but good future-proofing.

Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
---
M src/kudu/client/client-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/master_replication-itest.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-path-handlers.cc
10 files changed, 146 insertions(+), 105 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] remote bootstrap client: mild API changes

2016-08-02 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: remote_bootstrap_client: mild API changes
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/2702/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Fix block manager-test running in some builds

2016-08-02 Thread Adar Dembo (Code Review) 
Adar Dembo has submitted this change and it was merged.

Change subject: Fix block_manager-test running in some builds
..


Fix block_manager-test running in some builds

Even though the patch for KUDU-1538 passed pre-commit, it failed in the RELEASE
builds post-commit.

The issue appears to be that my trickery with weak symbols only worked in
some cases, depending on the link order, for dynamically linked builds,
since weak symbols are actually only relevant for statically linked ones.
Apparently in the precommit build, the link order was such that the gtest
detection did not work on block_manager-test, and thus things were fine.
But, in the release static build post-commit, the gtest detection worked,
and it broke the test in question.

This patch changes the gtest detection to use dlsym() instead of a weak symbol.
It appears to work more reliably in all types of builds. This also fixes the
test to properly manipulate the block ID sequence in the test that was failing.

Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Reviewed-on: http://gerrit.cloudera.org:8080/3733
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M CMakeLists.txt
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
A src/kudu/util/test_util_prod.cc
A src/kudu/util/test_util_prod.h
7 files changed, 81 insertions(+), 8 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Fix block manager-test running in some builds

2016-08-02 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: Fix block_manager-test running in some builds
..


Patch Set 4: Code-Review+2

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

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

[kudu-CR] make election timeout jitter more aggressive

2016-08-02 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change.

Change subject: make election timeout jitter more aggressive
..


Patch Set 1:

(1 comment)

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

Line 7: make election timeout jitter more aggressive
> yea, but the jitter is only aggressive due to the backoff being more aggres
I agree with Todd, backoff cap avoids insane exponential backoffs. It seems 
like the jitter is what we are really worried about here. And TBH I'm not 
convinced this is the problem. Although I'd support wider jitter variance.

On a sort of side note, I tried to add a generic exponential backoff helper a 
long time ago in https://gerrit.cloudera.org/#/c/979/ ... maybe we should 
partially resurrect that patch and work on ensuring that we have a single 
exponential backoff function that is parameterized and flexible enough to 
handle all situations.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c9dad820c2b7d4bc4b9e791b78222559cdf63c8
Gerrit-PatchSet: 1
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] make election timeout jitter more aggressive

2016-08-02 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: make election timeout jitter more aggressive
..


Patch Set 1:

(1 comment)

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

Line 7: make election timeout jitter more aggressive
> The lower bound timeout isn't changed, only the upper bound.  So the range 
yea, but the jitter is only aggressive due to the backoff being more aggressive


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

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

[kudu-CR] c++ client: remove unnecessary code

2016-08-02 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: c++ client: remove unnecessary code
..


Patch Set 2: Code-Review+1

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

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

[kudu-CR] c++ client: remove unnecessary code

2016-08-02 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: c++ client: remove unnecessary code
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3809/2/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

Line 556:   gscoped_ptr new_schema(new Schema());
can this use unique_ptr?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idda2cc15bc6224df992bfe2eec287c0222adced0
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] make election timeout jitter more aggressive

2016-08-02 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: make election timeout jitter more aggressive
..


Patch Set 1:

(1 comment)

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

Line 7: make election timeout jitter more aggressive
> isn't it making the backoff more aggressive rather than making the jitter m
The lower bound timeout isn't changed, only the upper bound.  So the range of 
backoff times is greatly increased.  For instance, the previous algorithm had 
spreads of (.15s, 0.315s, 0.4965s, 0.696s) after 0, 1, 2, 3 failed elections, 
respectively.  The new spreads are (0.75s, 1.875s, 3.56s, 6.09s).  The actual 
timeout is the base (1.5s) plus a random value between 0 and the spread.


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

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

[kudu-CR] Fix ksck-test reliance on unordered map iteration order

2016-08-02 Thread Will Berkeley (Code Review) 
Will Berkeley has abandoned this change.

Change subject: Fix ksck-test reliance on unordered map iteration order
..


Abandoned

whoops...already a review out for this

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: If9fea40bd8f6c82d1a9147d95c18c8001713a21c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Fix ksck-test reliance on unordered map iteration order

2016-08-02 Thread Todd Lipcon (Code Review) 
Todd Lipcon has posted comments on this change.

Change subject: Fix ksck-test reliance on unordered map iteration order
..


Patch Set 1:

Hey Will. I actually put a patch up already for this here 
https://gerrit.cloudera.org/#/c/3734/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If9fea40bd8f6c82d1a9147d95c18c8001713a21c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No