Anupama Gupta has posted comments on this change. ( http://gerrit.cloudera.org:8080/10505 )
Change subject: KUDU-702 Add block IDs to more log messages ...................................................................... Patch Set 4: (4 comments) Please review for submission. http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/cfile_reader.cc File src/kudu/cfile/cfile_reader.cc: http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/cfile_reader.cc@491 PS1, Line 491: LOG(WARNING) << "Unable to validate compressed block " << block_id().ToString() : << " at " << ptr.offset() << " of size " << block.size() << ": " : << s.ToString(); : return s; > Nit: the ordering here seems off to me, I think we should first identify th Done http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/index_btree.cc File src/kudu/cfile/index_btree.cc: http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/cfile/index_btree.cc@109 PS1, Line 109: return s.CloneAndPrepend("Unable to flush root index block"); > This is not likely to work as intended because a non-OK Status was returned Done http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/fs/log_block_manager.cc@2041 PS1, Line 2041: VLOG(2) << Substitute("Added block: id $0, offset $1, length $2", > nit: for consistency i have the same suggestion for ordering in this file ( Done http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/tserver/tablet_copy_source_session.cc File src/kudu/tserver/tablet_copy_source_session.cc: http://gerrit.cloudera.org:8080/#/c/10505/1/src/kudu/tserver/tablet_copy_source_session.cc@395 PS1, Line 395: LOG(DFATAL) << "Data block disappeared: " << s.ToString(); > This isn't necessary because we already included it in the error message on Done -- To view, visit http://gerrit.cloudera.org:8080/10505 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I895da1cc04ecbf006f412f06b31461b03072d32d Gerrit-Change-Number: 10505 Gerrit-PatchSet: 4 Gerrit-Owner: Anupama Gupta <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Anupama Gupta <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Sat, 26 May 2018 03:44:16 +0000 Gerrit-HasComments: Yes
