You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/05/16 21:27:25 UTC
[3/5] incubator-kudu git commit: bootstrap: use the iterator
interface to read the log
bootstrap: use the iterator interface to read the log
This uses the new log entry reader interface when replaying the log
during tablet bootstrap. Per the previous commit message, this makes
more effective use of allocator and CPU caches, so provides a bit of
a speed-up.
Before:
Time spent bootstrapping tablet: real 3.825s user 1.448s sys 1.000s
Time spent bootstrapping tablet: real 4.340s user 1.480s sys 0.988s
Time spent bootstrapping tablet: real 7.438s user 1.456s sys 1.080s
After:
Time spent bootstrapping tablet: real 5.563s user 1.092s sys 1.172s
Time spent bootstrapping tablet: real 5.931s user 1.172s sys 0.976s
Time spent bootstrapping tablet: real 8.050s user 1.188s sys 1.192s
(ignore the 'real' time here, which is mostly based on the inconsistent IO
performance of my laptop SSD, and instead focus on the 'user' time)
CPU time is reduced by about a third. In multithreaded bootstrap scenarios,
this is likely to have a more dramatic impact by reducing contention on the
allocator's central free list.
Change-Id: I31075a887d591071386c448d6e8769f92238bb07
Reviewed-on: http://gerrit.cloudera.org:8080/3039
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/d138dba6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/d138dba6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/d138dba6
Branch: refs/heads/master
Commit: d138dba61c2a889fde4c0467d5429a671fb950f9
Parents: f8370ef
Author: Todd Lipcon <to...@apache.org>
Authored: Wed May 11 16:03:17 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Mon May 16 21:26:47 2016 +0000
----------------------------------------------------------------------
src/kudu/tablet/tablet_bootstrap.cc | 52 +++++++++++++++-----------------
1 file changed, 25 insertions(+), 27 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/d138dba6/src/kudu/tablet/tablet_bootstrap.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_bootstrap.cc b/src/kudu/tablet/tablet_bootstrap.cc
index 7e05b62..f63143b 100644
--- a/src/kudu/tablet/tablet_bootstrap.cc
+++ b/src/kudu/tablet/tablet_bootstrap.cc
@@ -102,6 +102,7 @@ using server::Clock;
using std::map;
using std::shared_ptr;
using std::string;
+using std::unique_ptr;
using std::unordered_map;
using strings::Substitute;
using tserver::AlterSchemaRequestPB;
@@ -1032,41 +1033,38 @@ Status TabletBootstrap::PlaySegments(ConsensusBootstrapInfo* consensus_info) {
int segment_count = 0;
for (const scoped_refptr<ReadableLogSegment>& segment : segments) {
- vector<LogEntryPB*> entries;
- ElementDeleter deleter(&entries);
- // TODO: Optimize this to not read the whole thing into memory?
- Status read_status = segment->ReadEntries(&entries);
- for (int entry_idx = 0; entry_idx < entries.size(); ++entry_idx) {
- LogEntryPB* entry = entries[entry_idx];
- Status s = HandleEntry(&state, entry);
+ log::LogEntryReader reader(segment.get());
+
+ int entry_count = 0;
+ while (true) {
+ unique_ptr<LogEntryPB> entry(new LogEntryPB);
+
+ Status s = reader.ReadNextEntry(entry.get());
+ if (PREDICT_FALSE(!s.ok())) {
+ if (s.IsEndOfFile()) {
+ break;
+ }
+ return Status::Corruption(Substitute("Error reading Log Segment of tablet $0: $1 "
+ "(Read up to entry $2 of segment $3, in path $4)",
+ tablet_->tablet_id(),
+ s.ToString(),
+ entry_count,
+ segment->header().sequence_number(),
+ segment->path()));
+ }
+ entry_count++;
+
+ s = HandleEntry(&state, entry.get());
if (!s.ok()) {
DumpReplayStateToLog(state);
RETURN_NOT_OK_PREPEND(s, DebugInfo(tablet_->tablet_id(),
segment->header().sequence_number(),
- entry_idx, segment->path(),
+ entry_count, segment->path(),
*entry));
}
-
// If HandleEntry returns OK, then it has taken ownership of the entry.
- // So, we have to remove it from the entries vector to avoid it getting
- // freed by ElementDeleter.
- entries[entry_idx] = nullptr;
- }
-
- // If the LogReader failed to read for some reason, we'll still try to
- // replay as many entries as possible, and then fail with Corruption.
- // TODO: this is sort of scary -- why doesn't LogReader expose an
- // entry-by-entry iterator-like API instead? Seems better to avoid
- // exposing the idea of segments to callers.
- if (PREDICT_FALSE(!read_status.ok())) {
- return Status::Corruption(Substitute("Error reading Log Segment of tablet $0: $1 "
- "(Read up to entry $2 of segment $3, in path $4)",
- tablet_->tablet_id(),
- read_status.ToString(),
- entries.size(),
- segment->header().sequence_number(),
- segment->path()));
+ entry.release();
}
// TODO: could be more granular here and log during the segments as well,