Add initial blob support to batched MultiGet #7766
Conversation
…hods: a single boolean is not sufficient to communicate the blobness of all keys
There was a problem hiding this comment.
@ltamasi has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| constexpr char first_key[] = "first_key"; | ||
| constexpr char first_value[] = "short"; |
There was a problem hiding this comment.
I have a C++ question: seems const char* first_key = "first_key"; or std::string first_key = "first_key"; also works here, is there any reason we prefer constexpr char[] here?
There was a problem hiding this comment.
It's part personal preference, part performance (std::string potentially does a heap allocation, depending on size) but the biggest reason in this case is that I use these with static_assert. static_assert requires an expression that can be evaluated at compile time; hence the use of constexpr and arrays (the length of which can be determined at compile time using sizeof).
Summary: The patch adds initial support for reading blobs to the batched `MultiGet` API. The current implementation simply retrieves the blob values as the blob indexes are encountered; that is, reads from blob files are currently not batched. (This will be optimized in a separate phase.) In addition, the patch removes some dead code related to BlobDB from the batched `MultiGet` implementation, namely the `is_blob` / `is_blob_index` flags that are passed around in `DBImpl` and `MemTable` / `MemTableListVersion`. These were never hooked up to anything and wouldn't work anyways, since a single flag is not sufficient to communicate the "blobness" of multiple key-values. Pull Request resolved: facebook#7766 Test Plan: `make check` Reviewed By: jay-zhuang Differential Revision: D25479290 Pulled By: ltamasi fbshipit-source-id: 7aba2d290e31876ee592bcf1adfd1018713a8000
Summary:
The patch adds initial support for reading blobs to the batched
MultiGetAPI.The current implementation simply retrieves the blob values as the blob indexes
are encountered; that is, reads from blob files are currently not batched. (This
will be optimized in a separate phase.) In addition, the patch removes some dead
code related to BlobDB from the batched
MultiGetimplementation, namely theis_blob/is_blob_indexflags that are passed around inDBImplandMemTable/MemTableListVersion. These were never hooked up to anything and wouldn'twork anyways, since a single flag is not sufficient to communicate the "blobness"
of multiple key-values.
Test Plan:
make check