Andrew Wong has posted comments on this change.

Change subject: KUDU-1952 Remove round-robin for block placement
......................................................................


Patch Set 47:

(3 comments)

@dralves: this patch doesn't have that sort of test. Will add in a follow-up

http://gerrit.cloudera.org:8080/#/c/6636/40/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:

PS40, Line 239: bool GetDataDirGroupPB(const std::string& tablet_id, 
DataDirGroupPB* pb) const;
> why not return Status::NotFound()?
In a couple cases, this is used to check whether a tablet is registered (in 
which case there's no need to create a new group), so it's not necessarily an 
error for false to be returned.

That said, I removed the PREDICT_ from the return.


http://gerrit.cloudera.org:8080/#/c/6636/46/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

PS46, Line 388: 1.5.0
> you say 1.4.0 somewhere else
Done. Changed the other to 1.5.0


http://gerrit.cloudera.org:8080/#/c/6636/46/src/kudu/util/pb_util.proto
File src/kudu/util/pb_util.proto:

PS46, Line 45: }
> nit: what changed here?
Untracked this file, seems like git was not adding a proper newline at the end 
of the file.


-- 
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: 47
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to