Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6825
to look at the new patch set (#3).
Change subject: log block manager: reopen container metadata writers at the OS
level
......................................................................
log block manager: reopen container metadata writers at the OS level
Another patch I'm working on compacts LBM metadata files at startup by
writing compacted metadata into a temporary file, then overwriting the
existing metadata file with it. For this to work properly, "reopening" a
metadata file should actually reopen the file on the filesystem. This patch
does just that.
There should be no impact on any existing code; the reopening done after
truncating a partial record from a metadata file is just as happy if done at
the logical level (before) as at the physical level (now).
By popular demand, I've attached below a long explanation of Kudu's approach
to object initialization and thread safety that came up during code review.
Because we don't use exceptions within Kudu, the initialization of an object
is typically done in two phases:
1. Constructor, for operations that cannot fail.
2. Initializer function, for operations that may fail.
In most objects, the initializer function is named Init(), returns a Status
type, and needn't be thread safe because it is called right after the
constructor, before the object is "made public" to other threads.
Some objects offer two initializer functions: one to initialize a brand new
object and one to initialize an object with existing state. The idea is that
callers always use the constructor and then call one of the two initializer
functions, depending on their use case. WritablePBContainerFile is one such
object; Init() is for a brand new container file, and Open() is for a
container file that already exists on disk.
Previously, Reopen() served double duty: it was both the initializer
function for files with existing state, and it was an arbitrary function
(like Append()) that could be called at any time and by any thread. That
second responsibility is why it was thread safe. Now, OpenExisting() is just
the equivalent of CreateNew(), and so it makes sense for it to be just as
thread unsafe as CreateNew() is.
To be clear, the LBM was the only "arbitrary" caller to Reopen(), and the
LBM didn't _need_ it to be thread safe. But, I think it was net less
confusing for Reopen() to be thread safe (and thus equivalent in semantics
to Append()) than for it to be thread unsafe (and thus exceptional when
compared to Append(), Sync(), etc.).
Change-Id: I6029d499422f5a2db036d796c7e208f2af71c394
---
M src/kudu/fs/log_block_manager-test-util.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/util/pb_util-test.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
6 files changed, 65 insertions(+), 54 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6825/3
--
To view, visit http://gerrit.cloudera.org:8080/6825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6029d499422f5a2db036d796c7e208f2af71c394
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
