]> granicus.if.org Git - postgresql/commitdiff
amcheck: editorialize variable name & comment.
authorAndres Freund <andres@anarazel.de>
Fri, 10 Mar 2017 04:01:37 +0000 (20:01 -0800)
committerAndres Freund <andres@anarazel.de>
Fri, 10 Mar 2017 04:03:30 +0000 (20:03 -0800)
No exclusive lock is taken anymore...

contrib/amcheck/verify_nbtree.c

index 5724aa6be36a9dc1dd413606bc42faec80f37d89..c134e5f3b0a9dd6e7f2b12c69140e4aa22e9cc3e 100644 (file)
@@ -55,8 +55,8 @@ typedef struct BtreeCheckState
 
        /* B-Tree Index Relation */
        Relation        rel;
-       /* ExclusiveLock held? */
-       bool            exclusivelylocked;
+       /* ShareLock held on heap/index, rather than AccessShareLock? */
+       bool            readonly;
        /* Per-page context */
        MemoryContext targetcontext;
        /* Buffer access strategy */
@@ -94,7 +94,7 @@ PG_FUNCTION_INFO_V1(bt_index_parent_check);
 
 static void bt_index_check_internal(Oid indrelid, bool parentcheck);
 static inline void btree_index_checkable(Relation rel);
-static void bt_check_every_level(Relation rel, bool exclusivelylocked);
+static void bt_check_every_level(Relation rel, bool readonly);
 static BtreeLevel bt_check_level_from_leftmost(BtreeCheckState *state,
                                                         BtreeLevel level);
 static void bt_target_page_check(BtreeCheckState *state);
@@ -256,23 +256,23 @@ btree_index_checkable(Relation rel)
  * logical order, verifying invariants as it goes.
  *
  * It is the caller's responsibility to acquire appropriate heavyweight lock on
- * the index relation, and advise us if extra checks are safe when an
- * ExclusiveLock is held.
+ * the index relation, and advise us if extra checks are safe when a ShareLock
+ * is held.
  *
- * An ExclusiveLock is generally assumed to prevent any kind of physical
+ * A ShareLock is generally assumed to prevent any kind of physical
  * modification to the index structure, including modifications that VACUUM may
  * make.  This does not include setting of the LP_DEAD bit by concurrent index
  * scans, although that is just metadata that is not able to directly affect
  * any check performed here.  Any concurrent process that might act on the
  * LP_DEAD bit being set (recycle space) requires a heavyweight lock that
- * cannot be held while we hold an ExclusiveLock.  (Besides, even if that could
+ * cannot be held while we hold a ShareLock.  (Besides, even if that could
  * happen, the ad-hoc recycling when a page might otherwise split is performed
  * per-page, and requires an exclusive buffer lock, which wouldn't cause us
  * trouble.  _bt_delitems_vacuum() may only delete leaf items, and so the extra
  * parent/child check cannot be affected.)
  */
 static void
-bt_check_every_level(Relation rel, bool exclusivelylocked)
+bt_check_every_level(Relation rel, bool readonly)
 {
        BtreeCheckState *state;
        Page            metapage;
@@ -291,7 +291,7 @@ bt_check_every_level(Relation rel, bool exclusivelylocked)
         */
        state = palloc(sizeof(BtreeCheckState));
        state->rel = rel;
-       state->exclusivelylocked = exclusivelylocked;
+       state->readonly = readonly;
        /* Create context for page */
        state->targetcontext = AllocSetContextCreate(CurrentMemoryContext,
                                                                                                 "amcheck context",
@@ -353,7 +353,7 @@ bt_check_every_level(Relation rel, bool exclusivelylocked)
 
 /*
  * Given a left-most block at some level, move right, verifying each page
- * individually (with more verification across pages for "exclusivelylocked"
+ * individually (with more verification across pages for "readonly"
  * callers).  Caller should pass the true root page as the leftmost initially,
  * working their way down by passing what is returned for the last call here
  * until level 0 (leaf page level) was reached.
@@ -428,7 +428,7 @@ bt_check_level_from_leftmost(BtreeCheckState *state, BtreeLevel level)
                         * locking, check that the first valid page meets caller's
                         * expectations.
                         */
-                       if (state->exclusivelylocked)
+                       if (state->readonly)
                        {
                                if (!P_LEFTMOST(opaque))
                                        ereport(ERROR,
@@ -482,7 +482,7 @@ bt_check_level_from_leftmost(BtreeCheckState *state, BtreeLevel level)
                         */
                }
 
-               if (state->exclusivelylocked && opaque->btpo_prev != leftcurrent)
+               if (state->readonly && opaque->btpo_prev != leftcurrent)
                        ereport(ERROR,
                                        (errcode(ERRCODE_INDEX_CORRUPTED),
                                         errmsg("left link/right link pair in index \"%s\" not in agreement",
@@ -541,7 +541,7 @@ nextpage:
  *      "real" data item on the page to the right (if such a first item is
  *      available).
  *
- * Furthermore, when state passed shows ExclusiveLock held, and target page is
+ * Furthermore, when state passed shows ShareLock held, and target page is
  * internal page, function also checks:
  *
  * - That all child pages respect downlinks lower bound.
@@ -597,8 +597,8 @@ bt_target_page_check(BtreeCheckState *state)
                 * page items.
                 *
                 * We prefer to check all items against high key rather than checking
-                * just the first and trusting that the operator class obeys the
-                * transitive law (which implies that all subsequent items also
+                * just the last and trusting that the operator class obeys the
+                * transitive law (which implies that all previous items also
                 * respected the high key invariant if they pass the item order
                 * check).
                 *
@@ -705,12 +705,12 @@ bt_target_page_check(BtreeCheckState *state)
                        {
                                /*
                                 * As explained at length in bt_right_page_check_scankey(),
-                                * there is a known !exclusivelylocked race that could account
-                                * for apparent violation of invariant, which we must check
-                                * for before actually proceeding with raising error.  Our
-                                * canary condition is that target page was deleted.
+                                * there is a known !readonly race that could account for
+                                * apparent violation of invariant, which we must check for
+                                * before actually proceeding with raising error.  Our canary
+                                * condition is that target page was deleted.
                                 */
-                               if (!state->exclusivelylocked)
+                               if (!state->readonly)
                                {
                                        /* Get fresh copy of target page */
                                        state->target = palloc_btree_page(state, state->targetblock);
@@ -718,8 +718,7 @@ bt_target_page_check(BtreeCheckState *state)
                                        topaque = (BTPageOpaque) PageGetSpecialPointer(state->target);
 
                                        /*
-                                        * All !exclusivelylocked checks now performed; just
-                                        * return
+                                        * All !readonly checks now performed; just return
                                         */
                                        if (P_IGNORE(topaque))
                                                return;
@@ -740,11 +739,11 @@ bt_target_page_check(BtreeCheckState *state)
                 * * Downlink check *
                 *
                 * Additional check of child items iff this is an internal page and
-                * caller holds an ExclusiveLock.  This happens for every downlink
-                * (item) in target excluding the negative-infinity downlink (again,
-                * this is because it has no useful value to compare).
+                * caller holds a ShareLock.  This happens for every downlink (item)
+                * in target excluding the negative-infinity downlink (again, this is
+                * because it has no useful value to compare).
                 */
-               if (!P_ISLEAF(topaque) && state->exclusivelylocked)
+               if (!P_ISLEAF(topaque) && state->readonly)
                {
                        BlockNumber childblock = ItemPointerGetBlockNumber(&(itup->t_tid));
 
@@ -766,7 +765,7 @@ bt_target_page_check(BtreeCheckState *state)
  * with different parent page).  If no such valid item is available, return
  * NULL instead.
  *
- * Note that !exclusivelylocked callers must reverify that target page has not
+ * Note that !readonly callers must reverify that target page has not
  * been concurrently deleted.
  */
 static ScanKey
@@ -833,14 +832,14 @@ bt_right_page_check_scankey(BtreeCheckState *state)
        }
 
        /*
-        * No ExclusiveLock held case -- why it's safe to proceed.
+        * No ShareLock held case -- why it's safe to proceed.
         *
         * Problem:
         *
         * We must avoid false positive reports of corruption when caller treats
         * item returned here as an upper bound on target's last item.  In
         * general, false positives are disallowed.  Avoiding them here when
-        * caller is !exclusivelylocked is subtle.
+        * caller is !readonly is subtle.
         *
         * A concurrent page deletion by VACUUM of the target page can result in
         * the insertion of items on to this right sibling page that would
@@ -892,7 +891,7 @@ bt_right_page_check_scankey(BtreeCheckState *state)
         *
         * Top level tree walk caller moves on to next page (makes it the new
         * target) following recovery from this race.  (cf.  The rationale for
-        * child/downlink verification needing an ExclusiveLock within
+        * child/downlink verification needing a ShareLock within
         * bt_downlink_check(), where page deletion is also the main source of
         * trouble.)
         *
@@ -984,7 +983,7 @@ bt_downlink_check(BtreeCheckState *state, BlockNumber childblock,
        BTPageOpaque copaque;
 
        /*
-        * Caller must have ExclusiveLock on target relation, because of
+        * Caller must have ShareLock on target relation, because of
         * considerations around page deletion by VACUUM.
         *
         * NB: In general, page deletion deletes the right sibling's downlink, not
@@ -994,8 +993,8 @@ bt_downlink_check(BtreeCheckState *state, BlockNumber childblock,
         * page's rightmost child unless it is the last child page, and we intend
         * to also delete the parent itself.)
         *
-        * If this verification happened without an ExclusiveLock, the following
-        * race condition could cause false positives:
+        * If this verification happened without a ShareLock, the following race
+        * condition could cause false positives:
         *
         * In general, concurrent page deletion might occur, including deletion of
         * the left sibling of the child page that is examined here.  If such a
@@ -1009,11 +1008,11 @@ bt_downlink_check(BtreeCheckState *state, BlockNumber childblock,
         * part of deletion's first phase.)
         *
         * Note that while the cross-page-same-level last item check uses a trick
-        * that allows it to perform verification for !exclusivelylocked callers,
-        * a similar trick seems difficult here.  The trick that that other check
-        * uses is, in essence, to lock down race conditions to those that occur
-        * due to concurrent page deletion of the target; that's a race that can
-        * be reliably detected before actually reporting corruption.
+        * that allows it to perform verification for !readonly callers, a similar
+        * trick seems difficult here.  The trick that that other check uses is,
+        * in essence, to lock down race conditions to those that occur due to
+        * concurrent page deletion of the target; that's a race that can be
+        * reliably detected before actually reporting corruption.
         *
         * On the other hand, we'd need to lock down race conditions involving
         * deletion of child's left page, for long enough to read the child page
@@ -1021,7 +1020,7 @@ bt_downlink_check(BtreeCheckState *state, BlockNumber childblock,
         * locks on both child and left-of-child pages).  That's unacceptable for
         * amcheck functions on general principle, though.
         */
-       Assert(state->exclusivelylocked);
+       Assert(state->readonly);
 
        /*
         * Verify child page has the downlink key from target page (its parent) as