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

Orioledb basic support #1698

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

homper
Copy link
Contributor

@homper homper commented Apr 26, 2024

I added some basic support of orioledb by wal-g
I tested that with these small modifications these features work:

  • backup-push
  • backup-fetch
  • wal-push
  • wal-fetch

Adding Dockerfile, scripts, and log of running this test to prove that it works
Sorry, that I'm not added it to pg_tests yet. Was not sure.

For now I added code to postgres wal-g version with option to enable it

Orioledb implemented as postgres extension with own table access method, and so it basically works alongside regular postgres, but uses own file format to store table pages
Without modification wal-g performs a full backup (with WALG_DELTA_MAX_STEPS=6)

wal-push and wal-fetch works because we implemented custom wal records, so no additional code required for this

I duplicated some functions with Orioledb* prefixed versions just to not affect any non-postgres related code for now

But I wan't to discuss before going any further what will be the better way to implement this
I see three variants:

  1. Implement orioledb as a separate wal-g version, but it will duplicate many of postgres-es code
  2. Adding some kind of generic interface for processing data of postgres extensions
  3. Leave just orioledb as option but implement it in a different, better way

What do you think?

Files:
ubuntu_check.log
wal-g-orioledb-test.zip

@homper homper requested a review from a team as a code owner April 26, 2024 06:46
@akorotkov
Copy link

@homper, thank you.
I'd like to highlight some details.

  1. OrioleDB implements row-level WAL instead of block-level WAL. Thus, an OrioleDB data page doesn't contain the LSN. But instead, an OrioleDB data page contains a checkpoint number. This pull request implements incremental backup by archiving only pages whose checkpoint number is greater than the checkpoint number of the last backup.
  2. This is proof-of-concept. Obviously, this needs some cleanup. Also, it doesn't support OrioleDB compressed tables yet. We would like to get your initial feedback on which way to move it further (any of the options proposed by @homper or even something different).

Copy link
Collaborator

@x4m x4m left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Of course, this is a functionality we want in WAL-G.

But I wan't to discuss before going any further what will be the better way to implement this

Personally, I'd prefer if OrioleDB would be supported by regular wal-g binary for Postgres. I do not see any reason to make OrioleDB 2nd class citizen, enabled by some kind of option etc.
Also I see no reason to invent some generalization layer for other extensions.
Let's follow simple contract:

  1. Plz do not break Postgres stuff :) And other DBs
  2. If something bugs regarding OrioleDB will emerge and WAL-G team cannot fix them - we will ask for assistance.
  3. Modularity of a code is desired. It's cool if OrioleDB logic is concentrated in some specific place, but not so terrible if it's incorporated into many appropriate places.

Thanks!

@@ -28,6 +28,7 @@ const (
deltaFromNameFlag = "delta-from-name"
addUserDataFlag = "add-user-data"
withoutFilesMetadataFlag = "without-files-metadata"
withOrioledb = "with-orioledb"
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we query database for this somehow? Why ask user, if we can ask db?

@akorotkov
Copy link

@x4m, thank you! The contract is accepted!

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.

None yet

3 participants