Re: pg_basebackup failed to back up large file

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