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 storage engine type awareness in Kvrocks #1973

Merged
merged 7 commits into from
Jan 3, 2024
Merged

The storage engine type awareness in Kvrocks #1973

merged 7 commits into from
Jan 3, 2024

Conversation

MaheshMadushan
Copy link
Contributor

@MaheshMadushan MaheshMadushan commented Dec 31, 2023

#1966

Storage Engine Type Enum
enum class StorageEngineType : uint16_t { RocksDB, Speedb, };

Storage type stored in variable STORAGE_ENGINE_TYPE in src/storage.h. it can be StorageEngineType::RocksDB or StorageEngineType::Speedb

@git-hulk git-hulk requested a review from PragmaTwice December 31, 2023 12:13
git-hulk
git-hulk previously approved these changes Dec 31, 2023
Copy link
Member

@git-hulk git-hulk left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Copy link
Member

@PragmaTwice PragmaTwice left a comment

Choose a reason for hiding this comment

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

BTW, I think we can have an enum type in our code.

@MaheshMadushan
Copy link
Contributor Author

BTW, I think we can have an enum type in our code.

Under the namespace engine in src/storage.h following enum can be created.
enum class Type { RocksDB, Speedb, };

Then we can have macros for storage engine as following,
STORAGE_ENGINE=0 # for rocksdb
STORAGE_ENGINE=1 # for speedb

Then we can determine the engine type as following,
if (STORAGE_ENGINE == static_cast<int>(engine::Type::RocksDB)) {} ...

Would that be ideal?

@PragmaTwice
Copy link
Member

BTW, I think we can have an enum type in our code.

Under the namespace engine in src/storage.h following enum can be created.
enum class Type { RocksDB, Speedb, };

Then we can have macros for storage engine as following,
STORAGE_ENGINE=0 # for rocksdb
STORAGE_ENGINE=1 # for speedb

Then we can determine the engine type as following,
if (STORAGE_ENGINE == static_cast<int>(engine::Type::RocksDB)) {} ...

Would that be ideal?

Firstly, the enum type can definitely have a better name than Type.

Besides, we can have a static-storage-duration constant to wrap the macro, so that we don't need to do any type cast.

Copy link

sonarqubecloud bot commented Jan 3, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@PragmaTwice PragmaTwice merged commit 08c7335 into apache:unstable Jan 3, 2024
@PragmaTwice
Copy link
Member

Thank you for your contribution!

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.

4 participants