Skip to content

Add handling compression argument for postgres #280

Merged
huacnlee merged 1 commit into
gobackup:mainfrom
matfiz:main
Jan 24, 2025
Merged

Add handling compression argument for postgres #280
huacnlee merged 1 commit into
gobackup:mainfrom
matfiz:main

Conversation

@matfiz
Copy link
Copy Markdown
Contributor

@matfiz matfiz commented Jan 21, 2025

In relation to #208, for postgres database pg_dump supports --compress argument. This PR adds handling of compress from config, along with setting the correct file name. It saves disk space.

The allowed values for compress are: gzip, lz4 and std. The compression level can be set as: gzip:3.

Comment thread database/postgresql.go
"zstd": "zst",
}
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove this format check, allows any values.

We do not need to do to mush limit, user can follow the PostgreSQL document to do.

Copy link
Copy Markdown
Contributor Author

@matfiz matfiz Jan 22, 2025

Choose a reason for hiding this comment

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

@huacnlee In general I agree, but it is beneficial to set the correct file extension. This saves hassle while restoring from the backup when the file is named accordingly. What do you think? How do you propose to handle file extension if we do not have this mapping?

As an alternative, we can ofc add another config param with file extension if you prefer :-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@huacnlee I would appreciate if you could share your opinion. I need this compression param for backing up large (>100GB) databases. If there is no hope to merge and release it soon, I will need to base it on the fork. Thanks!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove this check, then I will going to merge this PR. I want Gobackup to not too many logic.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually, no this PR change, we also can use args option for this case, any arguments that pg_dump supports can assign by args option.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No @huacnlee, we cannot use args. Look at this PR pls, it changes the way the dump file name is constructed. This is the reason why the map is added.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see, only use args we can't change the output filename.

@huacnlee huacnlee merged commit 8c1693e into gobackup:main Jan 24, 2025
@huacnlee
Copy link
Copy Markdown
Collaborator

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants