From 37b1af07998a249eb6b254ae420a0440d67d3de4 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 29 Aug 2021 16:01:13 -0500 Subject: [PATCH v2 2/3] refactor the code to make sure that logic is not duplicated. 0001-write-vm-during-cluster-4.patch Date: Mon, 28 Jun 2021 11:22:01 +0300 From: Anna Akenteva --- src/backend/access/heap/heapam_handler.c | 19 +++--- src/backend/access/heap/rewriteheap.c | 12 +++- src/backend/access/heap/vacuumlazy.c | 75 ++++++++++++++++-------- src/backend/utils/sort/tuplesort.c | 9 ++- src/include/access/heapam.h | 3 + src/include/access/rewriteheap.h | 4 +- src/include/utils/tuplesort.h | 4 +- 7 files changed, 87 insertions(+), 39 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 9befe012a9..90361080d0 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -47,7 +47,8 @@ static void reform_and_rewrite_tuple(HeapTuple tuple, Relation OldHeap, Relation NewHeap, - Datum *values, bool *isnull, RewriteState rwstate); + Datum *values, bool *isnull, bool islive, + RewriteState rwstate); static bool SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer, HeapTuple tuple, @@ -782,6 +783,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, HeapTuple tuple; Buffer buf; bool isdead; + bool islive = false; CHECK_FOR_INTERRUPTS(); @@ -848,6 +850,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, case HEAPTUPLE_LIVE: /* Live or recently dead, must copy it */ isdead = false; + islive = true; break; case HEAPTUPLE_INSERT_IN_PROGRESS: @@ -903,7 +906,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, *num_tuples += 1; if (tuplesort != NULL) { - tuplesort_putheaptuple(tuplesort, tuple); + tuplesort_putheaptuple(tuplesort, tuple, islive); /* * In scan-and-sort mode, report increase in number of tuples @@ -921,7 +924,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, int64 ct_val[2]; reform_and_rewrite_tuple(tuple, OldHeap, NewHeap, - values, isnull, rwstate); + values, isnull, islive, rwstate); /* * In indexscan mode and also VACUUM FULL, report increase in @@ -961,17 +964,18 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, for (;;) { HeapTuple tuple; + bool islive; CHECK_FOR_INTERRUPTS(); - tuple = tuplesort_getheaptuple(tuplesort, true); + tuple = tuplesort_getheaptuple(tuplesort, true, &islive); if (tuple == NULL) break; n_tuples += 1; reform_and_rewrite_tuple(tuple, OldHeap, NewHeap, - values, isnull, + values, isnull, islive, rwstate); /* Report n_tuples */ pgstat_progress_update_param(PROGRESS_CLUSTER_HEAP_TUPLES_WRITTEN, @@ -2458,7 +2462,8 @@ heapam_scan_sample_next_tuple(TableScanDesc scan, SampleScanState *scanstate, static void reform_and_rewrite_tuple(HeapTuple tuple, Relation OldHeap, Relation NewHeap, - Datum *values, bool *isnull, RewriteState rwstate) + Datum *values, bool *isnull, bool islive, + RewriteState rwstate) { TupleDesc oldTupDesc = RelationGetDescr(OldHeap); TupleDesc newTupDesc = RelationGetDescr(NewHeap); @@ -2477,7 +2482,7 @@ reform_and_rewrite_tuple(HeapTuple tuple, copiedTuple = heap_form_tuple(newTupDesc, values, isnull); /* The heap rewrite module does the rest */ - rewrite_heap_tuple(rwstate, tuple, copiedTuple); + rewrite_heap_tuple(rwstate, islive, tuple, copiedTuple); heap_freetuple(copiedTuple); } diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index af0d1b2d0c..623149ac26 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -424,6 +424,8 @@ rewrite_set_vm_flags(RewriteState state) map[mapByte] |= (flags << mapOffset); } +// rewrite_update_vm_flags(state, new_tuple); + /* * Update rs_all_visible and rs_all_frozen flags according to the tuple. We * use simplified check assuming that HeapTupleSatisfiesVacuum() should already @@ -475,11 +477,12 @@ rewrite_update_vm_flags(RewriteState state, HeapTuple tuple) * it had better be temp storage not a pointer to the original tuple. * * state opaque state as returned by begin_heap_rewrite + * is_live whether the currently processed tuple is live * old_tuple original tuple in the old heap * new_tuple new, rewritten tuple to be inserted to new heap */ void -rewrite_heap_tuple(RewriteState state, +rewrite_heap_tuple(RewriteState state, bool is_live, HeapTuple old_tuple, HeapTuple new_tuple) { MemoryContext old_cxt; @@ -593,7 +596,12 @@ rewrite_heap_tuple(RewriteState state, /* Insert the tuple and find out where it's put in new_heap */ raw_heap_insert(state, new_tuple); - rewrite_update_vm_flags(state, new_tuple); + + heap_page_update_vm_flags(new_tuple, state->rs_oldest_xmin, + is_live, true, + &(state->rs_all_visible), + &(state->rs_all_frozen)); + new_tid = new_tuple->t_self; logical_rewrite_heap_tuple(state, old_tid, new_tuple); diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 558cc88a08..25ac3e7b51 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -3605,6 +3605,53 @@ vac_cmp_itemptr(const void *left, const void *right) return 0; } +/* + * Update all_visible and all_frozen flags according to the tuple. We + * use simplified check assuming that HeapTupleSatisfiesVacuum() should already + * set tuple hint bits. + */ +void +heap_page_update_vm_flags(HeapTuple tuple, TransactionId OldestXmin, + bool is_live, bool check_infomask, + bool *all_visible, bool *all_frozen) +{ + TransactionId xmin; + + /* Check comments in lazy_scan_prune. */ + if (!HeapTupleHeaderXminCommitted(tuple->t_data) || !is_live) + { + *all_visible = false; + *all_frozen = false; + return; + } + + /* + * The inserter definitely committed. But is it old enough + * that everyone sees it as committed? + */ + xmin = HeapTupleHeaderGetXmin(tuple->t_data); + if (!TransactionIdPrecedes(xmin, OldestXmin)) + { + *all_visible = false; + *all_frozen = false; + return; + } + + if (check_infomask && + !(tuple->t_data->t_infomask & HEAP_XMAX_INVALID) && + !HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_data->t_infomask)) + { + *all_visible = false; + *all_frozen = false; + return; + } + + /* Check whether this tuple is already frozen or not */ + if (*all_visible && *all_frozen && + heap_tuple_needs_eventual_freeze(tuple->t_data)) + *all_frozen = false; +} + /* * Check if every tuple in the given page is visible to all current and future * transactions. Also return the visibility_cutoff_xid which is the highest @@ -3673,34 +3720,14 @@ heap_page_is_all_visible(LVRelState *vacrel, Buffer buf, { TransactionId xmin; - /* Check comments in lazy_scan_prune. */ - if (!HeapTupleHeaderXminCommitted(tuple.t_data)) - { - all_visible = false; - *all_frozen = false; - break; - } - - /* - * The inserter definitely committed. But is it old enough - * that everyone sees it as committed? - */ - xmin = HeapTupleHeaderGetXmin(tuple.t_data); - if (!TransactionIdPrecedes(xmin, vacrel->OldestXmin)) - { - all_visible = false; - *all_frozen = false; - break; - } + heap_page_update_vm_flags(&tuple, vacrel->OldestXmin, + true, false, + &all_visible, all_frozen); /* Track newest xmin on page. */ + xmin = HeapTupleHeaderGetXmin(tuple.t_data); if (TransactionIdFollows(xmin, *visibility_cutoff_xid)) *visibility_cutoff_xid = xmin; - - /* Check whether this tuple is already frozen or not */ - if (all_visible && *all_frozen && - heap_tuple_needs_eventual_freeze(tuple.t_data)) - *all_frozen = false; } break; diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index 90e26745df..97b783e4a1 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -184,6 +184,7 @@ typedef struct Datum datum1; /* value of first key column */ bool isnull1; /* is first key column NULL? */ int srctape; /* source tape number */ + bool is_live; /* is the tuple alive? */ } SortTuple; /* @@ -1706,7 +1707,7 @@ tuplesort_puttupleslot(Tuplesortstate *state, TupleTableSlot *slot) * Note that the input data is always copied; the caller need not save it. */ void -tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup) +tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup, bool is_live) { MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext); SortTuple stup; @@ -1718,6 +1719,7 @@ tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup) COPYTUP(state, &stup, (void *) tup); puttuple_common(state, &stup); + stup.is_live = is_live; MemoryContextSwitchTo(oldcontext); } @@ -2442,7 +2444,7 @@ tuplesort_gettupleslot(Tuplesortstate *state, bool forward, bool copy, * remaining valid after any further manipulation of tuplesort. */ HeapTuple -tuplesort_getheaptuple(Tuplesortstate *state, bool forward) +tuplesort_getheaptuple(Tuplesortstate *state, bool forward, bool *is_live) { MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext); SortTuple stup; @@ -2452,6 +2454,9 @@ tuplesort_getheaptuple(Tuplesortstate *state, bool forward) MemoryContextSwitchTo(oldcontext); + if (is_live) + *is_live = stup.is_live; + return stup.tuple; } diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index e63b49fc38..71bda855f2 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -199,6 +199,9 @@ struct VacuumParams; extern void heap_vacuum_rel(Relation rel, struct VacuumParams *params, BufferAccessStrategy bstrategy); extern void parallel_vacuum_main(dsm_segment *seg, shm_toc *toc); +extern void heap_page_update_vm_flags(HeapTuple tuple, TransactionId OldestXmin, + bool is_live, bool check_infomask, + bool *all_visible, bool *all_frozen); /* in heap/heapam_visibility.c */ extern bool HeapTupleSatisfiesVisibility(HeapTuple stup, Snapshot snapshot, diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 121f552405..2cda7320d3 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -25,8 +25,8 @@ extern RewriteState begin_heap_rewrite(Relation OldHeap, Relation NewHeap, TransactionId OldestXmin, TransactionId FreezeXid, MultiXactId MultiXactCutoff); extern void end_heap_rewrite(RewriteState state); -extern void rewrite_heap_tuple(RewriteState state, HeapTuple oldTuple, - HeapTuple newTuple); +extern void rewrite_heap_tuple(RewriteState state, bool is_live, + HeapTuple oldTuple, HeapTuple newTuple); extern bool rewrite_heap_dead_tuple(RewriteState state, HeapTuple oldTuple); /* diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h index f94949370b..45442c4351 100644 --- a/src/include/utils/tuplesort.h +++ b/src/include/utils/tuplesort.h @@ -232,7 +232,7 @@ extern bool tuplesort_used_bound(Tuplesortstate *state); extern void tuplesort_puttupleslot(Tuplesortstate *state, TupleTableSlot *slot); -extern void tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup); +extern void tuplesort_putheaptuple(Tuplesortstate *state, HeapTuple tup, bool is_live); extern void tuplesort_putindextuplevalues(Tuplesortstate *state, Relation rel, ItemPointer self, Datum *values, bool *isnull); @@ -243,7 +243,7 @@ extern void tuplesort_performsort(Tuplesortstate *state); extern bool tuplesort_gettupleslot(Tuplesortstate *state, bool forward, bool copy, TupleTableSlot *slot, Datum *abbrev); -extern HeapTuple tuplesort_getheaptuple(Tuplesortstate *state, bool forward); +extern HeapTuple tuplesort_getheaptuple(Tuplesortstate *state, bool forward, bool *is_live); extern IndexTuple tuplesort_getindextuple(Tuplesortstate *state, bool forward); extern bool tuplesort_getdatum(Tuplesortstate *state, bool forward, Datum *val, bool *isNull, Datum *abbrev); -- 2.17.0