8000 Fix race condition with toast table access from a stale syscache entry. · jaylevitt/postgres@5e4dd5f · GitHub
[go: up one dir, main page]
Skip to content

Commit 5e4dd5f

Browse files
tglsfdctglsfdc
committed
Fix race condition with toast table access from a stale syscache entry.
If a tuple in a syscache contains an out-of-line toasted field, and we try to fetch that field shortly after some other transaction has committed an update or deletion of the tuple, there is a race condition: vacuum could come along and remove the toast tuples before we can fetch them. This leads to transient failures like "missing chunk number 0 for toast value NNNNN in pg_toast_2619", as seen in recent reports from Andrew Hammond and Tim Uckun. The design idea of syscache is that access to stale syscache entries should be prevented by relation-level locks, but that fails for at least two cases where toasted fields are possible: ANALYZE updates pg_statistic rows without locking out sessions that might want to plan queries on the same table, and CREATE OR REPLACE FUNCTION updates pg_proc rows without any meaningful lock at all. The least risky fix seems to be an idea that Heikki suggested when we were dealing with a related problem back in August: forcibly detoast any out-of-line fields before putting a tuple into syscache in the first place. This avoids the problem because at the time we fetch the parent tuple from the catalog, we should be holding an MVCC snapshot that will prevent removal of the toast tuples, even if the parent tuple is outdated immediately after we fetch it. (Note: I'm not convinced that this statement holds true at every instant where we could be fetching a syscache entry at all, but it does appear to hold true at the times where we could fetch an entry that could have a toasted field. We will need to be a bit wary of adding toast tables to low-level catalogs that don't have them already.) An additional benefit is that subsequent uses of the syscache entry should be faster, since they won't have to detoast the field. Back-patch to all supported versions. The problem is significantly harder to reproduce in pre-9.0 releases, because of their willingness to flush every entry in a syscache whenever the underlying catalog is vacuumed (cf CatalogCacheFlushRelation); but there is still a window for trouble.
1 parent 864c38c commit 5e4dd5f

File tree

3 files changed

+108
-1
lines changed

3 files changed

+108
-1
lines changed

‎src/backend/access/heap/tuptoaster.c

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,87 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
928928
}
929929

930930

931+
/* ----------
932+
* toast_flatten_tuple -
933+
*
934+
* "Flatten" a tuple to contain no out-of-line toasted fields.
935+
* (This does not eliminate compressed or short-header datums.)
936+
* ----------
937+
*/
938+
HeapTuple
939+
toast_flatten_tuple(HeapTuple tup, TupleDesc tupleDesc)
940+
{
941+
HeapTuple new_tuple;
942+
Form_pg_attribute *att = tupleDesc->attrs;
943+
int numAttrs = tupleDesc->natts;
944+
int i;
945+
Datum toast_values[MaxTupleAttributeNumber];
946+
bool toast_isnull[MaxTupleAttributeNumber];
947+
bool toast_free[MaxTupleAttributeNumber];
948+
949+
/*
950+
* Break down the tuple into fields.
951+
*/
952+
Assert(numAttrs <= MaxTupleAttributeNumber);
953+
heap_deform_tuple(tup, tupleDesc, toast_values, toast_isnull);
954+
955+
memset(toast_free, 0, numAttrs * sizeof(bool));
956+
957+
for (i = 0; i < numAttrs; i++)
958+
{
959+
/*
960+
* Look at non-null varlena attributes
961+
*/
962+
if (!toast_isnull[i] && att[i]->attlen == -1)
963+
{
964+
struct varlena *new_value;
965+
966+
new_value = (struct varlena *) DatumGetPointer(toast_values[i]);
967+
if (VARATT_IS_EXTERNAL(new_value))
968+
{
969+
new_value = toast_fetch_datum(new_value);
970+
toast_values[i] = PointerGetDatum(new_value);
971+
toast_free[i] = true;
972+
}
973+
}
974+
}
975+
976+
/*
977+
* Form the reconfigured tuple.
978+
*/
979+
new_tuple = heap_form_tuple(tupleDesc, toast_values, toast_isnull);
980+
981+
/*
982+
* Be sure to copy the tuple's OID and identity fields. We also make a
983+
* point of copying visibility info, just in case anybody looks at those
984+
* fields in a syscache entry.
985+
*/
986+
if (tupleDesc->tdhasoid)
987+
HeapTupleSetOid(new_tuple, HeapTupleGetOid(tup));
988+
989+
new_tuple->t_self = tup->t_self;
990+
new_tuple->t_tableOid = tup->t_tableOid;
991+
992+
new_tuple->t_data->t_choice = tup->t_data->t_choice;
993+
new_tuple->t_data->t_ctid = tup->t_data->t_ctid;
994+
new_tuple->t_data->t_infomask &= ~HEAP_XACT_MASK;
995+
new_tuple->t_data->t_infomask |=
996+
tup->t_data->t_infomask & HEAP_XACT_MASK;
997+
new_tuple->t_data->t_infomask2 &= ~HEAP2_XACT_MASK;
998+
new_tuple->t_data->t_infomask2 |=
999+
tup->t_data->t_infomask2 & HEAP2_XACT_MASK;
1000+
1001+
/*
1002+
* Free allocated temp values
1003+
*/
1004+
for (i = 0; i < numAttrs; i++)
1005+
if (toast_free[i])
1006+
pfree(DatumGetPointer(toast_values[i]));
1007+
1008+
return new_tuple;
1009+
}
1010+
1011+
9311012
/* ----------
9321013
* toast_flatten_tuple_attribute -
9331014
*

‎src/backend/utils/cache/catcache.c

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "access/heapam.h"
2020
#include "access/relscan.h"
2121
#include "access/sysattr.h"
22+
#include "access/tuptoaster.h"
2223
#include "access/valid.h"
2324
#include "catalog/pg_operator.h"
2425
#include "catalog/pg_type.h"
@@ -1598,16 +1599,32 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp,
15981599
uint32 hashValue, Index hashIndex, bool negative)
15991600
{
16001601
CatCTup *ct;
1602+
HeapTuple dtp;
16011603
MemoryContext oldcxt;
16021604

1605+
/*
1606+
* If there are any out-of-line toasted fields in the tuple, expand them
1607+
* in-line. This saves cycles during later use of the catcache entry,
1608+
* and also protects us against the possibility of the toast tuples being
1609+
* freed before we attempt to fetch them, in case of something using a
1610+
* slightly stale catcache entry.
1611+
*/
1612+
if (HeapTupleHasExternal(ntp))
1613+
dtp = toast_flatten_tuple(ntp, cache->cc_tupdesc);
1614+
else
1615+
dtp = ntp;
1616+
16031617
/*
16041618
* Allocate CatCTup header in cache memory, and copy the tuple there too.
16051619
*/
16061620
oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
16071621
ct = (CatCTup *) palloc(sizeof(CatCTup));
1608-
heap_copytuple_with_tuple(ntp, &ct->tuple);
1622+
heap_copytuple_with_tuple(dtp, &ct->tuple);
16091623
MemoryContextSwitchTo(oldcxt);
16101624

1625+
if (dtp != ntp)
1626+
heap_freetuple(dtp);
1627+
16111628
/*
16121629
* Finish initializing the CatCTup header, and add it to the cache's
16131630
* linked list and counts.

‎src/include/access/tuptoaster.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,15 @@ extern struct varlena *heap_tuple_untoast_attr_slice(struct varlena * attr,
144144
int32 sliceoffset,
145145
int32 slicelength);
146146

147+
/* ----------
148+
* toast_flatten_tuple -
149+
*
150+
* "Flatten" a tuple to contain no out-of-line toasted fields.
151+
* (This does not eliminate compressed or short-header datums.)
152+
* ----------
153+
*/
154+
extern HeapTuple toast_flatten_tuple(HeapTuple tup, TupleDesc tupleDesc);
155+
147156
/* ----------
148157
* toast_flatten_tuple_attribute -
149158
*

0 commit comments

Comments
 (0)
0