[kudu-CR] Avoid unused var warning in deltafile-test.cc

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Avoid unused var warning in deltafile-test.cc
..

Avoid unused var warning in deltafile-test.cc

Now that in memory env has been removed kTestPath is no longer
needed and was causing a compilation warning.

Change-Id: I8b8ad55b3a7711515e84bc6453bd152b41556f14
---
M src/kudu/tablet/deltafile-test.cc
1 file changed, 0 insertions(+), 3 deletions(-)


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

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

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4: Code-Review+2

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

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

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: RaftConsensus: Trigger election at startup if single node
..


Patch Set 2:

(1 comment)

can you add a test for this? something simple like setting the election timeout 
really high and then making sure we get a leader immediately?

http://gerrit.cloudera.org:8080/#/c/3344/2/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

Line 312:   bool only_voter = false;
pull this out to a method. something like IsSingleNodeConfig() or 
IsSingleVoterConfig


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief51ab20c051db83bea51c146b24a11036d9f953
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Add WaitUntilLeader() to Consensus interface

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add WaitUntilLeader() to Consensus interface
..


Patch Set 2:

where is this used?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6d8e0dd63e58c8ccdcf6497f0eb25be68b238ea
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Remove the LocalConsensus implementation

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 3:

general wondering. Should we just kill the consensus.h interface and refactor 
RaftConsensus->Consensus or use RaftConsensus directly? Seems unlikely that 
we'll ever want another consensus impl, so one less indirection layer and less 
virtual methods. Any roadblocks on the way to do this?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I24e671dc732a1cdf4c453dfec61cefa0c573252c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Remove the LocalConsensus implementation

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 3:

yeah was not saying that we needed to do it here. Not sure there is much to 
discuss though, since you already discussed the removal of local consensus and 
it since there is a single implementation its simple cleaning.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I24e671dc732a1cdf4c453dfec61cefa0c573252c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Do not run (g)addr2line translator on MacOS X
..


Patch Set 2: Code-Review+2

Ran the tests in my mac. When running on ctest all tests that injected failures 
were failing due to this. now they pass. lgtm (if jenkins agrees)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Allow to set RequestId in the RPC RequestHeader

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Allow to set RequestId in the RPC RequestHeader
..

Allow to set RequestId in the RPC RequestHeader

This allows to set a RequestId in the RpcController that will in turn
be set in the RequestHeader.

This doesn't have specific tests, as other patches test this functionality,
but having this be stand-alone allows to work on the client and the server
simultaneously.

Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4
---
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
3 files changed, 19 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add unique id generation to the client

[David Ribeiro Alves (Code Review)]

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

Change subject: Add unique id generation to the client
..


Add unique id generation to the client

This adds unique id generation to the client, which takes the form of
an uuid. The uuid is generated when the client is built, by a static
OidGenerator.

This also adds a test that generates 1000 clients and makes sure that
the generated ids are unique. Since this test will now run all the time
we should get notified if stuff becomes flaky dues to repeated ids.

Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650
Reviewed-on: http://gerrit.cloudera.org:8080/3077
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
Reviewed-by: Todd Lipcon 
---
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/integration-tests/client-stress-test.cc
4 files changed, 26 insertions(+), 0 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, but someone else must approve
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Fix stray memory writes due to tcmalloc profiling

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Fix stray memory writes due to tcmalloc profiling
..


Patch Set 1: Code-Review+2

good catch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9afca83d9cc24585960f6bf68d8996c4736ce6cb
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: song bruce zhang 
Gerrit-HasComments: No

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: RaftConsensus: Trigger election at startup if single node
..


Patch Set 5: Code-Review+2

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

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

[kudu-CR] Use RaftConsensus instead of LocalConsensus in tests

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Use RaftConsensus instead of LocalConsensus in tests
..


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3346/4/src/kudu/consensus/consensus.h
File src/kudu/consensus/consensus.h:

Line 35: 
remove extra line


PS4, Line 136: WaitUntilLeader
rename this? append ForTests


http://gerrit.cloudera.org:8080/#/c/3346/4/src/kudu/tserver/ts_tablet_manager-test.cc
File src/kudu/tserver/ts_tablet_manager-test.cc:

Line 89: 
RETURN_NOT_OK(tablet_peer->consensus()->WaitUntilLeader(MonoDelta::FromSeconds(10)));
can just return here


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

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

[kudu-CR] Remove the LocalConsensus implementation

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3350/5/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:

Line 89:   // Obsolete. This parameter has been retired.
why are we keeping this around?


http://gerrit.cloudera.org:8080/#/c/3350/5/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

Line 255: CHECK_EQ(server_->instance_pb().permanent_uuid(), 
config.peers(0).permanent_uuid());
can we change this to make it generic, i.e. make sure that our uuid is among 
the uuids of all peers?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I24e671dc732a1cdf4c453dfec61cefa0c573252c
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Avoid missing 'override' keyword warnings in 
raft_consensus-test.cc
..


Patch Set 2: Verified+1

unrelated failure on 
org.kududb.client.TestTimeouts.org.kududb.client.TestTimeouts

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
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: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Mark the Response accessors in transactions/transaction's state const

[David Ribeiro Alves (Code Review)]

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

Change subject: Mark the Response accessors in transactions/transaction's state 
const
..


Mark the Response accessors in transactions/transaction's state const

There is no reason why these accessors should't be const and by making
them as such we can access the response through state(), instead of
mutable_state().

Change-Id: I7dbcc4bf8fc657a80f08806ce5e016601ef429bc
Reviewed-on: http://gerrit.cloudera.org:8080/3419
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/write_transaction.h
3 files changed, 3 insertions(+), 3 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I7dbcc4bf8fc657a80f08806ce5e016601ef429bc
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: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Avoid missing 'override' keyword warnings in 
raft_consensus-test.cc
..


Patch Set 2:

was looking into updating gmock and it seems like it has moved to a new project 
https://github.com/google/googletest which has a different layout (uses cmake 
instead of make) so updating this won't just be updating the zip on s3. Would 
you rather still do it or can we just ignore the warning for now?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Added more to the documentation for botched installation. Users should uninstall the dependencies and install them again after cleaning the project.

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Added more to the documentation for botched installation. Users 
should uninstall the dependencies and install them again after cleaning the 
project.
..


Patch Set 1:

(2 comments)

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

Line 7: Added more to the documentation for botched installation. Users
look at other patches. we usually have a short title and then a longer 
explanation (with a blank line in between).

A possible example would be:

Added information about how to workaround a botched thirdparty build

In some cases the thirdparty build might become stuck if the user didn't make 
sure the pre-requisites were met before starting the thirdparty build. This 
patch adds information on what should be done in that case: the user should 
clean the workspace and then try to re-build.


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

Line 561: $ brew uninstall autoconf automake cmake libtool pkg-config boost 
pstree
did you need to uninstall/reinstall?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I451549b8396d929dd53db629ff9ba5c9f87ca5a2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] WIP: Exactly once semantics for writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP: Exactly once semantics for writes
..

WIP: Exactly once semantics for writes

Not for review, just getting jenkins runs

Change-Id: I99df757741057bc140272959576bd10cb63d7448
---
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/client/meta_cache.h
M src/kudu/client/symbols.map
M src/kudu/common/common.proto
M src/kudu/consensus/CMakeLists.txt
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/protoc-gen-krpc.cc
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/retriable_rpc.h
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_controller.cc
M src/kudu/rpc/rpc_controller.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/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/alter_schema_transaction.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/transaction_tracker-test.cc
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/tablet_service.cc
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_service.proto
42 files changed, 1,103 insertions(+), 77 deletions(-)


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

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

[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the ResultTracker into the rpc subsystem and add a 
test
..


Patch Set 12: Verified+1

(12 comments)

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

Line 10: to specify a new method option when defining rpc service methods.
> I think you missed this.
hrm, you're right. I could swear I had fixed this. must have lost a patch along 
the way. Done


http://gerrit.cloudera.org:8080/#/c/3192/11/src/kudu/common/common.proto
File src/kudu/common/common.proto:

Line 316
> I dont think this should be in 'common'. 'common' is for data-model type st
Done


http://gerrit.cloudera.org:8080/#/c/3192/11/src/kudu/rpc/CMakeLists.txt
File src/kudu/rpc/CMakeLists.txt:

Line 89: rpc_header_proto
> oh, now I see why it builds. This dependency seems misplaced. (eg keep in m
Done


http://gerrit.cloudera.org:8080/#/c/3192/11/src/kudu/rpc/protoc-gen-krpc.cc
File src/kudu/rpc/protoc-gen-krpc.cc:

Line 46: #include "kudu/rpc/rpc_header.pb.h"
> nit: sorting
Done


Line 467:   "mi->result_tracker.reset($result_tracker$);\n"
> we have a separate ResultTracker per RPC type? that doesn't seem right.
we do, why doesn't it seem right? easier to track where memory goes , maps will 
be smaller so faster. locks will be less contended... why wouldn't we want to 
have one per rpc?


http://gerrit.cloudera.org:8080/#/c/3192/11/src/kudu/rpc/rpc-stress-test.cc
File src/kudu/rpc/rpc-stress-test.cc:

PS11, Line 73: random amount 
> update
Done


Line 134: vector adders;
> use unique_ptr?
Done


http://gerrit.cloudera.org:8080/#/c/3192/11/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:

Line 288:   std::atomic_int exactly_once_test_val_;
> hrm, is this atomic? haven't seen this
yeah it's typedefd, want me to change it?


http://gerrit.cloudera.org:8080/#/c/3192/11/src/kudu/rpc/rpc_context.cc
File src/kudu/rpc/rpc_context.cc:

PS11, Line 66: call_->RespondSuccess(*response_pb_);
 : delete this;
 :  
> per the comment on the other review, it seems weird we now have this code s
well yeah, for the case when the result aren't being tracked. ideally we'd like 
to at least coalesce the log statements at least, was kinda bugging me but 
didn't spend too much time on it. Any ideas?


http://gerrit.cloudera.org:8080/#/c/3192/11/src/kudu/rpc/rtest.proto
File src/kudu/rpc/rtest.proto:

Line 129:   }
> add something to design-docs/rpc.md about this feature?
Done


http://gerrit.cloudera.org:8080/#/c/3192/6/src/kudu/rpc/service_if.cc
File src/kudu/rpc/service_if.cc:

PS6, Line 96:  req.release(),
:resp,
:
call->header().has_request_id() ? method_info->result_tracker :
:   
   nullptr);
: 
> Missed this?
Done


Line 116:   } else {
> Log the state too.
Done


-- 
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: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 13: Verified+1

unrelated flake ReplicatedAlterTableTest.TestReplicatedAlter

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

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

[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the ResultTracker into the rpc subsystem and add a 
test
..

Integrate the ResultTracker into the rpc subsystem and add a test

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 specificed _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.

Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
---
M docs/design-docs/rpc.md
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
12 files changed, 356 insertions(+), 44 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Allow to set RequestId in the RPC RequestHeader

[David Ribeiro Alves (Code Review)]

Hello Adar Dembo, Kudu Jenkins,

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

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

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

Change subject: Allow to set RequestId in the RPC RequestHeader
..

Allow to set RequestId in the RPC RequestHeader

This allows to set a RequestId in the RpcController that will in turn
be set in the RequestHeader.

This doesn't have specific tests, as other patches test this functionality,
but having this be stand-alone allows to work on the client and the server
simultaneously.

Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4
---
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
3 files changed, 34 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4
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: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

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

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

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


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3080/8//COMMIT_MSG
Commit Message:

Line 15: any specific tests.
> it's not possible to use RetriableRpc within rpc_stub-test to test the clie
I guess I can come up with a unit test for retriable rpc, if that helps, but it 
wouldn't test it with the meta cache (retriable rpc received a server picker 
that, on the client, is backed by the meta cache).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
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: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR](branch-0.9.x) catalog manager: fix a locking error

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: catalog_manager: fix a locking error
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8db7a78309bb9182ae1fdb92485e8467b90a84b2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-0.9.x
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-HasComments: No

[kudu-CR] KUDU-1477. Pending COMMIT message for failed write operation can prevent tablet startup

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1477. Pending COMMIT message for failed write operation 
can prevent tablet startup
..


Patch Set 3: Code-Review+2

(1 comment)

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

Line 271: // At least one operation resulted in a mutation to a store, and 
at least
> Changed the API as you suggested, though not sure what you mean about the "
though there would be some cases where we'd do both. even if not like it better 
like this. thanks for fixing.


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

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

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 12:

(1 comment)

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

Line 221: }
> lots of dup code in the above methods. any refactor doable? (even one that 
Done


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

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

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 1:

this is good to be reviewed, but it's still missing a flag to disable it until 
we've finished garbage collection

-- 
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: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] WIP: Exactly once semantics for writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has abandoned this change.

Change subject: WIP: Exactly once semantics for writes
..


Abandoned

superceded by other patches

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I99df757741057bc140272959576bd10cb63d7448
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

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

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new patch set (#8).

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.

Since this is hard to test in isolation (without the server part) and it
is already tested by the next patch in the sequence, this doesn't contain
any specific tests.

Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
---
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/client/meta_cache.h
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
8 files changed, 74 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a RpcContext::RespondFailure() method

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a RpcContext::RespondFailure() method
..


Patch Set 6:

(2 comments)

bq. wonder whether it would be a clearer API to instead have a flag like 
context->DoNotCacheResponse(); which must be called in these cases? It gets a 
bit ugly that we have so many different "RespondFailure" code paths now

Yeah, I don't really understand the difference between the names for the 
unsuccessful Respond* methods the naming distinction seems forced and 
meaningless. IMO these should all have the same name, i.e. be overloads of each 
other, explaining in which cases we'd want to use each different overload.

Using context->DoNotCacheResponse() breaks the abstraction in the single-server 
rpc case. Even though this might not be ideal I'd still prefer the new:

"
... set the error
context->RespondFailure()
"

to the alternative:

"
... set the error.
context->DontCacheResponse();
context->RespondSuccess(); // ?
"

Not that the user of this API does not need to know that the response is the 
same whether RespondSuccess() and RespondFailure(), it still makes more sense 
to call context->RespondFailure() after we've just set an error on the response.

http://gerrit.cloudera.org:8080/#/c/3191/6/src/kudu/rpc/rpc_context.h
File src/kudu/rpc/rpc_context.h:

Line 84:   // Mark this call as failed but don't actually change the response.
> this isn't clear that it also responds -- "Mark as failed" doesn't have the
Done


Line 87:   // to mark the call as failed.
> would be good to explain what actual effect this has
Done


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

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

[kudu-CR] Allow to set RequestId in the RPC RequestHeader

[David Ribeiro Alves (Code Review)]

Hello Adar Dembo,

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

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

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

Change subject: Allow to set RequestId in the RPC RequestHeader
..

Allow to set RequestId in the RPC RequestHeader

This allows to set a RequestId in the RpcController that will in turn
be set in the RequestHeader.

This doesn't have specific tests, as other patches test this functionality,
but having this be stand-alone allows to work on the client and the server
simultaneously.

Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4
---
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/rpc_controller.cc
M src/kudu/rpc/rpc_controller.h
3 files changed, 20 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4
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: Todd Lipcon 

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 402 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Avoid missing 'override' keyword warnings in 
raft_consensus-test.cc
..


Patch Set 2:

we can will do (disregard the update of this patch)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
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: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Clarify the different between 'call id' and 'request id'

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Clarify the different between 'call_id' and 'request_id'
..

Clarify the different between 'call_id' and 'request_id'

This adds a bit more information on the difference between 'call_id'
and 'request_id'.

Change-Id: If040e79baea55f4d16b43ff64b7ec27a403fc18f
---
M src/kudu/rpc/rpc_header.proto
1 file changed, 9 insertions(+), 3 deletions(-)


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

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

[kudu-CR] Allow to set RequestId in the RPC RequestHeader

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Allow to set RequestId in the RPC RequestHeader
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3179/3/src/kudu/rpc/rpc_controller.h
File src/kudu/rpc/rpc_controller.h:

Line 110:   void SetRequestIdPB(std::unique_ptr request_id);
> per some comments on an earlier (committed) patch in this series, I think w
are you referring to https://gerrit.cloudera.org/#/c/3079/ ? I did address your 
comment and add more info in rpc_header.proto.

I quite like the "request" terminology, think it makes sense even when sending 
something to different replicas.  Unless you have a better naming suggestion 
I'll add (even) more info to rpc_header.proto to make the distinction clear. 
Will push a patch just for that so that we can discuss it there.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I92566250683ee69fad57a9ae694842d4a0b17ab4
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: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Allow crcutil* symbols in the client

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Allow crcutil* symbols in the client
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3412/4/src/kudu/client/symbols.map
File src/kudu/client/symbols.map:

Line 32:   # From src/kudu/util/crc.{h,cc}.
> That's not quite right, though; these symbols are defined by crcutil in thi
honestly not sure what information that is adding, but Done.


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

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

[kudu-CR] Allow crcutil* symbols in the client

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Allow crcutil* symbols in the client
..


Patch Set 3:

don't follow, isn't it ok if it follows this one on the sequence?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87fa1b69390a566ab141cf4cf9be84608be73390
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: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the ResultTracker into the rpc subsystem and add a 
test
..

Integrate the ResultTracker into the rpc subsystem and add a test

This integrates the ResultTracker into the rpc subsystem by allowing
to specify a new mehtod option when defining rpc service methods.
When this method option is specificed _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.

Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
---
M src/kudu/common/common.proto
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/rtest.proto
M src/kudu/rpc/service_if.cc
M src/kudu/rpc/service_if.h
11 files changed, 325 insertions(+), 38 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
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: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Allow crcutil* symbols in the client

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Allow crcutil* symbols in the client
..

Allow crcutil* symbols in the client

A previous patch moved PB tracing utilities into pb_util.cc/h but
in order to use these in the client (in RcpController, follow up patch)
the crcutil* symbols need to be allowed.

Change-Id: I87fa1b69390a566ab141cf4cf9be84608be73390
---
M src/kudu/client/symbols.map
1 file changed, 2 insertions(+), 0 deletions(-)


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

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

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 411 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
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: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the ResultTracker into the rpc subsystem and add a 
test
..

Integrate the ResultTracker into the rpc subsystem and add a test

This integrates the ResultTracker into the rpc subsystem by allowing
to specify a new mehtod option when defining rpc service methods.
When this method option is specificed _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.

Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
---
M src/kudu/common/common.proto
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/rtest.proto
M src/kudu/rpc/service_if.cc
M src/kudu/rpc/service_if.h
11 files changed, 330 insertions(+), 43 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 423 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Clarify the difference between 'call id' and 'request id'

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Clarify the difference between 'call_id' and 'request_id'
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3409/2/src/kudu/rpc/rpc_header.proto
File src/kudu/rpc/rpc_header.proto:

PS2, Line 168: // Note that this is different from 'call_id' in that 'call_id's 
are per server unique and
 :   // 'request_id' are per logical request unique, even if when 
the request is tried on different
 :   // servers.
> Nit: reword as "Note that this is different from 'call_id' in that a call_i
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If040e79baea55f4d16b43ff64b7ec27a403fc18f
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: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Allow crcutil* symbols in the client

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Allow crcutil* symbols in the client
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3412/3/src/kudu/client/symbols.map
File src/kudu/client/symbols.map:

Line 31:   crcutil::*;
> Nit: break this out with an empty line and declare the name of the library 
Done


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

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

[kudu-CR] Add a RpcContext::RespondFailure() method

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a RpcContext::RespondFailure() method
..

Add a RpcContext::RespondFailure() method

In the case of certain errors we use RpcContext::RespondSuccess() because
setting the error is call dependent and it was done on the call's request.
Since we'll be relying on the RpcContext to call the appropriate method
on the ResultTracker, on the common case, we'd still be marking these error
cases as successful RPCs whose results we keep. This is problematic for
various reasons, the main one being that if a client gets a transient error
back (e.g. the call was throttled) all subsequent attemtpts at the same
call will get the same result.

To avoid this we need to be able to flag that a call failed even if we don't
need anything else done to the response. To this goal this patch adds
RpcContext::RespondFailure(), which currently does nothing else besides
call RespondSuccess(), but will do so when the result tracker is integrated
in.

Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
---
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
2 files changed, 19 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a RpcContext::RespondFailure() method

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a RpcContext::RespondFailure() method
..


Patch Set 10: Verified+1

unrelated flake MasterTest.TestMasterMetadataConsistentDespiteFailures

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

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

[kudu-CR] Make 'num attempts' in RequestIdPB required

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Make 'num_attempts' in RequestIdPB required
..

Make 'num_attempts' in RequestIdPB required

We need to differentiate between attempts, on the server, so that we
can handle dangling handlers when multiple attempts at the same RPC
are happening at the same time (e.g. a client-originated write rpc
and the same rpc originating from another replica).

Differentiating which attempt is executing on the server side
requires distinguishing between different attempts. Using 'num_attempts'
is a natural way to do so.

Change-Id: Idd8f780826438278993554546edfae90bf1a39df
---
M src/kudu/rpc/rpc_header.proto
1 file changed, 3 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd8f780826438278993554546edfae90bf1a39df
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: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Make 'num attempts' in RequestIdPB required

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Make 'num_attempts' in RequestIdPB required
..


Patch Set 2:

(1 comment)

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

Line 15: requires distinguising different attempts. Using 'num_attempts' is
> You missed "distinguishing" though.
dammit, thanks for catching that.
Done


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

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

[kudu-CR] RaftConsensus: Trigger election at startup if single node

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: RaftConsensus: Trigger election at startup if single node
..


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3344/4/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

PS4, Line 293: tablet
no need to mention "tablet", i.e. "...Raft configuration."


Line 294:   Status IsSingleVoterConfig(bool* only_voter) const;
you use "single" here and "only" in the param, can you refactor the param to be 
"single" too?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief51ab20c051db83bea51c146b24a11036d9f953
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Add blog post about removing LocalConsensus

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add blog post about removing LocalConsensus
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3398/1/_posts/2016-06-17-raft-consensus-single-node.md
File _posts/2016-06-17-raft-consensus-single-node.md:

Line 17: cluster's existing master server replication factor from 1 to 3 (or 5).
this might confuse people. its perfectly legal to increase to 2 right (just no 
fault tolerance) just mention from 1 to multiple nodes or something.


Line 71: in the future. Kudu fits this use case. When deploying Kudu, someone 
may wish
what use case? "Kudu fits..." sounds wrong to me here. I'd remove it


PS1, Line 78: two
you use words for the numbers here but use the actual numbers in the 2nd 
paragraph


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I571de0cbf4d73511f17bd57d374fed9e8ca302e4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] WIP: Exactly once semantics for writes

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: WIP: Exactly once semantics for writes
..

WIP: Exactly once semantics for writes

Not for review, just getting jenkins runs

Change-Id: I99df757741057bc140272959576bd10cb63d7448
---
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/client/meta_cache.h
M src/kudu/client/symbols.map
M src/kudu/common/common.proto
M src/kudu/consensus/CMakeLists.txt
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/protoc-gen-krpc.cc
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/retriable_rpc.h
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_controller.cc
M src/kudu/rpc/rpc_controller.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/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tablet/transactions/alter_schema_transaction.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/transaction_tracker-test.cc
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/tablet_service.cc
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_service.proto
42 files changed, 1,103 insertions(+), 77 deletions(-)


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

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

[kudu-CR] alter schema transaction: don't crash in ToString() when no timestamp

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: alter_schema_transaction: don't crash in ToString() when no 
timestamp
..


Patch Set 1: Code-Review+2

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

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

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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.

Since this is hard to test in isolation (without the server part) and it
is already tested by the next patch in the sequence, this doesn't contain
any specific tests.

Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
---
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_controller.cc
M src/kudu/rpc/rpc_controller.h
7 files changed, 57 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the result tracker with writes

[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 (#5).

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 this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/service_if.cc
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_server.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.proto
M src/kudu/tserver/tserver_service.proto
25 files changed, 387 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/3449/5
-- 
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: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Make RequestTracker not return Status on FirstIncomplete()

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Make RequestTracker not return Status on FirstIncomplete()
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cdcb2b4c0d2d983bd684b5dccf75a81530da93e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Make RequestTracker not return Status on FirstIncomplete()

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Make RequestTracker not return Status on FirstIncomplete()
..


Patch Set 2:

unrelated flake 
org.kududb.client.TestStatistics.org.kududb.client.TestStatistics

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cdcb2b4c0d2d983bd684b5dccf75a81530da93e
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] tablet: change default bloom filter FP rate to 0.01%

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: tablet: change default bloom filter FP rate to 0.01%
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3517/1/docs/release_notes.adoc
File docs/release_notes.adoc:

PS1, Line 72: substantially
how substantantially?


PS1, Line 74: incremental
which increase?

Can you provide rough numbers for both?


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

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

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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.

Since this is hard to test in isolation (without the server part) and it
is already tested by the next patch in the sequence, this doesn't contain
any specific tests.

Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
---
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_controller.cc
M src/kudu/rpc/rpc_controller.h
7 files changed, 65 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new patch set (#3).

Change subject: Avoid missing 'override' keyword warnings in 
raft_consensus-test.cc
..

Avoid missing 'override' keyword warnings in raft_consensus-test.cc

In this test we're using gmock and the compiler spews out warnings
that the mocked methods are missing the 'override' keyword.

This just makes it so that we ignore that warning in that file.

Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
---
M src/kudu/consensus/CMakeLists.txt
1 file changed, 8 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
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: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Avoid missing 'override' keyword warnings in 
raft_consensus-test.cc
..


Patch Set 2:

(1 comment)

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

Line 140: set_source_files_properties(raft_consensus-test.cc
> OK. fair enough on not upgrading, but maybe drop a comment here explaining 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
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: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a test for the integration of RequestTracker with the client and ResultTracker with the server

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a test for the integration of RequestTracker with the 
client and ResultTracker with the server
..

Add a test for the integration of RequestTracker with the client and 
ResultTracker with the server

This adds a test for the integration of the RequestTracker in the client
and the ResultTracker in the server. Specifically it tests that RetriableRpc
works in combination with replayed results from the ResultTracker and that
multiple simultaneous (similar) RPCs from the client are not executed more
than once.

Change-Id: Ic193dd5a547fe2a107a2ab1625f93fa0c19cc61b
---
A src/kudu/rpc/rpc-stress-test.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rtest.proto
3 files changed, 366 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic193dd5a547fe2a107a2ab1625f93fa0c19cc61b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] tablet: change default bloom filter FP rate to 0.01%

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: tablet: change default bloom filter FP rate to 0.01%
..


Patch Set 2: Code-Review+2

thanks for adding the additional info

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

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

[kudu-CR] WIP: Integration test for replay cache

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: WIP: Integration test for replay cache
..

WIP: Integration test for replay cache

This adds a new integration test for replay cache that writes to all
replicas of a tablet at the same time. This uses the fact that all replicas
must eventually agree, and once the result is stored in replay cache,
followers can also reply to rpc's, to store all the responses obtained from
all the replicas, for the same rpcs. This then proceeds to make sure the
responses are the same.

WIP because it needs cleanup and pulling of some suff to other patches.

Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
---
M src/kudu/consensus/consensus_peers.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/transactions/transaction_driver.cc
6 files changed, 212 insertions(+), 43 deletions(-)


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

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

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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 client, but adds a flag to allow enabling
it and enables it in all tests, by default. This allows to start adding exactly
once semantics to some RPCs and run tests without enabling it globally.

Note that the flag was added to env.cc as it had to be added somwhere that would
be close to a top level dependency (for instance, adding it to the rpc system
wouldn't allow to enable it in KuduTest).

Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
---
M src/kudu/rpc/retriable_rpc.h
M src/kudu/util/env.cc
M src/kudu/util/test_util.cc
3 files changed, 20 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77096be608afb31194f62f04a946bd3f42537a35
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the result tracker with writes

[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 (#7).

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 this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
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/service_if.cc
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_server.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.proto
M src/kudu/tserver/tserver_service.proto
26 files changed, 403 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/3449/7
-- 
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: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Avoid missing 'override' keyword warnings in raft consensus-test.cc

[David Ribeiro Alves (Code Review)]

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

Change subject: Avoid missing 'override' keyword warnings in 
raft_consensus-test.cc
..


Avoid missing 'override' keyword warnings in raft_consensus-test.cc

In this test we're using gmock and the compiler spews out warnings
that the mocked methods are missing the 'override' keyword.

This just makes it so that we ignore that warning in that file.

Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
Reviewed-on: http://gerrit.cloudera.org:8080/3289
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/consensus/CMakeLists.txt
1 file changed, 8 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9
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: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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 specificed _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.

A follow up patch tests this in integration with the client.

Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
---
M src/kudu/rpc/CMakeLists.txt
M src/kudu/rpc/inbound_call.cc
M src/kudu/rpc/protoc-gen-krpc.cc
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/service_if.cc
M src/kudu/rpc/service_if.h
8 files changed, 138 insertions(+), 42 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new patch set (#14).

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 497 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/3190/14
-- 
To view, visit http://gerrit.cloudera.org:8080/3190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new patch set (#2).

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 this patch doesn't include additional tests, which will
be pushed later, ad-hoc. The reasoning being that we'll benefit
from more coverage in the meanwhile.

Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
---
M src/kudu/client/batcher.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/service_if.cc
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_server.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.proto
M src/kudu/tserver/tserver_service.proto
26 files changed, 388 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/3449/2
-- 
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Remove the LocalConsensus implementation

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Remove the LocalConsensus implementation
..


Patch Set 6: Code-Review+2

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

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

[kudu-CR] Add request id and sequence number to the rpc request header

[David Ribeiro Alves (Code Review)]

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

Change subject: Add request id and sequence number to the rpc request header
..


Add request id and sequence number to the rpc request header

This adds a new (optional) field to RpcHeader: RequestIdPB.

This new field contains the information necessary to implement the "replay 
cache"
component, including the unique id of the client and the rpc's sequence number.
RequestIdPB also includes two additional "helper" fields, one to keep track of
number of attempts of a particular rpc, so that we can skip the replay cache
altogether in certain cases and one to let the server know that it can garbage
collect completion records.

Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703
Reviewed-on: http://gerrit.cloudera.org:8080/3079
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/rpc/rpc_header.proto
1 file changed, 22 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I30b07f070f8e80aaad982ccfe71ffb7e22a6a703
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Move the PbTracer in rpc_context.cc to pb_util

[David Ribeiro Alves (Code Review)]

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

Change subject: Move the PbTracer in rpc_context.cc to pb_util
..


Move the PbTracer in rpc_context.cc to pb_util

We'll need to use this outside of RpcContext so this moves
it to a place where it makes more sense and can be used
elsewhere.

Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335
Reviewed-on: http://gerrit.cloudera.org:8080/3194
Tested-by: David Ribeiro Alves 
Reviewed-by: Adar Dembo 
---
M src/kudu/rpc/rpc_context.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
3 files changed, 54 insertions(+), 36 deletions(-)

Approvals:
  David Ribeiro Alves: Verified
  Adar Dembo: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib601deea9fe0dcc6e2428416a6993c7567ff7335
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: Todd Lipcon 

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will addressing the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear waht it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
2 files changed, 347 insertions(+), 0 deletions(-)


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

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

[kudu-CR] Integrate the ResultTracker into the rpc subsystem and add a test

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Integrate the ResultTracker into the rpc subsystem and add a 
test
..

Integrate the ResultTracker into the rpc subsystem and add a test

This integrates the ResultTracker into the rpc subsystem by allowing
to specify a new mehtod option when defining rpc service methods.
When this method option is specificed _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.

Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
---
M src/kudu/common/common.proto
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/rtest.proto
M src/kudu/rpc/service_if.cc
M src/kudu/rpc/service_if.h
11 files changed, 321 insertions(+), 36 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a RpcContext::RespondFailure() method

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a RpcContext::RespondFailure() method
..

Add a RpcContext::RespondFailure() method

In the case of certain errors we use RpcContext::RespondSuccess() because
setting the error is call dependent and it was done on the call's request.
Since we'll be relying on the RpcContext to call the appropriate method
on the ResultTracker, on the common case, we'd still be marking these error
cases as successful RPCs whose results we keep. This is problematic for
various reasons, the main one being that if a client gets a transient error
back (e.g. the call was throttled) all subsequent attemtpts at the same
call will get the same result.

To avoid this we need to be able to flag that a call failed even if we don't
need anything else done to the response. To this goal this patch adds
RpcContext::RespondFailure(), which currently does nothing else besides
call RespondSuccess(), but will do so when the result tracker is integrated
in.

Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
---
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
2 files changed, 16 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a RpcContext::RespondFailure() method

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a RpcContext::RespondFailure() method
..

Add a RpcContext::RespondFailure() method

In the case of certain errors we use RpcContext::RespondSuccess() because
setting the error is call dependent and it was done on the call's request.
Since we'll be relying on the RpcContext to call the appropriate method
on the ResultTracker, on the common case, we'd still be marking these error
cases as successful RPCs whose results we keep. This is problematic for
various reasons, the main one being that if a client gets a transient error
back (e.g. the call was throttled) all subsequent attemtpts at the same
call will get the same result.

To avoid this we need to be able to flag that a call failed even if we don't
need anything else done to the response. To this goal this patch adds
RpcContext::RespondFailure(), which currently does nothing else besides
call RespondSuccess(), but will do so when the result tracker is integrated
in.

Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
---
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_context.h
2 files changed, 16 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add unique id generation to the client

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add unique id generation to the client
..


Patch Set 11:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3077/11/src/kudu/client/client_builder-internal.cc
File src/kudu/client/client_builder-internal.cc:

Line 26: default_rpc_timeout_(MonoDelta::FromSeconds(10)) {}
> Can you revert this change? It's just unnecessary noise.
Done


http://gerrit.cloudera.org:8080/#/c/3077/11/src/kudu/integration-tests/client-stress-test.cc
File src/kudu/integration-tests/client-stress-test.cc:

Line 42: namespace client {
> Hmm, while the test is called client-stress-test, it's part of the integrat
because I needed to make this test a friend class of client. I tried to make 
the friend class in client.h refer to this test with the kudu:: prefix, but to 
no avail. This is the only way I got it to work without exposing the client_id_ 
with an accessor.
Maybe I missed something. Ideas?


Line 279:  << total_num_rejections << " memory rejections";
> The old indentation was nicer, I think.
Done


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

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

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will addressing the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear waht it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 341 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Integrate the result tracker with writes

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 17:

(18 comments)

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

Line 51: #include "kudu/rpc/rpc_header.pb.h"
> unused?
Done


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

Line 182:   optional rpc.RequestIdPB request_id = 100;
> why using such a high id here? seems like it wastes a byte
the idea was that we could keep adding request types (like the optional below). 
changed this to be in sequence.


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

Line 226: bool final_driver = false;
> doc this
removed this (scaffolding code)


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

Line 251:  WriteResponsePB* response);
> mentioned on rev 1, but I dont like the side effect here without an accompa
Done


Line 1243:   if (tracking_results) {
> any way to extract this new bit of code to a separate function?
this is way simpler now with the atomic bit.  please take a new look.


Line 1279:   LOG(FATAL) << "Couldn't change bootstrapping txn to driver 
after 10 attempts for request: "
> this seems like a kind of arbitrary thing... should this be a DFATAL and ke
removed


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/tablet_peer.cc
File src/kudu/tablet/tablet_peer.cc:

Line 130: const scoped_refptr 
result_tracker,
> reference
Done


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

Line 212:   if (state()->are_results_tracked()) {
> change to if (!...) return
Done


Line 314: case REPLICATING:
> some spurious changes here
Done


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

PS17, Line 117: // 1 - When becoming leader, a replica has already prepared all 
the requests that it received
  : // from the previous leader that it will try to 
replicate/commit itself.
  : /
> not quite understanding this sentence
Done


PS17, Line 122: Requests originated on other replicas 
> not sure quite what you mean here. Do you mean operations which were starte
Done


PS17, Line 158: FailAndRespond() 
> hrm, is this FailAndRespond here actually responding to any RPCs in the cur
the design changed and the docs were not updated. change this to reflect the 
new state of affairs.


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

PS17, Line 196: c_
> typo
Done


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/remote_bootstrap_session-test.cc
File src/kudu/tserver/remote_bootstrap_session-test.cc:

Line 137:  dummy,
> can just use scoped_refptr(), no?
Done


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

Line 280: const char* ALREADY_PRESENT_ERROR = "There's already an attempt at 
the same operation "
> is this used?
Done


Line 295:   //LOG(INFO) << "Responding error to: " << 
context_->request_pb()->DebugString();
> remove
Done


Line 764:   // attempted elsewhere.
> this probably needs to be moved into tablet_peer
this now is done on the replicate message for all txns. removed


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

Line 123: const char* WRITE_METHOD_NAME = "Write";
> what's this used for? dont see it referenced elsewhere
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: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 25:

Todd: As we discussed I also changed the TrackRpc/ChangeDriver methods. I added 
a new TrackRpcUnlocked and a TrackRpcOrChangeDriver methods. The latter tracks 
the RPC or changes the driver, atomically.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
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: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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
---
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, 245 insertions(+), 6 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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
---
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, 388 insertions(+), 60 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Gerrit-PatchSet: 23
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Add a ResultTracker class that will track server side results
..

Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 599 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
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: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

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

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

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


Patch Set 17:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3080/17/src/kudu/rpc/retriable_rpc.h
File src/kudu/rpc/retriable_rpc.h:

Line 212:   // failure.
> something funny happened to a comment here
Done


http://gerrit.cloudera.org:8080/#/c/3080/17/src/kudu/rpc/rpc_controller.h
File src/kudu/rpc/rpc_controller.h:

Line 119:   // calling this method.
> I think this is more simply stated: REQUIRES: the controller has a request 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

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

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

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3506/8//COMMIT_MSG
Commit Message:

Line 14: Note that the flag was added to env.cc as it had to be added somwhere 
that would
> hrmmm... I'm not a fan of this.
changed the flag to be enabled server side and changed this too.


http://gerrit.cloudera.org:8080/#/c/3506/8/src/kudu/rpc/retriable_rpc.h
File src/kudu/rpc/retriable_rpc.h:

Line 193:   if (FLAGS_enable_exactly_once) {
> I think having the kill switch on the server side is preferable
Done


http://gerrit.cloudera.org:8080/#/c/3506/8/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:

Line 86:   FLAGS_enable_exactly_once = true;
> I don't think this is quite sufficient, because it won't enable it on exter
it was sufficient, since the flag was client side, but it isn't anymore. so 
added the flag where appropriate.


-- 
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: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a ResultTracker class that will track server side results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 22:

(2 comments)

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

Line 203:   // from before we reply to all the other ongoing ones.
> So its like this:
actually this is only important for the Fail* methods, but made sense to make 
iteration the same all over.


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

PS22, Line 44: client ID and same sequence number
> typo: same client ID and sequence number
Done


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

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

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

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

(8 comments)

i moved these to a new itest, also moved a couple of methods out of raft 
consensus itest to the parent class so that I can use them in the new test.

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 1043: RestartAnyCrashedTabletServers()
> This method needs a flag indicating whether servers are allowed to crash in
good catch. added the flag and also added an AssertNoTabletServersCrashed


Line 1047:   bool mismatched = false;
> Please add a comment here to summarize the purpose of this logic. It took m
Done


PS24, Line 1079: (1234
> s/1234/SeedRandom()/
this is on purpose so that all threads are seeded with the same random as they 
are supposed to generate the same rows. Will add a comment though


Line 1125: status = controller.status();
> Returning controller.status() is already what Write() does. See proxy.cc L9
Was mimicking the client code, but writes are async there. you're right. Done


Line 1141: FAIL() << "Couldn't write request to tablet server @ " << 
address.ToString();
> how about append the last status as well?
Done


Line 1181: TEST_F(RaftConsensusITest, 
TestWritesWithExactlyOnceSemanticsWithChurnyElections) {
> Needs comment
Done


PS24, Line 1184:  defined(__SANITIZE_ADDRESS__) || defined(ADDRESS_SANITIZER)
> we should only need one of these two, right?
gcc calls it one thing and clang calls it another. We've used the two elsewhere.


http://gerrit.cloudera.org:8080/#/c/3519/29/src/kudu/rpc/rpc_header.proto
File src/kudu/rpc/rpc_header.proto:

Line 130:   required int64 attempt_no = 4;
> Please squash this change into a previous commit, or squash this whole patc
this change is first needed in this patch. Usually attempt numbers never get 
this high, its just that the tests used very high numbers to make sure they are 
unique. Will mention this in the commit message.


-- 
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: 29
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

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

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded a new patch set (#30).

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/30
-- 
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: 30
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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

WIP: Add garbage collection to ResultTracker

This still needs testing and hooking up to the mm.

Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
---
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_header.proto
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/transactions/transaction_driver.cc
7 files changed, 162 insertions(+), 12 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] c++ client: various fixes to DDL operations

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: c++ client: various fixes to DDL operations
..


Patch Set 8: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I09768240bd04cca95d95aefe17c34d276075125b
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 
Gerrit-HasComments: No

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

[David Ribeiro Alves (Code Review)]

Hello Kudu Jenkins,

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

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

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

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

WIP: Add garbage collection to ResultTracker

This still needs testing and hooking up to the mm.

Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
---
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_header.proto
M src/kudu/rpc/service_if.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/transactions/transaction_driver.cc
7 files changed, 162 insertions(+), 12 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins

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

[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] Add integration tests for replay cache with writes

[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] Integrate the ResultTracker into the rpc subsystem

[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] Integrate the result tracker with writes

[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] Integrate the result tracker with writes

[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] Move the maintenance manager to util

[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] Disable exactly once semantics by default and add a flag to enable it for tests

[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] Add integration tests for replay cache with writes

[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] Memory tracking for result tracker

[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