Re: pg_basebackup failed to back up large file

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", &current_len_left) != 1)
> + if (sscanf(copybuf + 124, "%11lo", &current_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

In response to

Responses

Browse pgsql-hackers by date

  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