Todd Lipcon has posted comments on this change. Change subject: KUDU-1952 Improve block placement ......................................................................
Patch Set 1: (3 comments) Just took a quick look at the commit message and new configs, not too much on the code structure, since Adar said he'd be reviewing the code. http://gerrit.cloudera.org:8080/#/c/6636/1//COMMIT_MSG Commit Message: Line 10: single-disk failure. Throughout the code, the term "DataDir" refers to would be good to link to the design doc somewhere here PS1, Line 14: This patch adds a mapping from tablet to a set of disks and uses it to : replace the existing round-robin placement of blocks. Tablets are : m it would be good in the commit message to explain the impact of this patch. With just this patch, and no follow-up work, is there a negative impact here? e.g. are we now limiting tablets to 3 disks, but still not actually able to recover from a wider variety of failures? If so, we should probably have a flag to disable the new behavior so that our trunk remains non-regressive from the most recent release. Also curious about upgrade impact here. http://gerrit.cloudera.org:8080/#/c/6636/1/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: Line 57: "Indicates the target number of data dirs to spread each " what's the behavior here if the configuration is larger than the number of data dirs? Similar to one of my top-level comments on the commit message: I'm afraid this is currently adding additional placement restrictions but not achieving the recovery benefits, so we should probably have this off for now -- To view, visit http://gerrit.cloudera.org:8080/6636 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
