Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Avoid portability issues in autoprewarm.c.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 3 May 2018 16:50:27 +0000 (12:50 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 3 May 2018 16:50:34 +0000 (12:50 -0400)
autoprewarm.c mostly considered the number of blocks it might be dealing
with as being int64.  This is unnecessary, because NBuffers is declared
as int, and there's been no suggestion that we might widen it in the
foreseeable future.  Moreover, using int64 is problematic because the
code expected INT64_FORMAT to work with fscanf(), something we don't
guarantee, and which indeed fails on some older buildfarm members.

On top of that, the module randomly used uint32 rather than int64 variables
to hold block counters in several places, so it would fail anyway if we
ever did have NBuffers wider than that; and it also supposed that pg_qsort
could sort an int64 number of elements, which is wrong on 32-bit machines
(though no doubt a 32-bit machine couldn't actually have that many
buffers).

Hence, change all these variables to plain int.

In passing, avoid shadowing one variable named i with another,
and avoid casting away const in apw_compare_blockinfo.

Discussion: https://postgr.es/m/7773.1525288909@sss.pgh.pa.us

contrib/pg_prewarm/autoprewarm.c

index 9faabe576dd87fadb0480ef81d5a230f9cda54de..cc5e2dd89cd48de64d5e9dff6f10104f0596aad7 100644 (file)
@@ -25,6 +25,7 @@
  */
 
 #include "postgres.h"
+
 #include <unistd.h>
 
 #include "access/heapam.h"
@@ -73,9 +74,9 @@ typedef struct AutoPrewarmSharedState
    /* Following items are for communication with per-database worker */
    dsm_handle  block_info_handle;
    Oid         database;
-   int64       prewarm_start_idx;
-   int64       prewarm_stop_idx;
-   int64       prewarmed_blocks;
+   int         prewarm_start_idx;
+   int         prewarm_stop_idx;
+   int         prewarmed_blocks;
 } AutoPrewarmSharedState;
 
 void       _PG_init(void);
@@ -86,7 +87,7 @@ PG_FUNCTION_INFO_V1(autoprewarm_start_worker);
 PG_FUNCTION_INFO_V1(autoprewarm_dump_now);
 
 static void apw_load_buffers(void);
-static int64 apw_dump_now(bool is_bgworker, bool dump_unlogged);
+static int apw_dump_now(bool is_bgworker, bool dump_unlogged);
 static void apw_start_master_worker(void);
 static void apw_start_database_worker(void);
 static bool apw_init_shmem(void);
@@ -274,7 +275,7 @@ static void
 apw_load_buffers(void)
 {
    FILE       *file = NULL;
-   int64       num_elements,
+   int         num_elements,
                i;
    BlockInfoRecord *blkinfo;
    dsm_segment *seg;
@@ -317,7 +318,7 @@ apw_load_buffers(void)
    }
 
    /* First line of the file is a record count. */
-   if (fscanf(file, "<<" INT64_FORMAT ">>\n", &num_elements) != 1)
+   if (fscanf(file, "<<%d>>\n", &num_elements) != 1)
        ereport(ERROR,
                (errcode_for_file_access(),
                 errmsg("could not read from file \"%s\": %m",
@@ -336,7 +337,7 @@ apw_load_buffers(void)
                   &blkinfo[i].tablespace, &blkinfo[i].filenode,
                   &forknum, &blkinfo[i].blocknum) != 5)
            ereport(ERROR,
-                   (errmsg("autoprewarm block dump file is corrupted at line " INT64_FORMAT,
+                   (errmsg("autoprewarm block dump file is corrupted at line %d",
                            i + 1)));
        blkinfo[i].forknum = forknum;
    }
@@ -355,17 +356,17 @@ apw_load_buffers(void)
    /* Get the info position of the first block of the next database. */
    while (apw_state->prewarm_start_idx < num_elements)
    {
-       uint32      i = apw_state->prewarm_start_idx;
-       Oid         current_db = blkinfo[i].database;
+       int         j = apw_state->prewarm_start_idx;
+       Oid         current_db = blkinfo[j].database;
 
        /*
         * Advance the prewarm_stop_idx to the first BlockRecordInfo that does
         * not belong to this database.
         */
-       i++;
-       while (i < num_elements)
+       j++;
+       while (j < num_elements)
        {
-           if (current_db != blkinfo[i].database)
+           if (current_db != blkinfo[j].database)
            {
                /*
                 * Combine BlockRecordInfos for global objects with those of
@@ -373,10 +374,10 @@ apw_load_buffers(void)
                 */
                if (current_db != InvalidOid)
                    break;
-               current_db = blkinfo[i].database;
+               current_db = blkinfo[j].database;
            }
 
-           i++;
+           j++;
        }
 
        /*
@@ -388,7 +389,7 @@ apw_load_buffers(void)
            break;
 
        /* Configure stop point and database for next per-database worker. */
-       apw_state->prewarm_stop_idx = i;
+       apw_state->prewarm_stop_idx = j;
        apw_state->database = current_db;
        Assert(apw_state->prewarm_start_idx < apw_state->prewarm_stop_idx);
 
@@ -415,8 +416,7 @@ apw_load_buffers(void)
 
    /* Report our success. */
    ereport(LOG,
-           (errmsg("autoprewarm successfully prewarmed " INT64_FORMAT
-                   " of " INT64_FORMAT " previously-loaded blocks",
+           (errmsg("autoprewarm successfully prewarmed %d of %d previously-loaded blocks",
                    apw_state->prewarmed_blocks, num_elements)));
 }
 
@@ -427,7 +427,7 @@ apw_load_buffers(void)
 void
 autoprewarm_database_main(Datum main_arg)
 {
-   uint32      pos;
+   int         pos;
    BlockInfoRecord *block_info;
    Relation    rel = NULL;
    BlockNumber nblocks = 0;
@@ -557,13 +557,14 @@ autoprewarm_database_main(Datum main_arg)
  * Dump information on blocks in shared buffers.  We use a text format here
  * so that it's easy to understand and even change the file contents if
  * necessary.
+ * Returns the number of blocks dumped.
  */
-static int64
+static int
 apw_dump_now(bool is_bgworker, bool dump_unlogged)
 {
-   uint32      i;
+   int         num_blocks;
+   int         i;
    int         ret;
-   int64       num_blocks;
    BlockInfoRecord *block_info_array;
    BufferDesc *bufHdr;
    FILE       *file;
@@ -630,7 +631,7 @@ apw_dump_now(bool is_bgworker, bool dump_unlogged)
                 errmsg("could not open file \"%s\": %m",
                        transient_dump_file_path)));
 
-   ret = fprintf(file, "<<" INT64_FORMAT ">>\n", num_blocks);
+   ret = fprintf(file, "<<%d>>\n", num_blocks);
    if (ret < 0)
    {
        int         save_errno = errno;
@@ -691,8 +692,7 @@ apw_dump_now(bool is_bgworker, bool dump_unlogged)
    apw_state->pid_using_dumpfile = InvalidPid;
 
    ereport(DEBUG1,
-           (errmsg("wrote block details for " INT64_FORMAT " blocks",
-                   num_blocks)));
+           (errmsg("wrote block details for %d blocks", num_blocks)));
    return num_blocks;
 }
 
@@ -727,11 +727,14 @@ autoprewarm_start_worker(PG_FUNCTION_ARGS)
 
 /*
  * SQL-callable function to perform an immediate block dump.
+ *
+ * Note: this is declared to return int8, as insurance against some
+ * very distant day when we might make NBuffers wider than int.
  */
 Datum
 autoprewarm_dump_now(PG_FUNCTION_ARGS)
 {
-   int64       num_blocks;
+   int         num_blocks;
 
    apw_init_shmem();
 
@@ -741,7 +744,7 @@ autoprewarm_dump_now(PG_FUNCTION_ARGS)
    }
    PG_END_ENSURE_ERROR_CLEANUP(apw_detach_shmem, 0);
 
-   PG_RETURN_INT64(num_blocks);
+   PG_RETURN_INT64((int64) num_blocks);
 }
 
 /*
@@ -869,7 +872,7 @@ do { \
        return -1;              \
    else if (a->fld > b->fld)   \
        return 1;               \
-} while(0);
+} while(0)
 
 /*
  * apw_compare_blockinfo
@@ -883,8 +886,8 @@ do { \
 static int
 apw_compare_blockinfo(const void *p, const void *q)
 {
-   BlockInfoRecord *a = (BlockInfoRecord *) p;
-   BlockInfoRecord *b = (BlockInfoRecord *) q;
+   const BlockInfoRecord *a = (const BlockInfoRecord *) p;
+   const BlockInfoRecord *b = (const BlockInfoRecord *) q;
 
    cmp_member_elem(database);
    cmp_member_elem(tablespace);