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

Bug that may cause backup a lot of unuseful page. #76

Closed
movead opened this issue Sep 13, 2018 · 2 comments
Closed

Bug that may cause backup a lot of unuseful page. #76

movead opened this issue Sep 13, 2018 · 2 comments
Assignees

Comments

@movead
Copy link

movead commented Sep 13, 2018

pg_rman/backup.c

Line 1426 in 4abebbe

bool prev_file_not_found = false;

Here code has something wrong,and i add one line code.

static void
backup_files(const char *from_root,
			 const char *to_root,
			 parray *files,
			 parray *prev_files,
			 const XLogRecPtr *lsn,
			 bool compress,
			 const char *prefix)
{
	int				i;
	int				num_skipped = 0;
	struct timeval	tv;
	bool			prev_file_not_found = false;

	/* sort pathname ascending */
	parray_qsort(files, pgFileComparePath);

	gettimeofday(&tv, NULL);

	/* backup a file or create a directory */
	for (i = 0; i < parray_num(files); i++)
	{
		int			ret;
		struct stat	buf;

               pgFile *file = (pgFile *) parray_get(files, i);
               ///////////////////////////////////////////////////////////////////
               //I add code here
               prev_file_not_found = false;
               ...
               ...
               ...
       }

}

If not do this change, it may backup a lot of files which is nothing useful change when a table is more than 100G. It is due to PostgreSQL may change tuple header data without change SLN.

@MoonInsung MoonInsung self-assigned this Sep 21, 2018
@MoonInsung
Copy link
Contributor

Dear lchch.
Thank you for finding a bug.
I had reproduced the problem in this way.

  1. Input 500mb data to table A
  2. Input 1mb data to table B
  3. Run to full backup
  4. Add to 1 gb data to table A
  5. Add to 1mb data to table B
  6. Run to incremental backup
    -> It works the same as a full backup(2mb), not 1mb data which is incremental data for table B

This problem is that did not initialize the value of bool(prev_file_not_found) as you said.
I'll check if there's any other problem and try to fix the source-code.

Regards.

@movead
Copy link
Author

movead commented Sep 21, 2018

That is the bug.
It is dangerous for table A too because of it's sort.

Example:
Relfilenode of table A is 16384
Due to sort,16384.2 is backuped after16384.120,so if 16384.120 is a new file, 16384.2 may be copied too.
Luckily,we can solve the problems by initialize the value of bool(prev_file_not_found)

amitlan pushed a commit that referenced this issue Oct 10, 2018
In backup_files(), we forgot to reset prev_file_not_found, which once set, causes subsequent files to assume the same value resulting in redundant copying of data.
@movead movead closed this as completed Nov 7, 2018
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

No branches or pull requests

2 participants