David Ribeiro Alves has posted comments on this change. Change subject: KUDU-798 (part 5) Correct safe time advancement ......................................................................
Patch Set 28: (12 comments) http://gerrit.cloudera.org:8080/#/c/5240/27//COMMIT_MSG Commit Message: PS27, Line 15: n > nit: add comma or period? Done PS27, Line 15: ins > its Done http://gerrit.cloudera.org:8080/#/c/5240/27/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: Line 450: // If we're not sending ops to the follower, set the safe time on the request. > worth a TODO here that we could also set it if we are sending our latest op Done http://gerrit.cloudera.org:8080/#/c/5240/27/src/kudu/consensus/consensus_queue.h File src/kudu/consensus/consensus_queue.h: Line 34: //#include "kudu/consensus/time_manager.h" > nit Done http://gerrit.cloudera.org:8080/#/c/5240/28/src/kudu/consensus/time_manager.cc File src/kudu/consensus/time_manager.cc: PS28, Line 184: // Pre-flight checks, make sure we've heard from the leader recently and that safe time : // isn't lagging too much. > maybe we should do a GetSafeTime() call here so that if we're leader we 're Done http://gerrit.cloudera.org:8080/#/c/5240/28/src/kudu/consensus/time_manager.h File src/kudu/consensus/time_manager.h: Line 145: void MakeWaiterTimeoutMessageUnlocked(Timestamp timestmap, std::string* error_message); > warning: function 'kudu::consensus::TimeManager::MakeWaiterTimeoutMessageUn Done http://gerrit.cloudera.org:8080/#/c/5240/28/src/kudu/integration-tests/consistency-itest.cc File src/kudu/integration-tests/consistency-itest.cc: PS28, Line 639: 3 > What is the reason to have 3 rows inserted instead of 1? It would be nice Think was leftover from a previous patch. removed http://gerrit.cloudera.org:8080/#/c/5240/27/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: Line 37: #include "kudu/consensus/time_manager.h" > needed? Done http://gerrit.cloudera.org:8080/#/c/5240/27/src/kudu/tablet/tablet_peer_mm_ops.cc File src/kudu/tablet/tablet_peer_mm_ops.cc: Line 26: #include "kudu/consensus/time_manager.h" > unnecessary Done http://gerrit.cloudera.org:8080/#/c/5240/27/src/kudu/tablet/transactions/alter_schema_transaction.cc File src/kudu/tablet/transactions/alter_schema_transaction.cc: Line 23: #include "kudu/consensus/time_manager.h" > unnecessary Done http://gerrit.cloudera.org:8080/#/c/5240/27/src/kudu/tablet/transactions/transaction_tracker.cc File src/kudu/tablet/transactions/transaction_tracker.cc: Line 24: #include "kudu/consensus/time_manager.h" > unnecessary Done http://gerrit.cloudera.org:8080/#/c/5240/27/src/kudu/tablet/transactions/write_transaction.cc File src/kudu/tablet/transactions/write_transaction.cc: Line 25: #include "kudu/consensus/time_manager.h" > same Done -- To view, visit http://gerrit.cloudera.org:8080/5240 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8532fdb069c8bee7f3e08ffe74cab0273885cc8e Gerrit-PatchSet: 28 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
