date:20160714

Kudu Jenkins has posted comments on this change.

Change subject: doxygen for C++ client API
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] [java client] Integrate with the replay cache

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: [java client] Integrate with the replay cache
..


Patch Set 4:

(4 comments)

looks mostly good, does this still have the side effect of making us not ignore 
duplicated rows where we did before?

http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java
File java/kudu-client/src/main/java/org/kududb/client/KuduRpc.java:

PS4, Line 107: long
docs


http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/Operation.java
File java/kudu-client/src/main/java/org/kududb/client/Operation.java:

Line 163:   boolean isRetryableRpc() {
docs


http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java
File java/kudu-client/src/main/java/org/kududb/client/RequestTracker.java:

Line 41:   public RequestTracker(String clientId) {
does the request tracker need to have the client id?


http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
File java/kudu-client/src/main/java/org/kududb/client/TabletClient.java:

PS4, Line 254: 
requestIdBuilder.setFirstIncompleteSeqNo(requestTracker.firstIncomplete());
> In the previous patch I wasn't sending this if the value would have been NO
thanks


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Add integration tests for replay cache with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Add integration tests for replay cache with writes
..


Add integration tests for replay cache with writes

This adds a couple of new integration tests for replay cache with
writes. Both tests start multiple threads writing, independently, to
tablet servers simulaneously. The tests leverage the fact that followers
are also able to answer requests, once they are cached, and stores all
responses, which are compared at the end of the test.

Some of the requests (1/3) are "empty" writes, so that we stress the
serialization point in transaction_driver.cc without relying on row
lock serialization.

This adds two different tests, one that stresses a lot of elections
and one that crashes nodes. This is inline with other tests we already
had in raft_consensus-itest.

This also adds a new fault injection point right after the leader sends
a request. We currently have one right _before_ the leader sends
a request, but having one for after the request is sent encourages
stressing the path where a newly elected leader as both incoming
client request and ongoing replica transactions, which can possibly
race with each other if they correspond to the same write.

Finally this changes attempt_no in RequestIdPB to be an int64 instead
of just an int. While an int is more than enough in normal operation,
the new test generates many more attempts and we need a bigger number
to make sure all attempt numbers are unique.

I looped this about 1000 times, without related failures.

Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Reviewed-on: http://gerrit.cloudera.org:8080/3519
Reviewed-by: Mike Percy 
Tested-by: Kudu Jenkins
---
M src/kudu/consensus/consensus_peers.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/rpc/rpc_header.proto
6 files changed, 326 insertions(+), 33 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 35
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] doxygen for C++ client API

Kudu Jenkins has posted comments on this change.

Change subject: doxygen for C++ client API
..


Patch Set 7:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add integration tests for replay cache with writes

Mike Percy has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 34: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 34
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add integration tests for replay cache with writes

Kudu Jenkins has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 34:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 34
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add integration tests for replay cache with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

Hello Kudu Jenkins,

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

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

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

Change subject: Add integration tests for replay cache with writes
..

Add integration tests for replay cache with writes

This adds a couple of new integration tests for replay cache with
writes. Both tests start multiple threads writing, independently, to
tablet servers simulaneously. The tests leverage the fact that followers
are also able to answer requests, once they are cached, and stores all
responses, which are compared at the end of the test.

Some of the requests (1/3) are "empty" writes, so that we stress the
serialization point in transaction_driver.cc without relying on row
lock serialization.

This adds two different tests, one that stresses a lot of elections
and one that crashes nodes. This is inline with other tests we already
had in raft_consensus-itest.

This also adds a new fault injection point right after the leader sends
a request. We currently have one right _before_ the leader sends
a request, but having one for after the request is sent encourages
stressing the path where a newly elected leader as both incoming
client request and ongoing replica transactions, which can possibly
race with each other if they correspond to the same write.

Finally this changes attempt_no in RequestIdPB to be an int64 instead
of just an int. While an int is more than enough in normal operation,
the new test generates many more attempts and we need a bigger number
to make sure all attempt numbers are unique.

I looped this about 1000 times, without related failures.

Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
---
M src/kudu/consensus/consensus_peers.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/rpc/rpc_header.proto
6 files changed, 326 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 34
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] [c++-client]: cache non-covering ranges in meta cache

Hello Kudu Jenkins,

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

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

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

Change subject: [c++-client]: cache non-covering ranges in meta cache
..

[c++-client]: cache non-covering ranges in meta cache

This commit introduces a few features to the meta cache, all with the aim of
making it compatible with the upcoming add/drop range partitions feature.

1) Non-covered range partitions are now cached in the meta cache.  This is
achieved by storing MetaCacheEntry objects in the meta cache's partition-key
index instead of RemoteTablets.  The MetaCacheEntry holds either a RemoteTablet,
in which case it represents a covered partition range, or it represents a
non-covered partition range.

2) Entries are now removed from the meta cache's partition-key index when it can
be determined that the entries are no longer valid from the results of a
GetTableLocations RPC.

3) A basic TTL has been added to the GetTableLocationsResponsePB so that the
client can properly refresh the meta cache when necessary. The TTL is
configurable by the master, and defaults to one hour.

Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/client/schema.h
M src/kudu/client/table-internal.cc
M src/kudu/gutil/map-util.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/ksck_remote.cc
10 files changed, 438 insertions(+), 139 deletions(-)


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

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

[kudu-CR] [c++-client]: cache non-covering ranges in meta cache

Kudu Jenkins has posted comments on this change.

Change subject: [c++-client]: cache non-covering ranges in meta cache
..


Patch Set 8:

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

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

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

[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package

Mike Percy has posted comments on this change.

Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode 
package
..


Patch Set 3: Code-Review+2

Overriding flakiness due to isolate race condition

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] client.h: doxygen comments for C++ API

Mike Percy has posted comments on this change.

Change subject: client.h: doxygen comments for C++ API
..


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3619/6/CMakeLists.txt
File CMakeLists.txt:

Line 1000: # "make doxydocs" target
Sorry to be a buzz kill, but can we just make this target "doxygen" instead of 
"doxydocs"?


Line 1015: COMMAND make all install DESTDIR=${DOXY_CLIENT_DESTDIR}
Can this be made to work with Ninja was well? I think you just need to replace 
make with ${CMAKE_MAKE_PROGRAM}


http://gerrit.cloudera.org:8080/#/c/3619/6/docs/.gitignore
File docs/.gitignore:

Line 19
nit: spurious change


http://gerrit.cloudera.org:8080/#/c/3619/6/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 28: #include 
This change isn't required for this patch, is it?


http://gerrit.cloudera.org:8080/#/c/3619/6/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

Line 262: if ! `which -s doxygen` && [ ! -d $DOXYGEN_DIR ]; then
The -s command-line flag is not available on GNU systems. You can do:

if ! which doxygen >/dev/null && [ ! -d "$DOXYGEN_DIR" ]; then


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Memory tracking for result tracker

Adar Dembo has posted comments on this change.

Change subject: Memory tracking for result tracker
..


Patch Set 9:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3627/9/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 75: mem_tracker_->Consume(sizeof(ClientState));
If we want to make ClientState memory tracking accurate, we need to account for 
two additional things:

1. Malloc "slop". sizeof(ClientState) may not account for the actual size of 
the memory allocation used in "new ClientState()". For example, if the size of 
the struct is 40 bytes, the allocation may be the next power of 2 (i.e. 64 
bytes). We deal with this elsewhere by using kudu_malloc_usable_size(ptr) as 
the "size of object"; sometimes this is internalized inside a 
memory_footprint() method.

2. The size of any nested heap pointers. ClientState contains a map, and that 
map contains nested pointers whose memory consumption isn't being accounted 
for. Look at how Schema::name_to_index_ is managed to see how you might do that.


Line 85: mem_tracker_->Consume(sizeof(CompletionRecord));
Same issues with CompletionRecord and its nested vector of OngoingRpcs (you can 
use kudu_malloc_usable_size(vector.data()), though you  need to check that 
vector.capacity() >0 first).

I think you're already taking care of the nested protobuf elsewhere.


Line 234:   mem_tracker_->Consume(response->ByteSize());
Should use SpaceUsed(), I think.


Line 255:   mem_tracker_->Release(sizeof(OnGoingRpcInfo));
We've found that Release() in a loop is generally less efficient than adding up 
all the memory consumed in the loop and making one call to Release() at the 
end. That change reduced block manager CPU consumption pretty dramatically on 
startup.


http://gerrit.cloudera.org:8080/#/c/3627/9/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:

PS9, Line 89:   if (id != 0) {
: StrAppend(_str, " ", id);
:   }
Don't need this; each of these memtrackers is already "disambiguated" by virtue 
of having a unique parent.

That said, why bother creating a memtracker for the ResultTracker instead of 
just reusing the server memtracker? IIRC we only create separate memtrackers 
for objects that come and go (e.g. tablet, memrowset, deltamemstore).


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

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

[kudu-CR] client.h: doxygen comments for C++ API

Kudu Jenkins has posted comments on this change.

Change subject: client.h: doxygen comments for C++ API
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7d42fb1c90b83074e357dcecf42489ed9fc4f02
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add integration tests for replay cache with writes

Kudu Jenkins has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 33:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 33
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP: Add garbage collection to ResultTracker

Kudu Jenkins has posted comments on this change.

Change subject: WIP: Add garbage collection to ResultTracker
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Memory tracking for result tracker

Kudu Jenkins has posted comments on this change.

Change subject: Memory tracking for result tracker
..


Patch Set 9:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add integration tests for replay cache with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

Hello Kudu Jenkins,

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

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

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

Change subject: Add integration tests for replay cache with writes
..

Add integration tests for replay cache with writes

This adds a couple of new integration tests for replay cache with
writes. Both tests start multiple threads writing, independently, to
tablet servers simulaneously. The tests leverage the fact that followers
are also able to answer requests, once they are cached, and stores all
responses, which are compared at the end of the test.

Some of the requests (1/3) are "empty" writes, so that we stress the
serialization point in transaction_driver.cc without relying on row
lock serialization.

This adds two different tests, one that stresses a lot of elections
and one that crashes nodes. This is inline with other tests we already
had in raft_consensus-itest.

This also adds a new fault injection point right after the leader sends
a request. We currently have one right _before_ the leader sends
a request, but having one for after the request is sent encourages
stressing the path where a newly elected leader as both incoming
client request and ongoing replica transactions, which can possibly
race with each other if they correspond to the same write.

Finally this changes attempt_no in RequestIdPB to be an int64 instead
of just an int. While an int is more than enough in normal operation,
the new test generates many more attempts and we need a bigger number
to make sure all attempt numbers are unique.

I looped this about 1000 times, without related failures.

Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
---
M src/kudu/consensus/consensus_peers.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/rpc/rpc_header.proto
6 files changed, 325 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 33
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Integrate the result tracker with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Integrate the result tracker with writes
..


Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes and because testing it specifically is involved, this patch
doesn't include additional tests, these will be pushed as a follow up.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Reviewed-on: http://gerrit.cloudera.org:8080/3449
Reviewed-by: David Ribeiro Alves 
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Kudu Jenkins
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 399 insertions(+), 41 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 29
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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

[kudu-CR] Add a way to include request ids in log-dump

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Add a way to include request ids in log-dump
..


Patch Set 8: Verified+1

unrelated java flakiness.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add integration tests for replay cache with writes

Mike Percy has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 32:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3519/32/src/kudu/integration-tests/ts_itest-base.h
File src/kudu/integration-tests/ts_itest-base.h:

Line 101: // TODO remove this once we have ResultTracker GC
> There's no JIRA, the GC patch is up for review. Will remove these comments 
ok


Line 493:   void AssertNoTabletServersCrashed() {
> This is just a move from the child class. the raft itest uses this. Happy t
ok


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

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

[kudu-CR] Move the maintenance manager to util

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Move the maintenance manager to util
..


Move the maintenance manager to util

This moves the maintenance manager to util, along with its required protobufs.
This move is required as we'll need a server-level MaintenanceMananager to
run garbage collection on the ResultTracker.

Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0
Reviewed-on: http://gerrit.cloudera.org:8080/3656
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/master/master.cc
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.proto
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer_mm_ops.cc
M src/kudu/tablet/tablet_peer_mm_ops.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/util/CMakeLists.txt
R src/kudu/util/maintenance_manager-test.cc
R src/kudu/util/maintenance_manager.cc
R src/kudu/util/maintenance_manager.h
A src/kudu/util/maintenance_manager.proto
18 files changed, 85 insertions(+), 59 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Move the maintenance manager to util

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3656/3/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

Line 243:   void GetMaintenanceManagerStatusDump(MaintenanceManagerStatusPB* 
out_pb);
> Since this is a pointer, you don't even need to include maintenance_manager
But then I'll have to fwd declare it in a bunch of places. Since this is only a 
code move mind if I drop this?


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

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

[kudu-CR] rw mutex: prevent recursive use

Hello Dan Burkert,

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

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

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

Change subject: rw_mutex: prevent recursive use
..

rw_mutex: prevent recursive use

Todd provided an example[1] of deadlocked rwlocks due to a fairness policy.
In the example, T1 (holding the lock for reading) join()ed on T2 (trying to
acquire the lock for reading) all while T3 was trying to acquire the lock
for writing. The lock's fairness policy prevented T2 from acquiring the read
lock thus deadlocking all three threads. The takeaway is to be careful when
calling join() while holding locks.

Beyond that, deadlocks can arise if rwlocks are taken recursively. That's
not a feature we need in our rwlocks, so I tried to disable it at the
pthread level. Unfortunately, the best I can do is disable recursive write
lock acquisition; read locks are apparently always recursive (see "man
pthread_rwlockattr_setkind_np"). So instead, I built recursive checking into
the RWMutex itself. It's quite slow so it's only present in debug builds.

Note that pthread rwlocks do have some built-in deadlock detection (i.e.
lock calls may return EDEADLK), but it doesn't appear to be comprehensive.

1.
https://issues.apache.org/jira/browse/HDFS-2223?focusedCommentId=13097647=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13097647

Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db
---
M src/kudu/util/CMakeLists.txt
A src/kudu/util/rw_mutex-test.cc
M src/kudu/util/rw_mutex.cc
M src/kudu/util/rw_mutex.h
4 files changed, 314 insertions(+), 4 deletions(-)

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] rw mutex: prevent recursive use

Kudu Jenkins has posted comments on this change.

Change subject: rw_mutex: prevent recursive use
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db
Gerrit-PatchSet: 6
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-1530: Update docs about OS X build dependency on Xcode package

Adar Dembo has posted comments on this change.

Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode 
package
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode 
package
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package

2016-07-14 Thread Dinesh Bhat (Code Review)

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode 
package
..

KUDU-1530: Update docs about OS X build dependency on Xcode package

Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9
---
M docs/installation.adoc
1 file changed, 3 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: Add garbage collection to ResultTracker

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: WIP: Add garbage collection to ResultTracker
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3628/5/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

PS5, Line 91: // If the arriving request is older than our per-client GC 
watermark, report its
:   // staleness to the client.
> Yeah that's what I meant. So is this coming back as a row error or an RPC e
coming back as an RPC error (we don't cache results for individual rows, only 
for whole batches)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Move the maintenance manager to util

Adar Dembo has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3656/3/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

Line 243:   void GetMaintenanceManagerStatusDump(MaintenanceManagerStatusPB* 
out_pb);
Since this is a pointer, you don't even need to include 
maintenance_manager.pb.h; you could forward declare this instead.


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

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

[kudu-CR] [c++-client]: cache non-covering ranges in meta cache

Dan Burkert has posted comments on this change.

Change subject: [c++-client]: cache non-covering ranges in meta cache
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

Line 1238:   // Force a lookup and ensure the entry is refreshed.
> The TTL is 25ms, so it's conceivable that on a bogged down test environment
still working on this...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
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] [c++-client]: cache non-covering ranges in meta cache

Dan Burkert has posted comments on this change.

Change subject: [c++-client]: cache non-covering ranges in meta cache
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:

Line 885:   *cache_entry = *DCHECK_NOTNULL(FindFloorOrNull(tablets_by_key, 
rpc.partition_key()));
> There is no FindFloorOrDie.  I've updated to be a normal CHECK.  I original
Woops, I did actually implement FindFloorOrDie and switch this over to it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
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] [c++-client]: cache non-covering ranges in meta cache

Dan Burkert has posted comments on this change.

Change subject: [c++-client]: cache non-covering ranges in meta cache
..


Patch Set 7:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

PS5, Line 1223:   // Clear the cache.
  :   meta_cache->ClearCacheForTesting();
  :   internal::MetaCacheEntry entry;
  :   
ASSERT_FALSE(meta_cache->LookupTabletByKeyFastPath(client_table_.get(), "", 
));
  : 
> You can instantiate a FlagSaver instead.
Done


http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 300:   FRIEND_TEST(ClientTest, TestScanTimeout);
> Nit: sort alphabetically.
Done


http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:

PS5, Line 753:   if (new_status.IsServiceUnavailable()) {
 : // One or more of the tablets is not running; retry after a 
backoff period.
 : mutable_retrier()->DelayedRetry(this, new_status);
 : ignore_result(delete_me.release());
 : return;
 :   }
> To be clear, this has nothing to do with non-covering range partition suppo
Yes, that's correct.  This manifested in some of the more aggressive add/drop 
partitions tests that come in the next patch, but I thought it was more 
appropriate to fix in this patch.


Line 790:   const auto& tablet_locations = rpc.resp().tablet_locations();
> rpc.resp().tablet_locations() is called four times, maybe pull it out into 
Done


PS5, Line 817: lthough the existence of A as an initial
> Can you pull this (and end) into local variables?
Done


PS5, Line 848:  entry(expiration_time, last_upper_bound, tablet_lower_bound);
> This assumes that a tablet can't change in partition size (i.e. that should
We discussed this off-line and decided that the CHECKs are fine for now.  A 
malformed master response shouldn't result in memory corruption, and having a 
CHECK is good from a debugging standpoint.


PS5, Line 865:   scoped_refptr remote = 
FindPtrOrNull(tablets_by_id_, tablet_id);
 :   if (remote.get() != nullptr) {
 : // Partition should not have changed.
 : DCHECK_EQ(tablet_lower_bound, 
remote->partition().partition_key_start());
 : DCHECK_EQ(tablet_upper_bound, 
remote->partition().partition_key_end());
> Maybe encapsulate this (and the non-covered range variant) in a helper meth
I'm a little hesitant to do that because the cases are so short (just a few 
lines), every one is subtly different, and there is locking going on.


PS5, Line 874: // Update the entry TTL.
 : auto& entry = FindOrDie(tablets_by_k
> What if the total number of tablets is a multiple of MAX_RETURNED_TABLE_LOC
correct, that's exactly how it should work (we can't infer the presence of a 
non-covered range in that case).  It will be discovered if a lookup is 
requested for that range.


Line 885:  
tablets_by_key.lower_bound(tablet_upper_bound));
> Why FindFloorOrNull if you DCHECK_NOTNULL() right after? Seems unsafe.
There is no FindFloorOrDie.  I've updated to be a normal CHECK.  I originally 
had it as a DCHECK since it should only fire if there are bugs in the code, but 
I doubt in makes much of a performance difference.


http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/client/table-internal.cc
File src/kudu/client/table-internal.cc:

PS5, Line 133: if (s.ok()) {
 :   break;
 : } else {
 :   LOG(WARNING) << "Error getting table locations: " << 
s.ToString() << ", retrying.";
 : }
> Likewise, this isn't logically part of this change, right?
Right, same scenario.


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

Line 150: DEFINE_int32(table_locations_ttl_ms, 60 * 60 * 1000, // 1 hour
> How about expressing this as:
Done


http://gerrit.cloudera.org:8080/#/c/3581/5/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

Line 283:   SleepFor(MonoDelta::FromMilliseconds(100 * retries));
> That seems like a rather high sleep value. Does it cause the ksck tests to 
Done


Line 320:   new KsckTabletReplica(replica.ts_info().permanent_uuid(), 
is_leader, is_follower)));
> Nit: wasn't the old indentation correct?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Adar Dembo has posted comments on this change.

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..


Patch Set 4:

(37 comments)

http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/client/client.cc
File src/kudu/client/client.cc:

PS4, Line 892: 
RETURN_NOT_OK(data_->client_->data_->WaitForAlterTableToFinish(
 : data_->client_, alter_name, deadline));
If this fails, don't we want to clear the meta cache anyway? Maybe ClearCache() 
should be set up as a ScopedCleanup thing right after L888-889.


Line 908: // It is sufficient to clear that cache even if the table 
alteration is not
Nit: "the cache".

Also, not sure about "sufficient"; it implies that there was something more we 
could do but have chosen not to. Maybe you meant "necessary"?


http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/client/client.h
File src/kudu/client/client.h:

PS4, Line 537: unless the existing range partitions are dropped first
What happens if a single KuduTableAlterer has DropRange and AddRange on the 
same range? Is that legal, provided they're in that order?


Line 541:   // this method returns, however other existing clients may have to 
wait for a
Don't you mean "when Alter() returns success"?


http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:

Line 19: 
What were the include and using changes for?


http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/client/table_alterer-internal.h
File src/kudu/client/table_alterer-internal.h:

Line 57:   struct PartitioningStep {
I think the code would be net less complex if PartitionStep and Step were 
combined. The overhead of two unique_ptrs per Step for non-partitioning steps 
is minimal, and it'll simplify ToRequest() somewhat.


http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/integration-tests/alter_table-randomized-test.cc
File src/kudu/integration-tests/alter_table-randomized-test.cc:

Line 363: LOG(INFO) << "Adding column " << name;
Were these LOG statements just for debugging or do you want to keep them?


Line 459:   scanner.SetTimeoutMillis(6);
What's the significance of this value?


Line 510:   vector master_addrs_;
Unused?


PS4, Line 547: } else if (r < 850 && t.num_range_partitions() < 
kMaxRangePartitions) {
 :   t.AddRangePartition();
 : } else if (r < 900) {
 :   t.DropRangePartition();
It would be cool if this also tested "compound" operations, i.e. an 
AlterTable() that adds a partition, drops another, and adds a column too.


http://gerrit.cloudera.org:8080/#/c/3648/4/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

Line 27: #include "kudu/client/client.h"
Nit: the previous sort order was more correct.


Line 36: #include "kudu/master/master-test-util.h"
Nit: likewise, this was more correct before.


Line 463:   session->SetTimeoutMillis(15 * 1000);
Why this value?


Line 468: gscoped_ptr insert(table->NewInsert());
Use unique_ptr here?


PS4, Line 470: if (table->schema().num_columns() > 1) {
 :   RETURN_NOT_OK(insert->mutable_row()->SetInt32(1, i));
 : }
The assumption being that every column is going to be an Int32?


Line 550: int AlterTableTest::CountRows(const string& table_name) {
Can you reuse CountTableRows() from client-test-util.h? There may be some other 
useful goodies there.


Line 1064: TEST_F(AlterTableTest, TestAlterRangePartitioning) {
How about a test case for one AlterTable() adding and dropping the same 
partition?

Also, what about negative test cases? Like dropping partitions that don't 
exist, adding partitions that overlap with existing ones, adding the same 
partition twice, etc.


Line 1065:   gscoped_ptr table_alterer;
unique_ptr?


PS4, Line 1091:   table_alterer->wait(true);
  :   
ASSERT_OK(table_alterer->AddRangePartition(add_lower.release(), 
add_upper.release())->Alter());
Combine?


PS4, Line 1097:   ASSERT_OK(InsertRowsSequential(table_name, 0, 100));
  :   ASSERT_EQ(100, CountRows(table_name));
This is to test that IsAlterTableInProgress()->false actually means we can 
insert/scan?


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

PS4, Line 1215:   Schema schema;
  :   
RETURN_NOT_OK(SchemaFromPB(table->metadata().state().pb.schema(), ));
  :   PartitionSchema partition_schema;
  :   
RETURN_NOT_OK(PartitionSchema::FromPB(table->metadata().state().pb.partition_schema(),
  : schema, 
_schema));
This isn't safe; you should be getting the schema() through the COWLocked 
object.


PS4, Line 1223: std::lock_guard l(table->lock_);
  : tablets = table->tablet_map_;
This is why you needed to

[kudu-CR] [c++-client]: cache non-covering ranges in meta cache

Hello Kudu Jenkins,

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

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

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

Change subject: [c++-client]: cache non-covering ranges in meta cache
..

[c++-client]: cache non-covering ranges in meta cache

This commit introduces a few features to the meta cache, all with the aim of
making it compatible with the upcoming add/drop range partitions feature.

1) Non-covered range partitions are now cached in the meta cache.  This is
achieved by storing MetaCacheEntry objects in the meta cache's partition-key
index instead of RemoteTablets.  The MetaCacheEntry holds either a RemoteTablet,
in which case it represents a covered partition range, or it represents a
non-covered partition range.

2) Entries are now removed from the meta cache's partition-key index when it can
be determined that the entries are no longer valid from the results of a
GetTableLocations RPC.

3) A basic TTL has been added to the GetTableLocationsResponsePB so that the
client can properly refresh the meta cache when necessary. The TTL is
configurable by the master, and defaults to one hour.

Change-Id: I05bcb3fe05d51d7c455e1d68bd2baa6f3c2b9d21
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/client/schema.h
M src/kudu/client/table-internal.cc
M src/kudu/gutil/map-util.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master.proto
M src/kudu/tools/ksck_remote.cc
10 files changed, 421 insertions(+), 122 deletions(-)


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

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

[kudu-CR] KUDU-1311 [master] support adding and dropping range partitions

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1311 [master] support adding and dropping range partitions
..

KUDU-1311 [master] support adding and dropping range partitions

Change-Id: I42437f365397baf9d4b39b5b17a1587fae70c4be
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.cc
M src/kudu/client/meta_cache.h
M src/kudu/client/table_alterer-internal.cc
M src/kudu/client/table_alterer-internal.h
M src/kudu/common/partition.cc
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/tools/ksck.cc
17 files changed, 713 insertions(+), 119 deletions(-)


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

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

[kudu-CR] [c++-client]: cache non-covering ranges in meta cache

Kudu Jenkins has posted comments on this change.

Change subject: [c++-client]: cache non-covering ranges in meta cache
..


Patch Set 7:

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

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

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

[kudu-CR] Move the maintenance manager to util

Jean-Daniel Cryans has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 3: Code-Review+2

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

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

[kudu-CR] Integrate the result tracker with writes

Jean-Daniel Cryans has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 28: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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] WIP: Add garbage collection to ResultTracker

Jean-Daniel Cryans has posted comments on this change.

Change subject: WIP: Add garbage collection to ResultTracker
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3628/5/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

PS5, Line 45:  
> not sure that's a rule. but Done
Looking almost anywhere else it's done with trailing whitespaces.


PS5, Line 91: // If the arriving request is older than our per-client GC 
watermark, report its
:   // staleness to the client.
> you mean fatal in the sense that the client can't recover? yeah (not in the
Yeah that's what I meant. So is this coming back as a row error or an RPC error?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] WIP: Add garbage collection to ResultTracker

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: WIP: Add garbage collection to ResultTracker
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3628/5/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

PS5, Line 45:  
> nit: shouldn't whitespaces trail for multi-line strings?
not sure that's a rule. but Done


PS5, Line 91: // If the arriving request is older than our per-client GC 
watermark, report its
:   // staleness to the client.
> This is fatal, right? There's nothing the client can do?
you mean fatal in the sense that the client can't recover? yeah (not in the 
"crash the server" sense though)


http://gerrit.cloudera.org:8080/#/c/3628/5/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

PS5, Line 227: ClientSttate
> nit
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Integrate the result tracker with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 28: Code-Review+2

rebased this due to a problem with the mm patch. keeping the +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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] Add a way to include request ids in log-dump

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Add a way to include request ids in log-dump
..


Patch Set 8: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add a way to include request ids in log-dump

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Add a way to include request ids in log-dump
..


Patch Set 8:

tests failed due to a problem with the mm patch. just a rebase, keeping the +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Add integration tests for replay cache with writes

Kudu Jenkins has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 32:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 32
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a way to include request ids in log-dump

Kudu Jenkins has posted comments on this change.

Change subject: Add a way to include request ids in log-dump
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 28:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 28
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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] KUDU-1530: Update docs about OS X build dependency on Xcode package

2016-07-14 Thread Dinesh Bhat (Code Review)

Dinesh Bhat has posted comments on this change.

Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode 
package
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3653/2/docs/installation.adoc
File docs/installation.adoc:

Line 474: The Xcode package is necessary for compiling Kudu. Some of the 
instructions
> Along the same lines, will it be obvious to a macOS user how to get the Xco
Yeah I like that idea, perhaps we can simply point 'Xcode' to point to 
https://developer.apple.com/xcode/ ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Move the maintenance manager to util

Kudu Jenkins has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 3:

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

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

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

[kudu-CR] Move the maintenance manager to util

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3656/2/src/kudu/tablet/CMakeLists.txt
File src/kudu/tablet/CMakeLists.txt:

PS2, Line 100: ADD_KUDU_TEST(maintenance_manager-test)
> Forgot to remove this line?
too... many... patches... Done


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

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

[kudu-CR] Move the maintenance manager to util

Jean-Daniel Cryans has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3656/2/src/kudu/tablet/CMakeLists.txt
File src/kudu/tablet/CMakeLists.txt:

PS2, Line 100: ADD_KUDU_TEST(maintenance_manager-test)
Forgot to remove this line?


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

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

[kudu-CR] Add a way to include request ids in log-dump

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Add a way to include request ids in log-dump
..


Patch Set 7: Code-Review+2

just rebases, keeping earlier +2s

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes and because testing it specifically is involved, this patch
doesn't include additional tests, these will be pushed as a follow up.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 399 insertions(+), 41 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 26
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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

[kudu-CR] Integrate the result tracker with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the result tracker with writes
..

Integrate the result tracker with writes

This patch integrates the result tracker with write transactions,
including:
- Support for caching responses on tablet bootstrap.
- Support for caching responses for follower transactions.
- Handling of races between client originated and (previous?) leader
  originated transactions.
- An explanation of the interaction between the result tracker
  and the transaction driver in transaction_driver.h.

For integration tests, this patch removes the check that allowed
Status::AlreadyPresent() that we added as we didn't have support for
exactly once semantics. Without this check, in slow mode, some tests
like raft_consensus-itest would fail 100% of the time, due to rows
being inserted multiple times.

Because we'd have 100% failure rate without replay cache for
writes and because testing it specifically is involved, this patch
doesn't include additional tests, these will be pushed as a follow up.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/consensus/consensus.proto
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tools/insert-generated-rows.cc
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/remote_bootstrap_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/tserver/tserver_service.proto
25 files changed, 399 insertions(+), 41 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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

[kudu-CR] Add integration tests for replay cache with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 24:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3519/24/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:

PS24, Line 1184:  defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER)
> We should only need -DADDRESS_SANITIZER because it's something we put in. S
Done


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

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

[kudu-CR] Memory tracking for result tracker

2016-07-14 Thread David Ribeiro Alves (Code Review)

Hello Kudu Jenkins,

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

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

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

Change subject: Memory tracking for result tracker
..

Memory tracking for result tracker

This adds memory tracking to ResultTracker, making sure we account for the 
memory
as we cache responses for clients' requests.

Testing wise this adds memory consumption checks to rpc-stress-test.cc.

Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
---
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc-stress-test.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/server/server_base.cc
5 files changed, 106 insertions(+), 15 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Memory tracking for result tracker

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Memory tracking for result tracker
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3627/6//COMMIT_MSG
Commit Message:

PS6, Line 10: client's
> nit: clients'
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Memory tracking for result tracker

Kudu Jenkins has posted comments on this change.

Change subject: Memory tracking for result tracker
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a way to include request ids in log-dump

Kudu Jenkins has posted comments on this change.

Change subject: Add a way to include request ids in log-dump
..


Patch Set 7:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I88d7c65887a98544ee83b5b4bc0817bea7222131
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Memory tracking for result tracker

Kudu Jenkins has posted comments on this change.

Change subject: Memory tracking for result tracker
..


Patch Set 7:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Move the maintenance manager to util

Kudu Jenkins has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 2:

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

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

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

[kudu-CR] Add integration tests for replay cache with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

Hello Kudu Jenkins,

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

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

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

Change subject: Add integration tests for replay cache with writes
..

Add integration tests for replay cache with writes

This adds a couple of new integration tests for replay cache with
writes. Both tests start multiple threads writing, independently, to
tablet servers simulaneously. The tests leverage the fact that followers
are also able to answer requests, once they are cached, and stores all
responses, which are compared at the end of the test.

Some of the requests (1/3) are "empty" writes, so that we stress the
serialization point in transaction_driver.cc without relying on row
lock serialization.

This adds two different tests, one that stresses a lot of elections
and one that crashes nodes. This is inline with other tests we already
had in raft_consensus-itest.

This also adds a new fault injection point right after the leader sends
a request. We currently have one right _before_ the leader sends
a request, but having one for after the request is sent encourages
stressing the path where a newly elected leader as both incoming
client request and ongoing replica transactions, which can possibly
race with each other if they correspond to the same write.

Finally this changes attempt_no in RequestIdPB to be an int64 instead
of just an int. While an int is more than enough in normal operation,
the new test generates many more attempts and we need a bigger number
to make sure all attempt numbers are unique.

I looped this about 1000 times, without related failures.

Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
---
M src/kudu/consensus/consensus_peers.cc
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/rpc/rpc_header.proto
6 files changed, 322 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 31
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Add integration tests for replay cache with writes

Kudu Jenkins has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 31:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 31
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the result tracker with writes

Kudu Jenkins has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 26:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 26
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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] Disable exactly once semantics by default and add a flag to enable it for tests

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Disable exactly once semantics by default and add a flag to 
enable it for tests
..


Disable exactly once semantics by default and add a flag to enable it for tests

Since exactly once semantics is still missing some pieces, like garbage 
collection
this disables it by default on the server, but adds a flag to allow enabling
it and enables it in all tablet server tests, by default.

Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Reviewed-on: http://gerrit.cloudera.org:8080/3506
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Kudu Jenkins
---
M src/kudu/integration-tests/external_mini_cluster-itest-base.h
M src/kudu/integration-tests/remote_bootstrap-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/service_if.cc
M src/kudu/tserver/tablet_server-test-base.h
6 files changed, 26 insertions(+), 1 deletion(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Move the maintenance manager to util

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 1:

(16 comments)

This change is required as we'll need a server-wide MM for the result tracker. 
Added comment in that regard to the commit message.

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

PS1, Line 9: it's
> nit: its
Done


Line 10: It doesn't change the namespace (kudu::) since that would be more 
involved.
> But isn't kudu:: the right namespace for something in util?
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/integration-tests/full_stack-insert-scan-test.cc
File src/kudu/integration-tests/full_stack-insert-scan-test.cc:

Line 40: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/master/master.cc
File src/kudu/master/master.cc:

Line 39: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


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

Line 36: ../util/maintenance_manager.cc
> What's this doing here? The maintenance manager is now part of libkudu_util
clion refactor. Done


Line 101: ADD_KUDU_TEST(maintenance_manager-test)
> The test should be moved too.
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

Line 48: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet_peer-test.cc
File src/kudu/tablet/tablet_peer-test.cc:

Line 35: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet_peer_mm_ops.cc
File src/kudu/tablet/tablet_peer_mm_ops.cc:

Line 27: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


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

Line 34: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tablet_server-test-base.h
File src/kudu/tserver/tablet_server-test-base.h:

Line 43: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


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

Line 30: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tserver-path-handlers.cc
File src/kudu/tserver/tserver-path-handlers.cc:

Line 35: #include "kudu/util/maintenance_manager.h"
> Nit: resort.
Done


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

Line 25: #include "maintenance_manager.h"
> Should be kudu/util, and resort.
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:

Line 18: #include "maintenance_manager.h"
> Nit: kudu/util.
Done


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

Line 17: #pragma once
> Nit: separate from the license with an empty line.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0
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: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [java client] Integrate with the replay cache

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] Integrate with the replay cache
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3631/4/java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
File java/kudu-client/src/main/java/org/kududb/client/TabletClient.java:

PS4, Line 254: 
requestIdBuilder.setFirstIncompleteSeqNo(requestTracker.firstIncomplete());
In the previous patch I wasn't sending this if the value would have been 
NO_SEQ_NO, but the C++ client does it and David thinks that field should be 
required.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I108cd30acbc308bfb4577d072c2a8f26d1553c68
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Memory tracking for result tracker

Jean-Daniel Cryans has posted comments on this change.

Change subject: Memory tracking for result tracker
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3627/6//COMMIT_MSG
Commit Message:

PS6, Line 10: client's
nit: clients'
(regular noun, plural, to show their possession)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Move the maintenance manager to util

Adar Dembo has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 1:

(15 comments)

What's the motivation for this move?

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

Line 10: It doesn't change the namespace (kudu::) since that would be more 
involved.
But isn't kudu:: the right namespace for something in util?


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/integration-tests/full_stack-insert-scan-test.cc
File src/kudu/integration-tests/full_stack-insert-scan-test.cc:

Line 40: #include "kudu/util/maintenance_manager.h"
Nit: resort.


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/master/master.cc
File src/kudu/master/master.cc:

Line 39: #include "kudu/util/maintenance_manager.h"
Nit: resort.


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

Line 36: ../util/maintenance_manager.cc
What's this doing here? The maintenance manager is now part of libkudu_util, so 
shouldn't this just be removed?


Line 101: ADD_KUDU_TEST(maintenance_manager-test)
The test should be moved too.


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

Line 48: #include "kudu/util/maintenance_manager.h"
Nit: resort.


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet_peer-test.cc
File src/kudu/tablet/tablet_peer-test.cc:

Line 35: #include "kudu/util/maintenance_manager.h"
Nit: resort.


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tablet/tablet_peer_mm_ops.cc
File src/kudu/tablet/tablet_peer_mm_ops.cc:

Line 27: #include "kudu/util/maintenance_manager.h"
Nit: resort.


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

Line 34: #include "kudu/util/maintenance_manager.h"
Nit: resort.


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tablet_server-test-base.h
File src/kudu/tserver/tablet_server-test-base.h:

Line 43: #include "kudu/util/maintenance_manager.h"
Nit: resort.


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

Line 30: #include "kudu/util/maintenance_manager.h"
Nit: resort.


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/tserver/tserver-path-handlers.cc
File src/kudu/tserver/tserver-path-handlers.cc:

Line 35: #include "kudu/util/maintenance_manager.h"
Nit: resort.


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

Line 25: #include "maintenance_manager.h"
Should be kudu/util, and resort.


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager.cc
File src/kudu/util/maintenance_manager.cc:

Line 18: #include "maintenance_manager.h"
Nit: kudu/util.


http://gerrit.cloudera.org:8080/#/c/3656/1/src/kudu/util/maintenance_manager.h
File src/kudu/util/maintenance_manager.h:

Line 17: #pragma once
Nit: separate from the license with an empty line.


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

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

[kudu-CR] Integrate the result tracker with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 25:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.h
File src/kudu/tablet/transactions/transaction_driver.h:

PS25, Line 176: request were
> nit: either request should be plural or were should be was.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 25
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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] Integrate the result tracker with writes

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 25:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.cc
File src/kudu/tablet/transactions/transaction_driver.cc:

PS25, Line 210: it's
> nit: its
Done


PS25, Line 268: er.");
> Any more info that we can add here?
Added the request id. Note that the user won't see this as the request is 
transparently retried.


http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/transaction_driver.h
File src/kudu/tablet/transactions/transaction_driver.h:

PS25, Line 124: client originated
> nit: client-originated
Done


PS25, Line 124: a
> nit: remove
Done


PS25, Line 149: to to 
> nit
Done


PS25, Line 155: it's
> nit: its
Done


PS25, Line 156: requests
> nit: request? same for the other "requests" on that line.
Done


PS25, Line 157:  
> nit
Done


PS25, Line 168: and
> nit: remove that 'and'
Done


PS25, Line 169: received
> nit: receive
Done


PS25, Line 302: drivers
> nit: driver
Done


http://gerrit.cloudera.org:8080/#/c/3449/25/src/kudu/tablet/transactions/write_transaction.h
File src/kudu/tablet/transactions/write_transaction.h:

PS25, Line 192: pointers to the rpc context, request and response, lifecyle
  :   // is managed by the rpc subsystem.
> Since you're here, mind rewriting this sentence? Start with upper case, the
Done


PS25, Line 194: Request
> nit: 'request_'
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 25
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
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-1358 (part 3): new multi-master stress test

Adar Dembo has posted comments on this change.

Change subject: KUDU-1358 (part 3): new multi-master stress test
..


Patch Set 9: Verified+1

chrpath() failures.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] rw mutex: prevent recursive use

Adar Dembo has posted comments on this change.

Change subject: rw_mutex: prevent recursive use
..


Patch Set 5: Verified+1

KUDU-1527 and isolate failures.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db
Gerrit-PatchSet: 5
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-1358 (part 2): heartbeat to every master

Adar Dembo has posted comments on this change.

Change subject: KUDU-1358 (part 2): heartbeat to every master
..


Patch Set 9: Verified+1

Isolate failed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1358 (part 1): master should accept heartbeat even if follower

Adar Dembo has posted comments on this change.

Change subject: KUDU-1358 (part 1): master should accept heartbeat even if 
follower
..


Patch Set 9: Verified+1

chrpath() failed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] master: do not delete unknown tablets

Adar Dembo has posted comments on this change.

Change subject: master: do not delete unknown tablets
..


Patch Set 5: Verified+1

More isolate and chrpath() failures.

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

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

[kudu-CR] KUDU-1374: send full tablet report when new leader master is detected

Adar Dembo has posted comments on this change.

Change subject: KUDU-1374: send full tablet report when new leader master is 
detected
..


Patch Set 5: Verified+1

More chrpath() and isolate failures.

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

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

[kudu-CR] master: add assert checks for leader lock

Dan Burkert has posted comments on this change.

Change subject: master: add assert checks for leader_lock
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefb5762c70192b27490cc71e20568815d18d6ad5
Gerrit-PatchSet: 5
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] rw mutex: prevent recursive use

Dan Burkert has posted comments on this change.

Change subject: rw_mutex: prevent recursive use
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db
Gerrit-PatchSet: 5
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] Add integration tests for replay cache with writes

Mike Percy has posted comments on this change.

Change subject: Add integration tests for replay cache with writes
..


Patch Set 24:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3519/24/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:

PS24, Line 1184:  defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER)
> gcc calls it one thing and clang calls it another. We've used the two elsew
We should only need -DADDRESS_SANITIZER because it's something we put in. See 
/CMakeLists.txt L301


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

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

[kudu-CR] Move the maintenance manager to util

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Move the maintenance manager to util
..

Move the maintenance manager to util

This moves the maintenance manager to util, along with it's required protobufs.
It doesn't change the namespace (kudu::) since that would be more involved.

Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0
---
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/master/master.cc
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.proto
M src/kudu/tablet/tablet_mm_ops.h
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer_mm_ops.cc
M src/kudu/tablet/tablet_peer_mm_ops.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/util/CMakeLists.txt
R src/kudu/util/maintenance_manager-test.cc
R src/kudu/util/maintenance_manager.cc
R src/kudu/util/maintenance_manager.h
A src/kudu/util/maintenance_manager.proto
18 files changed, 83 insertions(+), 57 deletions(-)


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

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

[kudu-CR] Move the maintenance manager to util

Kudu Jenkins has posted comments on this change.

Change subject: Move the maintenance manager to util
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcf072d443ac3d069bda15b9dc0f8c442b9ac5c0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package

2016-07-14 Thread Dinesh Bhat (Code Review)

Dinesh Bhat has posted comments on this change.

Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode 
package
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3653/1/docs/installation.adoc
File docs/installation.adoc:

> What is the "XCode package"? Is that something that's totally obvious to a 
Package itself seems distinct from the cmdline tools, because I had cmdline 
tools installed before observing build failures. Is Xcode an obvious package 
for macOS user ? If you are building/developing on mac, I would guess so.


Line 474: The Xcode package is necessary for compiling Kudu. Some of the 
instructions
> This can probably be simplified to just say that the full xcode install is 
Cool, thanks.


Line 476: manual dependency installation is possible.
> Nit: "...that the Xcode package is..."
I figured it's better to remove cli tools references, and just say Xcode needed 
as Dan is suggesting.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Dinesh Bhat 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Disable exactly once semantics by default and add a flag to enable it for tests

Jean-Daniel Cryans has posted comments on this change.

Change subject: Disable exactly once semantics by default and add a flag to 
enable it for tests
..


Patch Set 16: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Disable exactly once semantics by default and add a flag to enable it for tests

Kudu Jenkins has posted comments on this change.

Change subject: Disable exactly once semantics by default and add a flag to 
enable it for tests
..


Patch Set 16:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Disable exactly once semantics by default and add a flag to enable it for tests

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has posted comments on this change.

Change subject: Disable exactly once semantics by default and add a flag to 
enable it for tests
..


Patch Set 15:

(1 comment)

adressed the nit, keeping the +2

http://gerrit.cloudera.org:8080/#/c/3506/15/src/kudu/integration-tests/ts_itest-base.h
File src/kudu/integration-tests/ts_itest-base.h:

PS15, Line 102: =true
> nit: you don't need that.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1530: Update docs about OS X build dependency on Xcode package

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1530: Update docs about OS X build dependency on Xcode 
package
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8962d5539f437dba8b120c70c90c1e384ed550c9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Integrate the request tracker with the client

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Integrate the request tracker with the client
..


Integrate the request tracker with the client

This integrates the request tracker with the client, making sure
that write requests sent by the client are tracked in the RequestTracker
and contain the information necessary for exactly once semantics.

This adds to rpc-stress-test a test that uses RetriableRpc, instead
of using the proxy directly.

Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Reviewed-on: http://gerrit.cloudera.org:8080/3080
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc-stress-test.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
M src/kudu/rpc/rtest.proto
10 files changed, 244 insertions(+), 6 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Integrate the ResultTracker into the rpc subsystem

2016-07-14 Thread David Ribeiro Alves (Code Review)

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Integrate the ResultTracker into the rpc subsystem
..


Integrate the ResultTracker into the rpc subsystem

This integrates the ResultTracker into the rpc subsystem by allowing
to specify a new method option when defining rpc service methods.
When this method option is specified _and_ the call's rpc header
has a RequestIdPB the results for the call will be tracked and the
call may have exactly once semantics.

This allows to have the simplest form of exactly once semantics for
single server calls. Of course limitations apply, like response
persistence across restarts is not supported, neither is propagating/rebuilding
responses to/on replicas for replicated calls. If support for this
is needed it will have to be done ad-hoc, case by case.

This adds a tests that has multiple threads trying to execute the same
rpc, and makes sure each one was executed exactly once.

Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Reviewed-on: http://gerrit.cloudera.org:8080/3192
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/master/master.cc
M src/kudu/master/master_service.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/protoc-gen-krpc.cc
A src/kudu/rpc/rpc-stress-test.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/rtest.proto
M src/kudu/rpc/service_if.cc
M src/kudu/rpc/service_if.h
M src/kudu/server/generic_service.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tserver/remote_bootstrap_service.cc
M src/kudu/tserver/remote_bootstrap_service.h
M src/kudu/tserver/tablet_server.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
21 files changed, 372 insertions(+), 53 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] Disable exactly once semantics by default and add a flag to enable it for tests

Jean-Daniel Cryans has posted comments on this change.

Change subject: Disable exactly once semantics by default and add a flag to 
enable it for tests
..


Patch Set 15: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3506/15/src/kudu/integration-tests/ts_itest-base.h
File src/kudu/integration-tests/ts_itest-base.h:

PS15, Line 102: =true
nit: you don't need that.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1358 (part 2): heartbeat to every master

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1358 (part 2): heartbeat to every master
..


Patch Set 9:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1358 (part 2): heartbeat to every master

Hello Kudu Jenkins,

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

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

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

Change subject: KUDU-1358 (part 2): heartbeat to every master
..

KUDU-1358 (part 2): heartbeat to every master

Now that followers accept heartbeats, let's modify the tserver to send one
to every master. Spawning a heartbeater thread for each master seemed like
the natural way to do this; it should simplify dynamic master changes in the
future (i.e. just add or remove threads as needed).

The "dirty tablet" state is now encapsulated in the heartbeater threads
themselves, and the heartbeater must "fan out" to manipulate all of it. It's
a little noisy but I think it's reasonable. The alternative is for this
state to remain in the TSTabletManager, for the heartbeater to continue
tracking which master is the leader, and for it to only send tablet reports
to that master. This can be done with a few changes (e.g. adding term
numbers to the heartbeat response), but the only benefit is reduced network
traffic when tablets are dirty, so that didn't seem worth the complexity.

There's no new test here, but this code path is exercised in the test I
reenabled, and in the new stress test (follow-on patch).

Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
---
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/tserver/heartbeater.h
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
7 files changed, 311 insertions(+), 263 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-1374: send full tablet report when new leader master is detected

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1374: send full tablet report when new leader master is 
detected
..


Patch Set 5:

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

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

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

[kudu-CR] KUDU-1358 (part 1): master should accept heartbeat even if follower

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1358 (part 1): master should accept heartbeat even if 
follower
..


Patch Set 9:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] master: do not delete unknown tablets

Kudu Jenkins has posted comments on this change.

Change subject: master: do not delete unknown tablets
..


Patch Set 5:

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

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

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

[kudu-CR] Integrate the ResultTracker into the rpc subsystem

Jean-Daniel Cryans has posted comments on this change.

Change subject: Integrate the ResultTracker into the rpc subsystem
..


Patch Set 26: Code-Review+2

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

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

[kudu-CR] master: add read-write lock to serialize operations around elections

Adar Dembo has submitted this change and it was merged.

Change subject: master: add read-write lock to serialize operations around 
elections
..


master: add read-write lock to serialize operations around elections

This rigmarole began with an investigation into a test failure [1], which
led to a new integration test that hammers VisitTablesAndTablets() while
creating tables. That test revealed other locking issues, which brings us
to where we are now.

This patch introduces a read-write lock to serialize all master operations
so that they fall on one side or the other of a leader election. The idea
is to avoid performing operations concurrently with a reload of the master
metadata; doing so can lead to problems in Shutdown() and (very rarely,
perhaps only conceptually) to inconsistent on-disk state.

I was hoping this lock could replace the fencing done by leader_ready_term_,
but eventually reasoned that we need both; without leader_ready_term_
fencing, the master's consensus state machine could fool an operation into
thinking the master became the leader before the metadata was reloaded.

Three other things of note here:
- The new lock is acquired via TryLock() so that, if the lock could not be
  acquired, the RPC will fail rather than block. A future patch modifies
  TSHeartbeat() to partially accept heartbeats even if the master is a
  follower; TryLock() means that a transitioning leader that is pelted with
  RPCs won't fill up its service queue and can still process heartbeats.
- TableInfo's AddTask() and RemoveTask() methods now don't hold the table's
  lock when adding and removing refs from the task respectively. This is
  the fix for the original test failure.
- When reloading metadata, we now abort all outstanding table tasks to
  avoid orphaning them.

1. 
http://dist-test.cloudera.org:8080/diagnose?key=224b3aa2-3c87-11e6-9a09-0242ac110001

Change-Id: I5084c09f1a77ccf620fb6cd621094c4778d636f8
Reviewed-on: http://gerrit.cloudera.org:8080/3550
Reviewed-by: Dan Burkert 
Tested-by: Adar Dembo 
---
M src/kudu/integration-tests/create-table-stress-test.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master_service.cc
6 files changed, 334 insertions(+), 144 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Adar Dembo: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5084c09f1a77ccf620fb6cd621094c4778d636f8
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] master: fix corruption when AlterTable() races with CreateTable()

Adar Dembo has submitted this change and it was merged.

Change subject: master: fix corruption when AlterTable() races with 
CreateTable()
..


master: fix corruption when AlterTable() races with CreateTable()

Admittedly, this is a contrived scenario:
1. T1 tries to create table with name 'foo'
2. T2 tries to rename table with name 'bar' to 'foo'

With just the right timing, both operations succeed and the metadata now has
two tables named 'foo', each with a different table ID. The fix is simple:
generalize the "tables being created" logic already used by CreateTable().

Without the fix, the new test failed every 50th run or so. With it, it
doesn't fail in 1000 runs.

Change-Id: I6c9e4214c09bc47a5a10b12d6ffe8b35906708c9
Reviewed-on: http://gerrit.cloudera.org:8080/3607
Reviewed-by: Dan Burkert 
Tested-by: Adar Dembo 
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-test.cc
3 files changed, 231 insertions(+), 30 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Adar Dembo: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6c9e4214c09bc47a5a10b12d6ffe8b35906708c9
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] master: fix corruption when AlterTable() races with CreateTable()