Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12075 )
Change subject: KUDU-2636: LBM supports deleting dead containers ...................................................................... Patch Set 14: (2 comments) http://gerrit.cloudera.org:8080/#/c/12075/12/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: http://gerrit.cloudera.org:8080/#/c/12075/12/src/kudu/fs/log_block_manager.cc@760 PS12, Line 760: { : uint64_t metadata_size = 0 > The motivation here is to support deleting incomplete files directly, other Oh, I think I get you now. It took a second reading of the code for me to spot the subtle difference: if one container file is missing, we'll delete the other file, even if it has a non-zero file size. Previously we would only delete the other file if it was empty (data file) or under the minimum length (metadata file). Right, it's possible for an ill-timed host crash to leave behind a non-empty data file (containing for example one block) and either an empty or non-existent metadata file. The concern though is, if some other event caused one of the data or metadata file to disappear, by deleting the other file we'd be making manual recovery impossible. Meaning, Kudu's well-intentioned action (to automate recovery by deleting the other container file) could in fact be leading to real data loss. I'm open to being convinced otherwise, but that's the position I currently have on this issue. http://gerrit.cloudera.org:8080/#/c/12075/12/src/kudu/fs/log_block_manager.cc@2366 PS12, Line 2366: } else if (static_cast<double>(container->live_blocks()) / : container->total_blocks() <= FLAGS_log_container_live_metadata_before_compact_ratio) { : // Metadata files of containers with very few live blocks will be compacte > Done I actually think we should keep this TODO as-is. Yes, we support container deletion at realtime now, but this change isn't actually addressing the TODO, so the request in it is still valid and worth preserving. -- To view, visit http://gerrit.cloudera.org:8080/12075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7d3672ae3c3dd9acd489120d653c44a641537f10 Gerrit-Change-Number: 12075 Gerrit-PatchSet: 14 Gerrit-Owner: helifu <hzhel...@corp.netease.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: helifu <hzhel...@corp.netease.com> Gerrit-Comment-Date: Thu, 10 Jan 2019 21:51:54 +0000 Gerrit-HasComments: Yes
