HaloDB.size and HaloDB.contains method#27
Conversation
|
@akhodakivskiy Do you still need this feature. If so, please update and rebase |
akhodakivskiy
force-pushed
the
db-size
branch
from
156e96f to
a821fab
Compare
|
Rebased. Thanks! |
akhodakivskiy
changed the title
| int size(byte[] key) { | ||
| InMemoryIndexMetaData metaData = inMemoryIndex.get(key); | ||
| if (metaData == null) { | ||
| return 0; |
There was a problem hiding this comment.
perhaps this should be -1 since 0 is a valid value size. Then -1 indicates the key is not present, and 0 indicates a value of size 0.
There was a problem hiding this comment.
Good point, fixed. Method HaloDB.get also returns 0 for keys that are not stored. I was following the same logic. But I agree that it should return -1 for abscent keys.
There was a problem hiding this comment.
Interesting... HaloDB.get returns a byte[], but HaloDBInternal has another get method that takes a ByteBuffer and reads the value into it (or truncating it if the value is larger than the buffer). This returns the number of bytes copied into the buffer, and 0 when not present.
However, this method is not used anywhere. Its not tested, its not used by the public facing API, it seems to be dead (and untested!) code.
akhodakivskiy
force-pushed
the
db-size
branch
from
a821fab to
0329bbb
Compare
scottcarey
left a comment
scottcarey
left a comment
There was a problem hiding this comment.
Oops, I meant to comment, not review.
| } | ||
| } | ||
|
|
||
| public int size(byte[] key) { |
There was a problem hiding this comment.
Is 'size' the right name? there is already a size method that returns the number of elements. To avoid confusion, we should consider a different name. Perhaps entrySize or valueSize ?
Also, the public API in HaloDB.java should document the expected behavior -- something like Returns the size of the value if present, or -1 if not present.
Ports yahoo#27: check key existence and read a value's size straight from the in-memory index with no disk access (valueSize returns -1 when absent). The upstream size(byte[]) is named valueSize here to avoid overloading size() (record count).
|
Thanks for this! Upstream looks unmaintained, so I've started a maintained fork — outr/HaloDB ( |
4 participants
Adding two features: