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

The behaviour of GetUpdatesSince is not the same as documented #1565

Closed
bluesalt opened this issue Nov 24, 2016 · 2 comments
Closed

The behaviour of GetUpdatesSince is not the same as documented #1565

bluesalt opened this issue Nov 24, 2016 · 2 comments
Assignees

Comments

@bluesalt
Copy link
Contributor

bluesalt commented Nov 24, 2016

Here is the document of the API

  // Sets iter to an iterator that is positioned at a write-batch containing
  // seq_number. If the sequence number is non existent, it returns an iterator
  // at the first available seq_no after the requested seq_no
  // Returns Status::OK if iterator is valid
  // Must set WAL_ttl_seconds or WAL_size_limit_MB to large values to
  // use this api, else the WAL files will get
  // cleared aggressively and the iterator might keep getting invalid before
  // an update is read.
  virtual Status GetUpdatesSince(
      SequenceNumber seq_number, unique_ptr<TransactionLogIterator>* iter,
      const TransactionLogIterator::ReadOptions&
          read_options = TransactionLogIterator::ReadOptions()) = 0;

It is clear that the iterator should be after the requested seq_no if the requested seq_no is not existing. I have a db which currently holds two sequence number 1, 4. However, the following programming emits

1
4

Per the document, only 4 should be emitted. Do I misunderstand the doc ?

Here is the program:

#include <iostream>
#include "rocksdb/db.h"

using namespace std;

int main() {
    rocksdb::DB* db;
    rocksdb::Options options;
    rocksdb::Status status = rocksdb::DB::OpenForReadOnly(options, "/tmp/kv", &db);

    unique_ptr<rocksdb::TransactionLogIterator> iter;
    status = db->GetUpdatesSince(2, &iter);
    rocksdb::WriteBatch wb;
    if (status.ok()) {
        for (; iter->Valid(); iter->Next()) {
            auto batch_result = iter->GetBatch();

            wb = *batch_result.writeBatchPtr;
            cout << batch_result.sequence << endl;
        }
    }

    delete db;

    return 0;
}
@markk12
Copy link

markk12 commented Oct 20, 2017

i m interested in it , but i dont understand how to read data .batch_result is batch class for executing a batch not for read what i executed before. I might to find i put the key X and value Y for example. And then if i work with columns how to do?

@ramvadiv ramvadiv assigned ramvadiv and unassigned miasantreble Mar 24, 2021
@ramvadiv ramvadiv assigned riversand963 and unassigned ramvadiv Sep 11, 2021
@riversand963 riversand963 added bug Confirmed RocksDB bugs and removed bug Confirmed RocksDB bugs labels Dec 31, 2021
@riversand963
Copy link
Contributor

@bluesalt yeah, it is possible that the requested seq meets the following conditions:

batch_start_seq <= seq <= batch_end_seq

while the TransactionLogIterator::GetBatch() just returns the batch_start_seq. Therefore, you will find that the first seq of GetBatch() is smaller than seq.

Proposed a fix in #9355

@riversand963 riversand963 linked a pull request Jan 27, 2022 that will close this issue
facebook-github-bot pushed a commit that referenced this issue Apr 15, 2022
Summary:
Make `DB::GetUpdatesSince` return early if told to scan WALs generated by transactions
with write-prepared or write-unprepared policies (`seq_per_batch` is true), as indicated by
API comment.

Also add checks to `TransactionLogIterator` to clarify some conditions.

No API change.

Pull Request resolved: #9459

Test Plan:
make check

Closing #1565

Reviewed By: akankshamahajan15

Differential Revision: D33821243

Pulled By: riversand963

fbshipit-source-id: c8b155d020ce0980e2d3b3b1da40b96e65b48d79
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants