]> granicus.if.org Git - postgresql/commitdiff
pg_rewind: Fix some problems when copying files >2GB.
authorRobert Haas <rhaas@postgresql.org>
Fri, 21 Jul 2017 18:25:36 +0000 (14:25 -0400)
committerRobert Haas <rhaas@postgresql.org>
Fri, 21 Jul 2017 18:25:36 +0000 (14:25 -0400)
When incrementally updating a file larger than 2GB, the old code could
either fail outright (if the client asked the server for bytes beyond
the 2GB boundary) or fail to copy all the blocks that had actually
been modified (if the server reported a file size to the client in
excess of 2GB), resulting in data corruption.  Generally, such files
won't occur anyway, but they might if using a non-default segment size
or if there the directory contains stray files unrelated to
PostgreSQL.  Fix by a more prudent choice of data types.

Even with these improvements, this code still uses a mix of different
types (off_t, size_t, uint64, int64) to represent file sizes and
offsets, not all of which necessarily have the same width or
signedness, so further cleanup might be in order here.  However, at
least now they all have the potential to be 64 bits wide on 64-bit
platforms.

Kuntal Ghosh and Michael Paquier, with a tweak by me.

Discussion: http://postgr.es/m/CAGz5QC+8gbkz=Brp0TgoKNqHWTzonbPtPex80U0O6Uh_bevbaA@mail.gmail.com

src/bin/pg_rewind/libpq_fetch.c

index cf3f64c3aa0919634ca74e5b536be362dd445f7d..6b4c8dd04e0f4ac36cee73ef33873a6a16818e7a 100644 (file)
@@ -194,7 +194,7 @@ libpqProcessFileList(void)
        for (i = 0; i < PQntuples(res); i++)
        {
                char       *path = PQgetvalue(res, i, 0);
-               int                     filesize = atoi(PQgetvalue(res, i, 1));
+               int64           filesize = atol(PQgetvalue(res, i, 1));
                bool            isdir = (strcmp(PQgetvalue(res, i, 2), "t") == 0);
                char       *link_target = PQgetvalue(res, i, 3);
                file_type_t type;
@@ -220,13 +220,35 @@ libpqProcessFileList(void)
        PQclear(res);
 }
 
+/*
+ * Converts an int64 from network byte order to native format.
+ */
+static int64
+pg_recvint64(int64 value)
+{
+       union
+       {
+               int64   i64;
+               uint32  i32[2];
+       } swap;
+       int64   result;
+
+       swap.i64 = value;
+
+       result = (uint32) ntohl(swap.i32[0]);
+       result <<= 32;
+       result |= (uint32) ntohl(swap.i32[1]);
+
+       return result;
+}
+
 /*----
  * Runs a query, which returns pieces of files from the remote source data
  * directory, and overwrites the corresponding parts of target files with
  * the received parts. The result set is expected to be of format:
  *
  * path                text    -- path in the data directory, e.g "base/1/123"
- * begin       int4    -- offset within the file
+ * begin       int8    -- offset within the file
  * chunk       bytea   -- file content
  *----
  */
@@ -247,7 +269,7 @@ receiveFileChunks(const char *sql)
        {
                char       *filename;
                int                     filenamelen;
-               int                     chunkoff;
+               int64           chunkoff;
                int                     chunksize;
                char       *chunk;
 
@@ -270,7 +292,7 @@ receiveFileChunks(const char *sql)
                        pg_fatal("unexpected result set size while fetching remote files\n");
 
                if (PQftype(res, 0) != TEXTOID ||
-                       PQftype(res, 1) != INT4OID ||
+                       PQftype(res, 1) != INT8OID ||
                        PQftype(res, 2) != BYTEAOID)
                {
                        pg_fatal("unexpected data types in result set while fetching remote files: %u %u %u\n",
@@ -290,12 +312,12 @@ receiveFileChunks(const char *sql)
                        pg_fatal("unexpected null values in result while fetching remote files\n");
                }
 
-               if (PQgetlength(res, 0, 1) != sizeof(int32))
+               if (PQgetlength(res, 0, 1) != sizeof(int64))
                        pg_fatal("unexpected result length while fetching remote files\n");
 
                /* Read result set to local variables */
-               memcpy(&chunkoff, PQgetvalue(res, 0, 1), sizeof(int32));
-               chunkoff = ntohl(chunkoff);
+               memcpy(&chunkoff, PQgetvalue(res, 0, 1), sizeof(int64));
+               chunkoff = pg_recvint64(chunkoff);
                chunksize = PQgetlength(res, 0, 2);
 
                filenamelen = PQgetlength(res, 0, 0);
@@ -320,7 +342,7 @@ receiveFileChunks(const char *sql)
                        continue;
                }
 
-               pg_log(PG_DEBUG, "received chunk for file \"%s\", offset %d, size %d\n",
+               pg_log(PG_DEBUG, "received chunk for file \"%s\", offset " INT64_FORMAT ", size %d\n",
                           filename, chunkoff, chunksize);
 
                open_target_file(filename, false);
@@ -380,7 +402,7 @@ libpqGetFile(const char *filename, size_t *filesize)
  * function to actually fetch the data.
  */
 static void
-fetch_file_range(const char *path, unsigned int begin, unsigned int end)
+fetch_file_range(const char *path, uint64 begin, uint64 end)
 {
        char            linebuf[MAXPGPATH + 23];
 
@@ -389,12 +411,13 @@ fetch_file_range(const char *path, unsigned int begin, unsigned int end)
        {
                unsigned int len;
 
+               /* Fine as long as CHUNKSIZE is not bigger than UINT32_MAX */
                if (end - begin > CHUNKSIZE)
                        len = CHUNKSIZE;
                else
-                       len = end - begin;
+                       len = (unsigned int) (end - begin);
 
-               snprintf(linebuf, sizeof(linebuf), "%s\t%u\t%u\n", path, begin, len);
+               snprintf(linebuf, sizeof(linebuf), "%s\t" UINT64_FORMAT "\t%u\n", path, begin, len);
 
                if (PQputCopyData(conn, linebuf, strlen(linebuf)) != 1)
                        pg_fatal("could not send COPY data: %s",
@@ -419,7 +442,7 @@ libpq_executeFileMap(filemap_t *map)
         * First create a temporary table, and load it with the blocks that we
         * need to fetch.
         */
-       sql = "CREATE TEMPORARY TABLE fetchchunks(path text, begin int4, len int4);";
+       sql = "CREATE TEMPORARY TABLE fetchchunks(path text, begin int8, len int4);";
        res = PQexec(conn, sql);
 
        if (PQresultStatus(res) != PGRES_COMMAND_OK)