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

Incorrect timestamps returns by MultiGet when hit row cache #7930

Closed
burtonli opened this issue Feb 4, 2021 · 10 comments
Closed

Incorrect timestamps returns by MultiGet when hit row cache #7930

burtonli opened this issue Feb 4, 2021 · 10 comments
Assignees
Labels
bug Confirmed RocksDB bugs

Comments

@burtonli
Copy link
Contributor

burtonli commented Feb 4, 2021

Note: Please use Issues only for bug reports. For questions, discussions, feature requests, etc. post to dev group: https://groups.google.com/forum/#!forum/rocksdb or https://www.facebook.com/groups/rocksdb.dev

Expected behavior

Timestamps returned by MultiGet should be matched with timestamps of WriteOptions when records were written .

Actual behavior

When hit record cache, the returned timestamps are matched with the timestamp of ReadOptions.

Steps to reproduce the behavior

Initial DB with enough capacity for row_cache.

  1. Write record A with V1+ timestamp "1000"
  2. Write record A with V2 + timestamp "2000"
  3. Read record A with timestamp "1500", it returns V1 + timestamp with "1000" correctly.
  4. Read record A with timestamp "1500" again (hit row cache), it returns V1 + timestamp "1500" ("1000" is expected)

Root cause:
In the implementation of TableCache::Get(), when the user query key hit row cache, it call replayGetContextLog() by directly passing user query key, where it will extract query timestamp (1500) instead of timestamp for the record (1000).
@riversand963

Internal ref: T86715827

@jay-zhuang
Copy link
Contributor

jay-zhuang commented Feb 4, 2021

Yeah, currently timestamp + row_cache is not supported yet.

@riversand963
Copy link
Contributor

Hi @burtonli , would you like to contribute this feature?

@burtonli
Copy link
Contributor Author

burtonli commented Apr 1, 2021

I don't have plan in short future.

@riversand963 riversand963 added the up-for-grabs Up for grabs label Apr 3, 2021
@ajkr ajkr added the bug Confirmed RocksDB bugs label Apr 9, 2021
@thejchap
Copy link
Contributor

i am happy to take a look at this one

@ajkr ajkr removed the up-for-grabs Up for grabs label May 14, 2021
@burtonli
Copy link
Contributor Author

burtonli commented Aug 4, 2021

Thank you @thejchap , do we have any finding on this?

@thejchap
Copy link
Contributor

thejchap commented Aug 5, 2021

@burtonli thanks for following up. got derailed with team selection and onboarding. fully onboarded now and easing back in to normal day to day life. i plan to revisit this next week.

thejchap added a commit to thejchap/rocksdb that referenced this issue Nov 4, 2021
thejchap added a commit to thejchap/rocksdb that referenced this issue Nov 4, 2021
@sherriiiliu
Copy link
Contributor

Hi @thejchap , is there any update on this bug fix?

@sherriiiliu
Copy link
Contributor

Hi @thejchap , is there any update on this bug fix?

Kindly ping again @thejchap , is the bug fixed?

@cz2h
Copy link
Contributor

cz2h commented Jul 24, 2023

Hi @thejchap , any updates on this issue? If this issue is still up for grad, can I work on this?

@thejchap
Copy link
Contributor

@cz2h all yours

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed RocksDB bugs
Projects
None yet
7 participants