Alexey Serbin has posted comments on this change. Change subject: [catalog_manager] categorization of rw operation failures ......................................................................
Patch Set 26: (5 comments) http://gerrit.cloudera.org:8080/#/c/6170/26/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: PS26, Line 471: const > Why not? And why Status is so special? ok, I think your idea of following the same style here makes sense. PS26, Line 484: const > Probably, const Status& s = ...; would be more preferable? ok, agreed. PS26, Line 762: const > Yep, I just want to clarify on why it's not acceptable. Keeping the same style makes sense. However, in this case I would prefer to keep the const modifier. The reason is that this method returns that value in the very end, and for the reader it's good to have an easy way to know that's the exact value captured in the very beginning of the function. If you feel strongly opposed to this, I'll update it removing the const modifier. But I hope you will also find it's better from the readability standpoint. PS26, Line 3231: LOG_WITH_PREFIX(INFO) : << "Latest config for has opid_index of " << latest_index : << " while this task has opid_index of " : << cstate_.config().opid_index() << ". Aborting task."; > spurious reformat Done PS26, Line 3285: LOG_WITH_PREFIX(WARNING) : << "ChangeConfig() failed with leader " : << target_ts_desc_->ToString() : << " due to CAS failure. No further retry: " : << status.ToString(); : MarkFailed(); : break; : default: : LOG_WITH_PREFIX(INFO) : << "ChangeConfig() failed with leader " : << target_ts_desc_->ToString() : << " due to error " : << TabletServerErrorPB::Code_Name(resp_.error().code()) : << ". This operation will be retried. Error detail: " : << status.ToString(); : break; > spurious reformat Done -- To view, visit http://gerrit.cloudera.org:8080/6170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8 Gerrit-PatchSet: 26 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
