Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong,

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14819

to look at the new patch set (#6).

Change subject: log: remove SegmentAllocator functors
......................................................................

log: remove SegmentAllocator functors

When we roll the log, we close the active segment before switching to the
next preallocated segment. What happens when we close the active segment?
1. We sync it.
2. We write out the (now complete) footer.
3. We close the segment file. Under the hood, this will truncate any unused
   preallocated space out of the file and sync it one last time.
4. We open the closed segment file for reading. Note that this is the second
   time we've opened it; we also opened it once when we last rolled the log.
5. We create a new in-memory segment reader. To avoid needless IO, we
   initialize it using the in-memory header and footer, but we do end up
   reloading the segment's file size from disk.
6. We replace the last segment reader in the log with the reader constructed
   in step #5. Side effect: the original segment reader is now closed.

My original intent with this patch was to replace steps 4-6 with a simpler
"inject the footer into the existing in-memory segment reader". This proved
to be impossible without adding a new synchronization primitive to protect
the footer in the segment reader, which is something I wanted to avoid. And
the win would have been minimal: with proper file cache usage, step 4 is
likely to be a cache hit.

So now this patch just cleans up SegmentAllocator by removing its functors,
which I found difficult to follow. Instead, Log now manipulates LogReader
directly, using information passed back from SegmentAllocator functions.

Change-Id: Id2f5dd47e3be930288dcfeb1f77409fda45daac4
---
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/log_reader.h
M src/kudu/consensus/log_util.cc
M src/kudu/consensus/log_util.h
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tserver/tablet_copy-test-base.h
12 files changed, 211 insertions(+), 190 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/14819/6
--
To view, visit http://gerrit.cloudera.org:8080/14819
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id2f5dd47e3be930288dcfeb1f77409fda45daac4
Gerrit-Change-Number: 14819
Gerrit-PatchSet: 6
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

Reply via email to