Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12119 )
Change subject: [blog] a blogpost about location awareness in Kudu ...................................................................... Patch Set 1: (46 comments) Gerrit gets funny when there's a lot of comments on one file, it seems, so I'm posting this first batch and I will continue reviewing later. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md File _posts/2018-12-20-location-awareness.md: http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@7 PS1, Line 7: a high-level overview An overview is always high-level, so remove "high-level". http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@9 PS1, Line 9: principles behind the design decisions principles of the design http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@11 PS1, Line 11: ways how can be extended and enhanced in the future potential future enhancements and extensions http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@17 PS1, Line 17: the Remove. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@17 PS1, Line 17: to fit for http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@20 PS1, Line 20: data replicas http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@20 PS1, Line 20: Having servers in multiple racks in a Kudu cluster In a Kudu cluster consisting of multiple servers spread over several racks http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@21 PS1, Line 21: 's data Remove. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@21 PS1, Line 21: replicated Remove. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@21 PS1, Line 21: in http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@22 PS1, Line 22: becomes become http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@21 PS1, Line 21: for : reading and writing Remove. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@22 PS1, Line 22: if any subset of servers even if all the servers http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@25 PS1, Line 25: In other words, a single rack failure should not lead to data unavailability : in a Kudu cluster. Remove (redundant). http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@28 PS1, Line 28: In that sense, a rack is a fault domain (usually, a fault domain : defined is a set of hardware components that share a single point of failure). I don't think we need to use this concept very much so I wouldn't include it. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@31 PS1, Line 31: To generalize, replace 'rack' with any other physical aggregation of nodes Join this paragraph with the previous paragraph. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@31 PS1, Line 31: To generalize nit: I prefer "More generally", but "To generalize" is ok. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@31 PS1, Line 31: physical Remove. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@32 PS1, Line 32: chassis Is a chassis a common term for something multiple servers would run on? To me it sounds synonymous with a single physical host, unless the cluster is running in VMs and some VMs share a physical host. If that's the case, I think we ought to say "shared VM host" or some such instead of "chassis". http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@32 PS1, Line 32: site Does a site differ from a datacenter? http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@33 PS1, Line 33: It might be applicable even to a datacenter : given that the network latency between datacenters is negligible : (which, frankly, isn't quite realistic). Rephrase this more positively: "This even applies to a datacenter if the network latency between datacenters is low." http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@37 PS1, Line 37: At this point, the location awareness feature is a work in progress and may be : released with Kudu 1.9.0. I'm not sure what our publishing schedule is, but I think we ought not publish this until the feature is sure to be released with the next release and the next release is coming in the next month or two. In that case, we shouldn't include this sentence, and we should mention the release the feature will first appear in above the fold. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@49 PS1, Line 49: exploiting to exploit http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@49 PS1, Line 49: for to establish http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@50 PS1, Line 50: other components of the Hadoop ecosystem (e.g., HDFS) I think it's just HDFS. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@57 PS1, Line 57: location a location http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@57 PS1, Line 57: leader master Actually all masters assign a location, but it's the leader master's location that is used in placement decisions, etc. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@57 PS1, Line 57: for to http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@57 PS1, Line 57: Kudu The http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@57 PS1, Line 57: upon its registration when it registers http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@58 PS1, Line 58: being registered Remove. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@58 PS1, Line 58: location the location http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@59 PS1, Line 59: the http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@60 PS1, Line 60: for the specified IP address/hostname Remove. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@60 PS1, Line 60: a the corresponding http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@62 PS1, Line 62: next registration of the tablet server the tablet server re-registers, which only occurs if the master or tablet server restarts. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@62 PS1, Line 62: The assigned : location is not reported back to the tablet server since Kudu tablet servers : are location-agnostic as of now nit: I would rephrase as "Kudu tablet servers are location agnostic, at least for now, so the assigned location is not reported back to the tablet server. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@64 PS1, Line 64: : masters . Masters http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@65 PS1, Line 65: internally Explain how, briefly (maybe by just pointing the reader to the section below). http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@65 PS1, Line 65: provide connected clients with that information as well Ditto. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@67 PS1, Line 67: Kudu leader master assigns location for a client upon connecting to : a cluster (i.e. when the client invokes the `ConnectToMaster` RPC). : The assigned location is sent back to the client along with other : information in response to `ConnectToMaster` RPC. I think this is too detailed, but if you want to include it mention the `ConnectToMaster` RPCs above when you first introduce how tablet servers are assigned location when they register. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@73 PS1, Line 73: `--location_mapping_cmd` master's Swap these two words. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@76 PS1, Line 76: Does this paragraph belong in the next section? It repeats part of it. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@84 PS1, Line 84: 3 (three) My preference is to use the word "three", not the symbol "3", but either is OK. Putting both is unnecessary, I think. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@97 PS1, Line 97: By design, keeping the target replication factor for tablets has higher I'd first restate that re-replication and the placement of new replicas attempts to spread replicas across locations so that the failure of tablet servers in one location does not make tablets unavailable. http://gerrit.cloudera.org:8080/#/c/12119/1/_posts/2018-12-20-location-awareness.md@113 PS1, Line 113: is are -- To view, visit http://gerrit.cloudera.org:8080/12119 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I10b30a80d5661fb889a11285b8118cdea1a87cd2 Gerrit-Change-Number: 12119 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Wed, 26 Dec 2018 16:49:48 +0000 Gerrit-HasComments: Yes
