You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2019/11/02 02:09:48 UTC

[kudu-CR] log: separate out allocation logic

Hello Adar Dembo, 

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

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

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

Change subject: log: separate out allocation logic
......................................................................

log: separate out allocation logic

The Log class has a few responsibilities:
1. adding batches to an in-memory queue asynchronously
2. picking off batches from the queue and writing them to disk
3. pre-allocating files in the background when a log segment grows too
   large
4. "rolling" onto a newly pre-allocated file once it is fully allocated
5. garbage collecting old log segments
6. a myriad of other bookkeeping related to indexing and size

1 and 2 have been neatly encapsulated in the Log::AppendThread subclass,
leaving the rest as the monolith that is the Log class.

This patch separates out 3 and 4 into a separate SegmentAllocator class.

This meant a few things:
- I've pushed the schema into the SegmentAllocator. This makes sense
  because the schema is only really needed by the WAL so it can write
  out the current schema of each segment. As such, only the entity that
  manages segment allocation ought to know about it.
- Many of the required Log members are put into a new LogContext,
  containing tablet ID, metrics, etc. We could also use this in the
  AppendThread if we wanted to, but I opted to keep this patch focused
  on the SegmentAllocator.
- The SegmentAllocator needs to call back into the Log in order to
  update the LogReader when it rolls over. Since the LogReader is
  protected by a lock in the Log, rather than providing a back-pointer
  to the Log class or the LogReader class, this passes thread-safe Log
  function calls into the SegmentAllocator.

I tried to keep the SegmentAllocator methods in the same locations as
their Log predecessors to keep the diff small. I'll put them closer
together in a follow-up patch.

Change-Id: I08be4dbdd8e98b02278de76273e931c314b08161
---
M src/kudu/consensus/log-test.cc
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tserver/tablet_server-test.cc
5 files changed, 479 insertions(+), 378 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/14479/5
-- 
To view, visit http://gerrit.cloudera.org:8080/14479
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I08be4dbdd8e98b02278de76273e931c314b08161
Gerrit-Change-Number: 14479
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>