Todd Lipcon has posted comments on this change. Change subject: log: refactor log reading into an iterator-like interface ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/3038/2/src/kudu/consensus/log_util.cc File src/kudu/consensus/log_util.cc: Line 187: return s.CloneAndPrepend(Substitute("Error reading from log $0", seg_->path_)); > Nit: lower-case for Error? This will probably be appended to another Status k, edited this one since i'm editing the code anyway Line 223: // TODO: can remove some params here? > What is this referring to? There's just the status param. yea, left over from an intermediate step of the refactoring, thanks Line 231: for (const auto& r : recent_entries_) { > It's a fair amount of work to maintain recent_entries_ and I see it's just yea, I agree it's a fair bit of work, but we added this a while back because it's super helpful when debugging various kinds of issues in log corruption/replay. One of those things that isn't needed 99% of the time, but the 1% of the time when you need it, it's super helpful, so I'm inclined to keep it. http://gerrit.cloudera.org:8080/#/c/3038/2/src/kudu/consensus/log_util.h File src/kudu/consensus/log_util.h: Line 130: int64_t batches_read_; > Nit: num_batches_read_ Done Line 201: std::unique_ptr<LogEntryReader> NewLogEntryReader(); > Curious why you picked this approach to constructing a LogEntryReader inste Done -- To view, visit http://gerrit.cloudera.org:8080/3038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5727951664600b7a591a6c892ea1e62469ceb109 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
