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

ADBDEV-5541: Backup statistics by one row #77

Merged
merged 20 commits into from
May 27, 2024
Merged

ADBDEV-5541: Backup statistics by one row #77

merged 20 commits into from
May 27, 2024

Conversation

RekGRpth
Copy link
Member

@RekGRpth RekGRpth commented Apr 27, 2024

Backup statistics by one row

gpbackup backuped statistics, loading them entirely into RAM. This can create
difficulties in the case of a huge number of tables and may come down to OOM.

This patch changes the backup logic: now statistics are not loaded entirely at
once, but are requested in portions by one row. This significantly reduces RAM
consumption when backing up statistics.

The test was not added because the result does not change. And to watch the
difference in memory consumption, we need a lot of tables with large statistics,
which is not very good for running tests.

@RekGRpth RekGRpth changed the title ADBDEV-5541: backup statistic table by table ADBDEV-5541: Backup statistics in batches Apr 27, 2024
@RekGRpth RekGRpth marked this pull request as ready for review May 7, 2024 04:01
@RekGRpth RekGRpth marked this pull request as draft May 20, 2024 04:30
@RekGRpth RekGRpth marked this pull request as ready for review May 20, 2024 07:54
@RekGRpth RekGRpth changed the title ADBDEV-5541: Backup statistics in batches ADBDEV-5541: Backup statistics by cursors May 21, 2024
backup/statistics.go Outdated Show resolved Hide resolved
backup/queries_statistics.go Outdated Show resolved Hide resolved
@RekGRpth RekGRpth changed the title ADBDEV-5541: Backup statistics by cursors ADBDEV-5541: Backup statistics by one row May 23, 2024
@whitehawk
Copy link

When reading the description:

...
gpbackup backups statistics, first loading them entirely into RAM.
...
This patch allows to backup statistics not entirely at once, but by requesting them in portions by one row.
...

I can interpret in a way that this change adds an option: either use old way (load everything into RAM) or new way (get the data in portions).

I think it is better to tune the description, for ex:

...
gpbackup backuped statistics, loading them entirely into RAM.
...
This patch changes the backup logic: now statistics are not loaded entirely at once, but are requested in portions by one row.
...

to avoid possible wrong interpetation.

@RekGRpth
Copy link
Member Author

When reading the description:

...
gpbackup backups statistics, first loading them entirely into RAM.
...
This patch allows to backup statistics not entirely at once, but by requesting them in portions by one row.
...

I can interpret in a way that this change adds an option: either use old way (load everything into RAM) or new way (get the data in portions).

I think it is better to tune the description, for ex:

...
gpbackup backuped statistics, loading them entirely into RAM.
...
This patch changes the backup logic: now statistics are not loaded entirely at once, but are requested in portions by one row.
...

to avoid possible wrong interpetation.

applied

@whitehawk
Copy link

In the description:

gpbackup backups statistics, first them entirely into RAM.

I guess the 'loading' word is missed. Also, it is better to use past tense for old problematic behavior.

@RekGRpth
Copy link
Member Author

In the description:

gpbackup backups statistics, first them entirely into RAM.

I guess the 'loading' word is missed. Also, it is better to use past tense for old problematic behavior.

fixed

whitehawk
whitehawk previously approved these changes May 24, 2024
backup/statistics.go Outdated Show resolved Hide resolved
@RekGRpth RekGRpth dismissed stale reviews from bimboterminator1 and whitehawk via 8682ffc May 27, 2024 08:52
@andr-sokolov andr-sokolov merged commit 6cd8a51 into master May 27, 2024
2 checks passed
@andr-sokolov andr-sokolov deleted the ADBDEV-5541 branch May 27, 2024 14:06
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