Todd Lipcon has posted comments on this change.

Change subject: Add time/watermark based garbage collection to ResultTracker

Patch Set 17:

File src/kudu/rpc/CMakeLists.txt:

Line 121: ADD_KUDU_TEST(exactly_once_rpc-test)
> Can you resort the tests?
File src/kudu/rpc/

PS17, Line 152:         completion_record->state = RpcState::IN_PROGRESS;
              :         completion_record->driver_attempt_no = 
> Can we add these to the CR constructor?

Line 232:   completion_record->last_updated = MonoTime::Now();
> Technically we haven't mutated the CR, so why update its timestamp?
yea, not sure, will remove.

Line 348:   completion_record->last_updated = MonoTime::Now();
> Likewise, shouldn't we push this down into where we actually update the CR?
not convinced of this one -- if we moved it inside the loop, it might get 
updated multiple times, and no sense in wasting the extra clock calls.
File src/kudu/tablet/

PS17, Line 1247:     // We only replay committed requests so the result of 
tracking this request can be:
               :     // NEW - This is a previously untracked request, or we 
changed the driver -> store the result
               :     // COMPLETED - We've bootstrapped this tablet twice, and 
previously stored the result -> do
               :     //             nothing.
> Update this comment.
File src/kudu/tablet/transactions/

PS17, Line 227:       VLOG(1) << state()->result_tracker() << " Follower Rpc 
was not NEW or IN_PROGRESS: "
              :           << rpc_state << " OpId: " << 
              :           << " RequestId: " << 
> Update this too? Should have been "not NEW or COMPLETED", I think.
done. also changed to VLOG(2) since this is quite internal

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Alexey Serbin <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: David Ribeiro Alves <>
Gerrit-Reviewer: Jean-Daniel Cryans <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to