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

Rename "account policy" Column Family to "config" in RocksDB and Implement Database Migration #100

Open
msk opened this issue Jun 20, 2023 · 3 comments
Assignees
Labels
refactor Code refactoring

Comments

@msk
Copy link
Contributor

msk commented Jun 20, 2023

Upon reviewing the current utilization of the RocksDB within the review-database crate, it has been observed that there is room for improvement and efficiency gains. The focus of this issue is the Column Family titled "account policy", which is currently used to house a single entry - the "account policy key".

Considering the capacity of RocksDB's Column Families, the solitary use of this Column Family seems inefficient. There's an identified requirement to store additional configuration values within the RocksDB. To address this, it is proposed to repurpose the existing "account policy" Column Family to meet this need.

Suggested Modifications:

  1. Rename the "account policy" Column Family to "config".
  2. Update all instances in the code where this Column Family is referenced, replacing "account policy" with "config".
  3. Refactor the functions responsible for the addition, retrieval, and deletion of entries from this Column Family, enabling them to handle multiple configuration values, not just the "account policy key".
  4. Crucially, implement a database migration strategy to ensure the transition from the "account policy" Column Family to the "config" Column Family is smooth and does not result in data loss or system downtime.

Anticipated Advantages:

  1. Utilizes the capabilities of RocksDB's Column Families more efficiently.
  2. Consolidates the storage of various configuration values, streamlining access and modification processes.
  3. The new name, "config", is more intuitive and descriptive, facilitating easier understanding for new contributors and improving overall maintainability.
@henry0715-dev
Copy link
Contributor

@minshao
aicers/review-web#41
It seems that #100 work needs to be preceded in order to do that work.
Do you have an expected progress schedule?

@minshao
Copy link
Contributor

minshao commented Aug 12, 2024

@minshao aicers/review-web#41 It seems that #100 work needs to be preceded in order to do that work. Do you have an expected progress schedule?

I think #100 is not a prerequisite of aicers/review-web#41. #100 is about renaming that involves a bit complexity on the database side. It won't affect user side directly. And as such, it has a pretty low priority for now.

However, in order to accomplish aicers/review-web#41, database changes is inevitable. Please add an issue for review-database.

Is there a particular schedule for aicers/review-web#41? Do let me know.

@henry0715-dev
Copy link
Contributor

@minshao aicers/review-web#41 It seems that #100 work needs to be preceded in order to do that work. Do you have an expected progress schedule?

I think #100 is not a prerequisite of aicers/review-web#41. #100 is about renaming that involves a bit complexity on the database side. It won't affect user side directly. And as such, it has a pretty low priority for now.

However, in order to accomplish aicers/review-web#41, database changes is inevitable. Please add an issue for review-database.

Is there a particular schedule for aicers/review-web#41? Do let me know.

The review-web#41 task also has a lower priority.
If it is a review-database#100, we will proceed with the work accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code refactoring
Projects
None yet
Development

No branches or pull requests

3 participants