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

refactor: use rocksdb_wrapper::get to reimplement incr #651

Merged
merged 25 commits into from
Dec 10, 2020

Conversation

levy5307
Copy link
Contributor

@levy5307 levy5307 commented Dec 3, 2020

What problem does this PR solve?

  1. Add a class rocksdb_wrapper to wrap the operation of rocksdb, and put db_get into it.
  2. Use rocksdb_wrapper::get to reimplement incr function.

Check List

Tests

  • Unit test

@levy5307 levy5307 changed the title refactor: move db_get into rocksdb_wrapper refactor: use rocksdb_wrapper::get to reimplement incr Dec 3, 2020
neverchanje
neverchanje previously approved these changes Dec 3, 2020
src/server/rocksdb_wrapper.cpp Outdated Show resolved Hide resolved
struct db_get_context;
class pegasus_server_impl;

static constexpr int FAIL_DB_GET = -104;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you define it in cpp?
And what does '-104' mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It it internal error codes used for fail injection, and there are three other error codes defined in pegasus_write_service_impl.h:

static constexpr int FAIL_DB_WRITE_BATCH_PUT = -101;
static constexpr int FAIL_DB_WRITE_BATCH_DELETE = -102;
static constexpr int FAIL_DB_WRITE = -103;

I think 104 has no special meaning. And I don't want to change it, because it is not the main purpose for this pull reqeust.

acelyc111
acelyc111 previously approved these changes Dec 7, 2020
@neverchanje neverchanje merged commit c212da6 into apache:master Dec 10, 2020
@neverchanje neverchanje mentioned this pull request Mar 1, 2021
@levy5307 levy5307 deleted the db_get branch March 18, 2021 03:16
acelyc111 pushed a commit that referenced this pull request Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants