[kudu-CR] KUDU-78. Fix pb util functions which return bool to return Status or void
Todd Lipcon has submitted this change and it was merged. Change subject: KUDU-78. Fix pb_util functions which return bool to return Status or void .. KUDU-78. Fix pb_util functions which return bool to return Status or void As suggested in ticket, I've refactored four pb_util functions returning bool. As it turns out, three of them should never fail (except with CHECKS), so I've changed these to return void and added one missing check. The return type change was propogated to callers as well: if any of them can't fail anymore it's also refactored to return void. The fourth one (ParseFromSequentialFile) can actually fail in two ways: either with IO error or with Protobuf error (Status::Corruption). To track which one, I had to add GetStatus() method into SequentialFileFileInputStream. Change-Id: Ib760793f6e6da3e357573e525f47b32c79472468 Reviewed-on: http://gerrit.cloudera.org:8080/4800 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/cfile/bloomfile.cc M src/kudu/cfile/cfile_writer.cc M src/kudu/consensus/log.cc M src/kudu/consensus/log.h M src/kudu/consensus/log_util.cc M src/kudu/consensus/mt-log-test.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tablet/delta_compaction-test.cc M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_store.cc M src/kudu/tablet/deltafile-test.cc M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/deltamemstore.cc M src/kudu/util/pb_util-internal.h M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 18 files changed, 87 insertions(+), 125 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib760793f6e6da3e357573e525f47b32c79472468 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Maxim Smyatkin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Maxim Smyatkin Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-78. Fix pb util functions which return bool to return Status or void
Maxim Smyatkin has posted comments on this change. Change subject: KUDU-78. Fix pb_util functions which return bool to return Status or void .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4800/3//COMMIT_MSG Commit Message: PS3, Line 7: Status > Nit: Do you still want to keep the commit header as Status or change it as Yes, probably should be updated -- To view, visit http://gerrit.cloudera.org:8080/4800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib760793f6e6da3e357573e525f47b32c79472468 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Maxim SmyatkinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Maxim Smyatkin Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-78. Fix pb util functions which return bool to return Status or void
Hello Dinesh Bhat, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4800 to look at the new patch set (#4). Change subject: KUDU-78. Fix pb_util functions which return bool to return Status or void .. KUDU-78. Fix pb_util functions which return bool to return Status or void As suggested in ticket, I've refactored four pb_util functions returning bool. As it turns out, three of them should never fail (except with CHECKS), so I've changed these to return void and added one missing check. The return type change was propogated to callers as well: if any of them can't fail anymore it's also refactored to return void. The fourth one (ParseFromSequentialFile) can actually fail in two ways: either with IO error or with Protobuf error (Status::Corruption). To track which one, I had to add GetStatus() method into SequentialFileFileInputStream. Change-Id: Ib760793f6e6da3e357573e525f47b32c79472468 --- M src/kudu/cfile/bloomfile.cc M src/kudu/cfile/cfile_writer.cc M src/kudu/consensus/log.cc M src/kudu/consensus/log.h M src/kudu/consensus/log_util.cc M src/kudu/consensus/mt-log-test.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tablet/delta_compaction-test.cc M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_store.cc M src/kudu/tablet/deltafile-test.cc M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/deltamemstore.cc M src/kudu/util/pb_util-internal.h M src/kudu/util/pb_util.cc M src/kudu/util/pb_util.h 18 files changed, 87 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/4800/4 -- To view, visit http://gerrit.cloudera.org:8080/4800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib760793f6e6da3e357573e525f47b32c79472468 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Maxim SmyatkinGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Maxim Smyatkin Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon