Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snapshot behavior: Surfacing keys added after snapshot? #382

Closed
dermesser opened this issue Jun 11, 2016 · 1 comment
Closed

Snapshot behavior: Surfacing keys added after snapshot? #382

dermesser opened this issue Jun 11, 2016 · 1 comment
Labels

Comments

@dermesser
Copy link

dermesser commented Jun 11, 2016

I've been reading the LevelDB source code a bit, and got to one corner case that I couldn't find implemented -- turns out, it isn't :-) This case is looking up an entry that did not exist at a given snapshot within that snapshot. It should not be found, but it is found using Get().

The documentation says Snapshots provide consistent read-only views over the entire state of the key-value store. This is correct for iterators; db_iter.cc:179 has a condition that skips "too new" entries. But in the Get() path, the tag of the key is cut off before looking up the key in either MemTable or Table/TableCache.

Short example:

  1 # include <leveldb/db.h>
  2 # include <string>
  3 # include <iostream>
  4 # include <cstdlib>
  5 
  6 using namespace leveldb;
  7 
  8 DB* open() {
  9     DB* db;
 10     Options opt;
 11     opt.create_if_missing = true;
 12     Status s = DB::Open(opt, std::string("local.db"), &db);
 13     
 14     if (!s.ok()) {
 15         std::cerr << s.ToString();
 16         std::exit(0);
 17     }
 18     
 19     return db;
 20 }
 21 
 22 int main(void) {
 23     DB* db = open();
 24     
 25     if (!db) {
 26         std::cerr << "DB is null\n";
 27         std::exit(0);
 28     }
 29     
 30     const Snapshot* snapshot0 = db->GetSnapshot();
 31     db->Put(WriteOptions(), "abc", "def");
 32     const Snapshot* snapshot1 = db->GetSnapshot();
 33     db->Put(WriteOptions(), "abx", "deg");
 34     const Snapshot* snapshot2 = db->GetSnapshot();
 35     
 36     std::string val;
 37     val.resize(100);
 38     ReadOptions ro;
 39     ro.snapshot = snapshot0;
 40     db->Get(ro, "abx", &val); // yields "deg" instead of NotFound
 41     std::cout << val << std::endl;
 42 }

There are essentially two solutions to this IMO -

  • Update the documentation to exclude Get() from the existing clause (in case this behaviour is WAI)
  • Check the sequence number before returning a success from the various lookup functions.
@cmumford cmumford added the bug label Aug 11, 2016
@dermesser
Copy link
Author

Nevermind, I cleared up my confusion.

The nested comparators (KeyComparator -> InternalKeyComparator -> User provided comparator) with InternalKeyComparator providing the required ordering prevents this bug, and my PoC program was flawed, too.

maochongxin pushed a commit to maochongxin/leveldb that referenced this issue Jul 21, 2022
When stopping a timer, the current time is subtracted
from the start time. However, when the times are identical,
or sufficiently close together, the subtraction can result
in a negative number.

For some reason MinGW is the only platform where this problem
manifests. I suspect it's due to MinGW specific behavior in either
the CPU timing code, floating point model, or printf formatting.

Either way, the fix for MinGW should be correct across all platforms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants