database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-10 09:21:54 +03:00
Code

amcheck: editorialize variable name & comment.

Browse Source 
No exclusive lock is taken anymore...
This commit is contained in:
Andres Freund 2017-03-09 20:01:37 -08:00
parent 56018bf26e
commit fcd8d25d38
1 changed files with 38 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
contrib/amcheck/verify_nbtree.c
View File

@ -55,8 +55,8 @@ typedef struct BtreeCheckState
/* B-Tree Index Relation */ /* B-Tree Index Relation */
Relation rel; Relation rel;
/* ExclusiveLock held? */ /* ShareLock held on heap/index, rather than AccessShareLock? */
bool exclusivelylocked; bool readonly;
/* Per-page context */ /* Per-page context */
MemoryContext targetcontext; MemoryContext targetcontext;
/* Buffer access strategy */ /* 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 void bt_index_check_internal(Oid indrelid, bool parentcheck);
static inline void btree_index_checkable(Relation rel); 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, static BtreeLevel bt_check_level_from_leftmost(BtreeCheckState *state,
BtreeLevel level); BtreeLevel level);
static void bt_target_page_check(BtreeCheckState *state); static void bt_target_page_check(BtreeCheckState *state);
@ -256,23 +256,23 @@ btree_index_checkable(Relation rel)
* logical order, verifying invariants as it goes. * logical order, verifying invariants as it goes.
* *
* It is the caller's responsibility to acquire appropriate heavyweight lock on * 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 * the index relation, and advise us if extra checks are safe when a ShareLock
* ExclusiveLock is held. * 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 * modification to the index structure, including modifications that VACUUM may
* make. This does not include setting of the LP_DEAD bit by concurrent index * 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 * 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 * any check performed here. Any concurrent process that might act on the
* LP_DEAD bit being set (recycle space) requires a heavyweight lock that * 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 * 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 * 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 * trouble. _bt_delitems_vacuum() may only delete leaf items, and so the extra
* parent/child check cannot be affected.) * parent/child check cannot be affected.)
*/ */
static void static void
bt_check_every_level(Relation rel, bool exclusivelylocked) bt_check_every_level(Relation rel, bool readonly)
{ {
BtreeCheckState *state; BtreeCheckState *state;
Page metapage; Page metapage;
@ -291,7 +291,7 @@ bt_check_every_level(Relation rel, bool exclusivelylocked)
*/ */
state = palloc(sizeof(BtreeCheckState)); state = palloc(sizeof(BtreeCheckState));
state->rel = rel; state->rel = rel;
state->exclusivelylocked = exclusivelylocked; state->readonly = readonly;
/* Create context for page */ /* Create context for page */
state->targetcontext = AllocSetContextCreate(CurrentMemoryContext, state->targetcontext = AllocSetContextCreate(CurrentMemoryContext,
"amcheck context", "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 * 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, * 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 * working their way down by passing what is returned for the last call here
* until level 0 (leaf page level) was reached. * 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 * locking, check that the first valid page meets caller's
* expectations. * expectations.
*/ */
if (state->exclusivelylocked) if (state->readonly)
{ {
if (!P_LEFTMOST(opaque)) if (!P_LEFTMOST(opaque))
ereport(ERROR, 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, ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED), (errcode(ERRCODE_INDEX_CORRUPTED),
errmsg("left link/right link pair in index \"%s\" not in agreement", 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 * "real" data item on the page to the right (if such a first item is
* available). * 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: * internal page, function also checks:
* *
* - That all child pages respect downlinks lower bound. * - That all child pages respect downlinks lower bound.
@ -597,8 +597,8 @@ bt_target_page_check(BtreeCheckState *state)
* page items. * page items.
* *
* We prefer to check all items against high key rather than checking * We prefer to check all items against high key rather than checking
* just the first and trusting that the operator class obeys the * just the last and trusting that the operator class obeys the
* transitive law (which implies that all subsequent items also * transitive law (which implies that all previous items also
* respected the high key invariant if they pass the item order * respected the high key invariant if they pass the item order
* check). * check).
* *
@ -705,12 +705,12 @@ bt_target_page_check(BtreeCheckState *state)
{ {
/* /*
* As explained at length in bt_right_page_check_scankey(), * As explained at length in bt_right_page_check_scankey(),
* there is a known !exclusivelylocked race that could account * there is a known !readonly race that could account for
* for apparent violation of invariant, which we must check * apparent violation of invariant, which we must check for
* for before actually proceeding with raising error. Our * before actually proceeding with raising error. Our canary
* canary condition is that target page was deleted. * condition is that target page was deleted.
*/ */
if (!state->exclusivelylocked) if (!state->readonly)
{ {
/* Get fresh copy of target page */ /* Get fresh copy of target page */
state->target = palloc_btree_page(state, state->targetblock); state->target = palloc_btree_page(state, state->targetblock);
@ -718,8 +718,7 @@ bt_target_page_check(BtreeCheckState *state)
topaque = (BTPageOpaque) PageGetSpecialPointer(state->target); topaque = (BTPageOpaque) PageGetSpecialPointer(state->target);
/* /*
* All !exclusivelylocked checks now performed; just * All !readonly checks now performed; just return
* return
*/ */
if (P_IGNORE(topaque)) if (P_IGNORE(topaque))
return; return;
@ -740,11 +739,11 @@ bt_target_page_check(BtreeCheckState *state)
* * Downlink check * * * Downlink check *
* *
* Additional check of child items iff this is an internal page and * Additional check of child items iff this is an internal page and
* caller holds an ExclusiveLock. This happens for every downlink * caller holds a ShareLock. This happens for every downlink (item)
* (item) in target excluding the negative-infinity downlink (again, * in target excluding the negative-infinity downlink (again, this is
* this is because it has no useful value to compare). * 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)); 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 * with different parent page). If no such valid item is available, return
* NULL instead. * 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. * been concurrently deleted.
*/ */
static ScanKey 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: * Problem:
* *
* We must avoid false positive reports of corruption when caller treats * We must avoid false positive reports of corruption when caller treats
* item returned here as an upper bound on target's last item. In * item returned here as an upper bound on target's last item. In
* general, false positives are disallowed. Avoiding them here when * 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 * 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 * 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 * Top level tree walk caller moves on to next page (makes it the new
* target) following recovery from this race. (cf. The rationale for * 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 * bt_downlink_check(), where page deletion is also the main source of
* trouble.) * trouble.)
* *
@ -984,7 +983,7 @@ bt_downlink_check(BtreeCheckState *state, BlockNumber childblock,
BTPageOpaque copaque; 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. * considerations around page deletion by VACUUM.
* *
* NB: In general, page deletion deletes the right sibling's downlink, not * 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 * page's rightmost child unless it is the last child page, and we intend
* to also delete the parent itself.) * to also delete the parent itself.)
* *
* If this verification happened without an ExclusiveLock, the following * If this verification happened without a ShareLock, the following race
* race condition could cause false positives: * condition could cause false positives:
* *
* In general, concurrent page deletion might occur, including deletion of * In general, concurrent page deletion might occur, including deletion of
* the left sibling of the child page that is examined here. If such a * 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.) * part of deletion's first phase.)
* *
* Note that while the cross-page-same-level last item check uses a trick * Note that while the cross-page-same-level last item check uses a trick
* that allows it to perform verification for !exclusivelylocked callers, * that allows it to perform verification for !readonly callers, a similar
* a similar trick seems difficult here. The trick that that other check * trick seems difficult here. The trick that that other check uses is,
* uses is, in essence, to lock down race conditions to those that occur * in essence, to lock down race conditions to those that occur due to
* due to concurrent page deletion of the target; that's a race that can * concurrent page deletion of the target; that's a race that can be
* be reliably detected before actually reporting corruption. * reliably detected before actually reporting corruption.
* *
* On the other hand, we'd need to lock down race conditions involving * 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 * 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 * locks on both child and left-of-child pages). That's unacceptable for
* amcheck functions on general principle, though. * 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 * Verify child page has the downlink key from target page (its parent) as