facebook
Refactor to avoid confusing "raw block"
Summary: We have a lot of confusing code because of mixed, sometimes
completely opposite uses of of the term "raw block" or "raw contents",
sometimes within the same source file. For example, in BlockBasedTableBuilder,
raw_block_contents and raw_size generally referred to uncompressed block
contents and size, while WriteRawBlock referred to writing a block that
is already compressed if it is going to be. Meanwhile, in
BlockBasedTable, raw_block_contents either referred to a (maybe
compressed) block with trailer, or a maybe compressed block maybe
without trailer. (Note: left as follow-up work to use C++ typing to
better sort out the various kinds of BlockContents.)
This change primarily tries to apply some consistent terminology around the kinds of block representations, avoiding the unclear "raw". (Any meaning of "raw" assumes some bias toward the storage layer or toward the logical data layer.) Preferred terminology:
- Serialized block - bytes that go into storage. For block-based table
(usually the case) this includes the block trailer. WART: block
sizemay or may not include the trailer; need to be clear about whether it does or not. - Maybe compressed block - like a serialized block, but without the trailer (or no promise of including a trailer). Must be accompanied by a CompressionType.
- Uncompressed block - "payload" bytes that are either stored with no compression, used as input to compression function, or result of decompression function.
- Parsed block - an in-memory form of a block in block cache, as it is used by the table reader. Different C++ types are used depending on the block type (see block_like_traits.h).
Other refactorings:
- Misc corrections/improvements of internal API comments
- Remove a few misleading / unhelpful / redundant comments.
- Use move semantics in some places to simplify contracts
- Use better parameter names to indicate which parameters are used for outputs
- Remove some extraneous
extern - Various clean-ups to
CacheDumperImpl(mostly unnecessary code)
Test Plan: existing tests
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@pdillinger has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.