Alexey Serbin has posted comments on this change. Change subject: [catalog_manager] categorization of rw operation failures ......................................................................
Patch Set 8: (10 comments) http://gerrit.cloudera.org:8080/#/c/6170/8//COMMIT_MSG Commit Message: PS8, Line 10: d/write operation failures happened while executing the leader : post-election callback > sentence fragment Done PS8, Line 14: / > Make it explicit whether this means 'and' or 'or'. Done http://gerrit.cloudera.org:8080/#/c/6170/8/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 463: const Status& s = catalog_manager_->ProcessPendingAssignments(to_process); > Would you mind making this an owned Status here and below? I find the by-v Done PS8, Line 763: found > is found Done PS8, Line 770: > the master Done PS8, Line 774: already : // contain CA certificate information written by the new leader master > contain the CA certificate information when this master is re-elected next. Done Line 790: return s; > I'm guessing this is an unnecessary copy because the status is kept as a re That's a good point. PS8, Line 911: const Consensus& consensus > this could just be captured. If you don't mind, I would prefer to leave it as it is for better modularity. Line 914: const int64_t term_pre = > Why not use 'term'? Because there is pairing 'term_post'. I updated this code so this is no longer an issue. Line 938: LOG(WARNING) << op_description << " failed; " > This is logged at INFO level on line 899, why WARNING here? It's in the context of the 'if (!s.ok())' path -- some operation has failed. But since you noticed this, I think it's better to change it to INFO as well because the failed operation means we are not using that anyway and some other master server picked this up. -- 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: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[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
