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

Enhance backup::restore function to support PostgreSQL in addition to RocksDB #80

Open
msk opened this issue May 26, 2023 · 4 comments
Open
Labels
enhancement New feature or request

Comments

@msk
Copy link
Contributor

msk commented May 26, 2023

Description

Currently, the backup::restore function is designed to restore backups for RocksDB only. This issue aims to enhance this function to support PostgreSQL as well.

This change would involve not only restoring the state of the RocksDB store but also the PostgreSQL database from its corresponding backup. It's proposed to use psql command for restoring the PostgreSQL backup.

Tasks

  1. Modify the backup::restore function to restore both RocksDB and PostgreSQL databases.
  2. Ensure the function identifies the correct backup files for each database type.
  3. Use the psql command to restore the PostgreSQL backup.
  4. Thoroughly test the new functionality, ensuring that it can correctly restore the state of both the RocksDB store and the PostgreSQL database.

Acceptance Criteria

  1. The backup::restore function is able to restore both RocksDB and PostgreSQL backups.
  2. All tests pass without causing any regression.
@msk msk added the enhancement New feature or request label May 26, 2023
@kimhanbeom
Copy link
Contributor

@msk @syncpark

I am currently working on an implementation, referring to the restoration part of https://github.com/aicers/cli.
In order to use this feature, I need to call sudo_run_command and that requires root privileges.

To solve this, it looks like we need to choose one of 2 ways.

  1. Run review as root to perform the command.
  2. Add sudo_run_command to roxy to perform the command via roxy.

@msk
Copy link
Contributor Author

msk commented Jun 2, 2023

Which step requires the root privilege?

@kimhanbeom
Copy link
Contributor

The part of the postgres_restore_docker function that moves data from the existing postgresql to tmp requires permissions.

@msk
Copy link
Contributor Author

msk commented Jun 5, 2023

Could we consider the option of creating a temporary directory that's writable for the REview process instead? This way, we can circumvent the need for superuser permissions during the process of moving data from existing postgresql to a temporary directory.

It's generally a bad practice to run applications with root privileges unless absolutely necessary, due to the significant security risks involved. When a process runs with root privileges, it has unrestricted access to the system. This means it can read, write, and delete every file on your system, and it can execute any command. If any vulnerabilities exist within the application, an attacker could exploit them to gain full control over the system.

kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jun 23, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jun 23, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jun 23, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jun 27, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jun 28, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jun 29, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jul 6, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jul 6, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
kimhanbeom pushed a commit to kimhanbeom/review-database that referenced this issue Jul 6, 2023
- Added new module 'postgres' to support postgresql.
- Enhance `backup::list`/`backup::create`/`backup::restore`/`backup::recovery`
  function to support postgresql.
- Added `backup::purge_old_backups` for apply immediately after
  `num_backups_to_keep` is changed.

Closes: petabi#71, petabi#76, petabi#80
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants