Fix more bugs when inserting a lot of data.

cb_fsmpage_get_fsm_bit was supposed to test whether a certain bit
is set in the page, but randomly used the wrong variable. Fixed.

cbfsmpage_find_free_segment used buggy logic. Replace with logic
modelled on the latest version of cb_metapage_find_free_segment.

ConveyorSearchFSMPages had an off-by-one error in the code that
decides where the last FSM page was, and a separate off-by-one
error in the code that decides whether to extend the relation.
Repair both of those.

All per test case from Dilip Kumar.

diff --git a/src/backend/access/conveyor/cbfsmpage.c b/src/backend/access/conveyor/cbfsmpage.c
index a0723ed4c0fe5f0c4a244215bcfca0fae05c5baf..65b8f252b7b5e000e48ad635e1e770a402737879 100644 (file)
--- a/src/backend/access/conveyor/cbfsmpage.c
+++ b/src/backend/access/conveyor/cbfsmpage.c
@@ -65,7 +65,7 @@ cb_fsmpage_get_fsm_bit(Page page, CBSegNo segno)
        bitno = segno - fsmp->cbfsm_start;
        byte = fsmp->cbfsm_state[bitno / BITS_PER_BYTE];
        mask = 1 << (bitno % BITS_PER_BYTE);
-       return (segno & mask) != 0;
+       return (byte & mask) != 0;
 }
 
 /*
@@ -113,24 +113,22 @@ cbfsmpage_find_free_segment(Page page)
 {
        CBFSMPageData *fsmp = cb_fsmpage_get_special(page);
        unsigned        i;
+       unsigned        j;
 
        StaticAssertStmt(CB_FSMPAGE_FREESPACE_BYTES % sizeof(uint64) == 0,
                                         "CB_FSMPAGE_FREESPACE_BYTES should be a multiple of 8");
 
-       for (i = 0; i < CB_FSMPAGE_FREESPACE_BYTES; i += sizeof(uint64))
+       for (i = 0; i < CB_FSMPAGE_FREESPACE_BYTES; ++i)
        {
-               uint64  word = * (uint64 *) &fsmp->cbfsm_state[i];
+               uint8   b = fsmp->cbfsm_state[i];
 
-               if (word != PG_UINT64_MAX)
-               {
-                       uint64          flipword = ~word;
-                       int                     b = fls((int) flipword);
-
-                       if (b == 0)
-                               b = 32 + fls((int) (flipword >> 32));
+               if (b == 0xFF)
+                       continue;
 
-                       Assert(b >= 1 && b <= 64);
-                       return fsmp->cbfsm_start + (i * BITS_PER_BYTE) + (b - 1);
+               for (j = 0; j < BITS_PER_BYTE; ++j)
+               {
+                       if ((b & (1 << j)) == 0)
+                               return fsmp->cbfsm_start + (i * BITS_PER_BYTE) + j;
                }
        }
 

diff --git a/src/backend/access/conveyor/conveyor.c b/src/backend/access/conveyor/conveyor.c
index fc4cf77c8f25e7c2871450d6bb3036f7e609bf46..586d51808559d7e4cd4424ff94b338f15ed321a4 100644 (file)
--- a/src/backend/access/conveyor/conveyor.c
+++ b/src/backend/access/conveyor/conveyor.c
@@ -1390,9 +1390,27 @@ ConveyorSearchFSMPages(ConveyorBelt *cb, CBSegNo next_segment,
                *fsmbuffer = InvalidBuffer;
        }
 
-       /* Work out the locations of the FSM blocks. */
+       /*
+        * Work out the locations of the FSM blocks.
+        *
+        * stopblkno doesn't need to be perfectly accurate, just good enough that
+        * we search all of the FSM pages that are guaranteed to exist and no more.
+        * If next_segment is greater than zero, we know that the segment prior to
+        * the next segment has to exist, and so any FSM pages which would precede
+        * that must also exist. However, if next_segment is 0, or really any value
+        * less than or equal to CB_FSM_SEGMENTS_FOR_METAPAGE, then there may be
+        * no FSM pages at all.
+        *
+        * NB: When next_segment points to the first segment covered by some FSM
+        * page, that FSM page doesn't have to exist yet. We have to be careful
+        * to assume only that the previous segment exists.
+        */
        firstblkno = cb_first_fsm_block(cb->cb_pages_per_segment);
-       stopblkno = cb_segment_to_block(cb->cb_pages_per_segment, next_segment, 0);
+       if (next_segment <= CB_FSM_SEGMENTS_FOR_METAPAGE)
+               stopblkno = 0;
+       else
+               stopblkno = cb_segment_to_block(cb->cb_pages_per_segment,
+                                                                               next_segment - 1, 0);
        stride = cb_fsm_block_spacing(cb->cb_pages_per_segment);
 
        /*
@@ -1449,7 +1467,7 @@ ConveyorSearchFSMPages(ConveyorBelt *cb, CBSegNo next_segment,
                nblocks = RelationGetNumberOfBlocksInFork(cb->cb_rel, cb->cb_fork);
 
                /* If the relation needs to be physically extended, do so. */
-               if (nblocks < currentblkno)
+               if (nblocks <= currentblkno)
                {
                        /*
                         * We don't currently have a concept of separate relation