From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pg_basebackup failed to back up large file |
Date: | 2014-06-03 14:32:31 |
Message-ID: | 20140603143231.GL24145@awork2.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi
On 2014-06-03 23:19:37 +0900, Fujii Masao wrote:
> I got the bug report of pg_basebackup off-list that it causes an error
> when there is large file (e.g., 4GB) in the database cluster. It's easy
> to reproduce this problem.
>
> The cause of this problem is that pg_basebackup uses an integer to
> store the size of the file to receive from the server and an integer
> overflow can happen when the file is very large. I think that
> pg_basebackup should be able to handle even such large file properly
> because it can exist in the database cluster, for example,
> the server log file under $PGDATA/pg_log can be such large one.
> Attached patch changes pg_basebackup so that it uses uint64 to store
> the file size and doesn't cause an integer overflow.
Right, makes sense.
> ---
> src/bin/pg_basebackup/pg_basebackup.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
> index b119fc0..959f0c0 100644
> --- a/src/bin/pg_basebackup/pg_basebackup.c
> +++ b/src/bin/pg_basebackup/pg_basebackup.c
> @@ -1150,7 +1150,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
> {
> char current_path[MAXPGPATH];
> char filename[MAXPGPATH];
> - int current_len_left;
> + uint64 current_len_left;
Any reason you changed the signedness here instead of just using a
int64? Did you review all current users?
> int current_padding = 0;
> bool basetablespace = PQgetisnull(res, rownum, 0);
> char *copybuf = NULL;
> @@ -1216,7 +1216,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
> }
> totaldone += 512;
>
> - if (sscanf(copybuf + 124, "%11o", ¤t_len_left) != 1)
> + if (sscanf(copybuf + 124, "%11lo", ¤t_len_left) != 1)
That's probably not going to work on 32bit platforms or windows where
you might need to use ll instead of l as a prefix. Also notice that
apparently (c.f. 9d7ded0f4277f5c0063eca8e871a34e2355a8371) sscanf can't
reliably be used for 64bit input :(. That pretty much sucks...
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2014-06-03 14:37:53 | Re: [HACKERS] BUG #9652: inet types don't support min/max |
Previous Message | Andres Freund | 2014-06-03 14:27:33 | Re: [BUGS] BUG #9652: inet types don't support min/max |