Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit ac8bc3b

Browse files
committed
Remove unnecessary relcache flushes after changing btree metapages.
These flushes were added in my commit d2896a9, which added the btree logic that keeps a cached copy of the index metapage data in index relcache entries. The idea was to ensure that other backends would promptly update their cached copies after a change. However, this is not really necessary, since _bt_getroot() has adequate defenses against believing a stale root page link, and _bt_getrootheight() doesn't have to be 100% right. Moreover, if it were necessary, a relcache flush would be an unreliable way to do it, since the sinval mechanism believes that relcache flush requests represent transactional updates, and therefore discards them on transaction rollback. Therefore, we might as well drop these flush requests and save the time to rebuild the whole relcache entry after a metapage change. If we ever try to support in-place truncation of btree indexes, it might be necessary to revisit this issue so that _bt_getroot() can't get caught by trying to follow a metapage link to a page that no longer exists. A possible solution to that is to make use of an smgr, rather than relcache, inval request to force other backends to discard their cached metapages. But for the moment this is not worth pursuing.
1 parent 14aa601 commit ac8bc3b

File tree

4 files changed

+14
-37
lines changed

4 files changed

+14
-37
lines changed

src/backend/access/nbtree/README

+11-8
Original file line numberDiff line numberDiff line change
@@ -438,14 +438,17 @@ location of the root page --- both the true root and the current effective
438438
root ("fast" root). To avoid fetching the metapage for every single index
439439
search, we cache a copy of the meta-data information in the index's
440440
relcache entry (rd_amcache). This is a bit ticklish since using the cache
441-
implies following a root page pointer that could be stale. We require
442-
every metapage update to send out a SI "relcache inval" message on the
443-
index relation. That ensures that each backend will flush its cached copy
444-
not later than the start of its next transaction. Therefore, stale
445-
pointers cannot be used for longer than the current transaction, which
446-
reduces the problem to the same one already dealt with for concurrent
447-
VACUUM --- we can just imagine that each open transaction is potentially
448-
"already in flight" to the old root.
441+
implies following a root page pointer that could be stale. However, a
442+
backend following a cached pointer can sufficiently verify whether it
443+
reached the intended page; either by checking the is-root flag when it
444+
is going to the true root, or by checking that the page has no siblings
445+
when going to the fast root. At worst, this could result in descending
446+
some extra tree levels if we have a cached pointer to a fast root that is
447+
now above the real fast root. Such cases shouldn't arise often enough to
448+
be worth optimizing; and in any case we can expect a relcache flush will
449+
discard the cached metapage before long, since a VACUUM that's moved the
450+
fast root pointer can be expected to issue a statistics update for the
451+
index.
449452

450453
The algorithm assumes we can fit at least three items per page
451454
(a "high key" and two real data items). Therefore it's unsafe

src/backend/access/nbtree/nbtinsert.c

+1-10
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include "miscadmin.h"
2222
#include "storage/lmgr.h"
2323
#include "storage/predicate.h"
24-
#include "utils/inval.h"
2524
#include "utils/tqual.h"
2625

2726

@@ -868,13 +867,9 @@ _bt_insertonpg(Relation rel,
868867

869868
END_CRIT_SECTION();
870869

871-
/* release buffers; send out relcache inval if metapage changed */
870+
/* release buffers */
872871
if (BufferIsValid(metabuf))
873-
{
874-
if (!InRecovery)
875-
CacheInvalidateRelcache(rel);
876872
_bt_relbuf(rel, metabuf);
877-
}
878873

879874
_bt_relbuf(rel, buf);
880875
}
@@ -1963,10 +1958,6 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf)
19631958

19641959
END_CRIT_SECTION();
19651960

1966-
/* send out relcache inval for metapage change */
1967-
if (!InRecovery)
1968-
CacheInvalidateRelcache(rel);
1969-
19701961
/* done with metapage */
19711962
_bt_relbuf(rel, metabuf);
19721963

src/backend/access/nbtree/nbtpage.c

+2-11
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "storage/indexfsm.h"
2929
#include "storage/lmgr.h"
3030
#include "storage/predicate.h"
31-
#include "utils/inval.h"
3231
#include "utils/snapmgr.h"
3332

3433

@@ -246,12 +245,6 @@ _bt_getroot(Relation rel, int access)
246245

247246
END_CRIT_SECTION();
248247

249-
/*
250-
* Send out relcache inval for metapage change (probably unnecessary
251-
* here, but let's be safe).
252-
*/
253-
CacheInvalidateRelcache(rel);
254-
255248
/*
256249
* swap root write lock for read lock. There is no danger of anyone
257250
* else accessing the new root page while it's unlocked, since no one
@@ -1545,12 +1538,10 @@ _bt_pagedel(Relation rel, Buffer buf, BTStack stack)
15451538

15461539
END_CRIT_SECTION();
15471540

1548-
/* release metapage; send out relcache inval if metapage changed */
1541+
/* release metapage */
15491542
if (BufferIsValid(metabuf))
1550-
{
1551-
CacheInvalidateRelcache(rel);
15521543
_bt_relbuf(rel, metabuf);
1553-
}
1544+
15541545
/* can always release leftsib immediately */
15551546
if (BufferIsValid(lbuf))
15561547
_bt_relbuf(rel, lbuf);

src/backend/access/nbtree/nbtree.c

-8
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,6 @@ btbuild(PG_FUNCTION_ARGS)
148148
}
149149
#endif /* BTREE_BUILD_STATS */
150150

151-
/*
152-
* If we are reindexing a pre-existing index, it is critical to send out a
153-
* relcache invalidation SI message to ensure all backends re-read the
154-
* index metapage. We expect that the caller will ensure that happens
155-
* (typically as a side effect of updating index stats, but it must happen
156-
* even if the stats don't change!)
157-
*/
158-
159151
/*
160152
* Return statistics
161153
*/

0 commit comments

Comments
 (0)