public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 1/2] Added offset with block number in vacuum errcontext
6+ messages / 4 participants
[nested] [flat]
* [PATCH 1/2] Added offset with block number in vacuum errcontext
@ 2020-07-28 18:53 Mahendra Singh Thalor <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Mahendra Singh Thalor @ 2020-07-28 18:53 UTC (permalink / raw)
---
src/backend/access/heap/vacuumlazy.c | 62 +++++++++++++++++++++-------
1 file changed, 46 insertions(+), 16 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 1bbc4598f7..78d1db9ae2 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -316,6 +316,7 @@ typedef struct LVRelStats
/* Used for error callback */
char *indname;
BlockNumber blkno; /* used only for heap operations */
+ OffsetNumber offnum;
VacErrPhase phase;
} LVRelStats;
@@ -323,6 +324,7 @@ typedef struct LVRelStats
typedef struct LVSavedErrInfo
{
BlockNumber blkno;
+ OffsetNumber offnum;
VacErrPhase phase;
} LVSavedErrInfo;
@@ -341,7 +343,8 @@ static void lazy_scan_heap(Relation onerel, VacuumParams *params,
LVRelStats *vacrelstats, Relation *Irel, int nindexes,
bool aggressive);
static void lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats);
-static bool lazy_check_needs_freeze(Buffer buf, bool *hastup);
+static bool lazy_check_needs_freeze(Buffer buf, bool *hastup,
+ LVRelStats *vacrelstats);
static void lazy_vacuum_all_indexes(Relation onerel, Relation *Irel,
IndexBulkDeleteResult **stats,
LVRelStats *vacrelstats, LVParallelState *lps,
@@ -364,6 +367,7 @@ static void lazy_record_dead_tuple(LVDeadTuples *dead_tuples,
static bool lazy_tid_reaped(ItemPointer itemptr, void *state);
static int vac_cmp_itemptr(const void *left, const void *right);
static bool heap_page_is_all_visible(Relation rel, Buffer buf,
+ LVRelStats *vacrelstats,
TransactionId *visibility_cutoff_xid, bool *all_frozen);
static void lazy_parallel_vacuum_indexes(Relation *Irel, IndexBulkDeleteResult **stats,
LVRelStats *vacrelstats, LVParallelState *lps,
@@ -396,7 +400,8 @@ static LVSharedIndStats *get_indstats(LVShared *lvshared, int n);
static bool skip_parallel_vacuum_index(Relation indrel, LVShared *lvshared);
static void vacuum_error_callback(void *arg);
static void update_vacuum_error_info(LVRelStats *errinfo, LVSavedErrInfo *saved_err_info,
- int phase, BlockNumber blkno);
+ int phase, BlockNumber blkno,
+ OffsetNumber offnum);
static void restore_vacuum_error_info(LVRelStats *errinfo, const LVSavedErrInfo *saved_err_info);
@@ -547,7 +552,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
* revert to the previous phase.
*/
update_vacuum_error_info(vacrelstats, NULL, VACUUM_ERRCB_PHASE_TRUNCATE,
- vacrelstats->nonempty_pages);
+ vacrelstats->nonempty_pages,
+ InvalidOffsetNumber);
lazy_truncate_heap(onerel, vacrelstats);
}
@@ -957,7 +963,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
update_vacuum_error_info(vacrelstats, NULL, VACUUM_ERRCB_PHASE_SCAN_HEAP,
- blkno);
+ blkno, InvalidOffsetNumber);
if (blkno == next_unskippable_block)
{
@@ -1126,7 +1132,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
* to use lazy_check_needs_freeze() for both situations, though.
*/
LockBuffer(buf, BUFFER_LOCK_SHARE);
- if (!lazy_check_needs_freeze(buf, &hastup))
+ if (!lazy_check_needs_freeze(buf, &hastup, vacrelstats))
{
UnlockReleaseBuffer(buf);
vacrelstats->scanned_pages++;
@@ -1263,6 +1269,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
{
ItemId itemid;
+ vacrelstats->offnum = offnum;
itemid = PageGetItemId(page, offnum);
/* Unused items require no processing, but we count 'em */
@@ -1836,7 +1843,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
/* Update error traceback information */
update_vacuum_error_info(vacrelstats, &saved_err_info, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
- InvalidBlockNumber);
+ InvalidBlockNumber, InvalidOffsetNumber);
pg_rusage_init(&ru0);
npages = 0;
@@ -1853,6 +1860,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
tblk = ItemPointerGetBlockNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]);
vacrelstats->blkno = tblk;
+ vacrelstats->offnum = ItemPointerGetOffsetNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]);
buf = ReadBufferExtended(onerel, MAIN_FORKNUM, tblk, RBM_NORMAL,
vac_strategy);
if (!ConditionalLockBufferForCleanup(buf))
@@ -1915,7 +1923,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
/* Update error traceback information */
update_vacuum_error_info(vacrelstats, &saved_err_info, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
- blkno);
+ blkno, InvalidOffsetNumber);
START_CRIT_SECTION();
@@ -1929,6 +1937,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
if (tblk != blkno)
break; /* past end of tuples for this block */
toff = ItemPointerGetOffsetNumber(&dead_tuples->itemptrs[tupindex]);
+ vacrelstats->offnum = toff;
itemid = PageGetItemId(page, toff);
ItemIdSetUnused(itemid);
unused[uncnt++] = toff;
@@ -1967,7 +1976,8 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
* dirty, exclusively locked, and, if needed, a full page image has been
* emitted in the log_heap_clean() above.
*/
- if (heap_page_is_all_visible(onerel, buffer, &visibility_cutoff_xid,
+ if (heap_page_is_all_visible(onerel, buffer, vacrelstats,
+ &visibility_cutoff_xid,
&all_frozen))
PageSetAllVisible(page);
@@ -2006,7 +2016,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
* Also returns a flag indicating whether page contains any tuples at all.
*/
static bool
-lazy_check_needs_freeze(Buffer buf, bool *hastup)
+lazy_check_needs_freeze(Buffer buf, bool *hastup, LVRelStats *vacrelstats)
{
Page page = BufferGetPage(buf);
OffsetNumber offnum,
@@ -2031,6 +2041,7 @@ lazy_check_needs_freeze(Buffer buf, bool *hastup)
{
ItemId itemid;
+ vacrelstats->offnum = offnum;
itemid = PageGetItemId(page, offnum);
/* this should match hastup test in count_nondeletable_pages() */
@@ -2426,7 +2437,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
update_vacuum_error_info(vacrelstats, &saved_err_info,
VACUUM_ERRCB_PHASE_VACUUM_INDEX,
- InvalidBlockNumber);
+ InvalidBlockNumber, InvalidOffsetNumber);
/* Do bulk deletion */
*stats = index_bulk_delete(&ivinfo, *stats,
@@ -2486,7 +2497,7 @@ lazy_cleanup_index(Relation indrel,
vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
update_vacuum_error_info(vacrelstats, &saved_err_info,
VACUUM_ERRCB_PHASE_INDEX_CLEANUP,
- InvalidBlockNumber);
+ InvalidBlockNumber, InvalidOffsetNumber);
*stats = index_vacuum_cleanup(&ivinfo, *stats);
@@ -2640,6 +2651,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
*/
new_rel_pages = count_nondeletable_pages(onerel, vacrelstats);
vacrelstats->blkno = new_rel_pages;
+ vacrelstats->offnum = InvalidOffsetNumber;
if (new_rel_pages >= old_rel_pages)
{
@@ -2952,6 +2964,7 @@ vac_cmp_itemptr(const void *left, const void *right)
*/
static bool
heap_page_is_all_visible(Relation rel, Buffer buf,
+ LVRelStats *vacrelstats,
TransactionId *visibility_cutoff_xid,
bool *all_frozen)
{
@@ -2976,6 +2989,7 @@ heap_page_is_all_visible(Relation rel, Buffer buf,
ItemId itemid;
HeapTupleData tuple;
+ vacrelstats->offnum = offnum;
itemid = PageGetItemId(page, offnum);
/* Unused or redirect line pointers are of no interest */
@@ -3574,14 +3588,26 @@ vacuum_error_callback(void *arg)
{
case VACUUM_ERRCB_PHASE_SCAN_HEAP:
if (BlockNumberIsValid(errinfo->blkno))
- errcontext("while scanning block %u of relation \"%s.%s\"",
- errinfo->blkno, errinfo->relnamespace, errinfo->relname);
+ {
+ if (OffsetNumberIsValid(errinfo->offnum))
+ errcontext("while scanning block %u and offset %u of relation \"%s.%s\"",
+ errinfo->blkno, errinfo->offnum, errinfo->relnamespace, errinfo->relname);
+ else
+ errcontext("while scanning block %u of relation \"%s.%s\"",
+ errinfo->blkno, errinfo->relnamespace, errinfo->relname);
+ }
break;
case VACUUM_ERRCB_PHASE_VACUUM_HEAP:
if (BlockNumberIsValid(errinfo->blkno))
- errcontext("while vacuuming block %u of relation \"%s.%s\"",
- errinfo->blkno, errinfo->relnamespace, errinfo->relname);
+ {
+ if (OffsetNumberIsValid(errinfo->offnum))
+ errcontext("while vacuuming block %u and offset %u of relation \"%s.%s\"",
+ errinfo->blkno, errinfo->offnum, errinfo->relnamespace, errinfo->relname);
+ else
+ errcontext("while vacuuming block %u of relation \"%s.%s\"",
+ errinfo->blkno, errinfo->relnamespace, errinfo->relname);
+ }
break;
case VACUUM_ERRCB_PHASE_VACUUM_INDEX:
@@ -3589,6 +3615,7 @@ vacuum_error_callback(void *arg)
errinfo->indname, errinfo->relnamespace, errinfo->relname);
break;
+
case VACUUM_ERRCB_PHASE_INDEX_CLEANUP:
errcontext("while cleaning up index \"%s\" of relation \"%s.%s\"",
errinfo->indname, errinfo->relnamespace, errinfo->relname);
@@ -3613,15 +3640,17 @@ vacuum_error_callback(void *arg)
*/
static void
update_vacuum_error_info(LVRelStats *errinfo, LVSavedErrInfo *saved_err_info, int phase,
- BlockNumber blkno)
+ BlockNumber blkno, OffsetNumber offnum)
{
if (saved_err_info)
{
+ saved_err_info->offnum = errinfo->offnum;
saved_err_info->blkno = errinfo->blkno;
saved_err_info->phase = errinfo->phase;
}
errinfo->blkno = blkno;
+ errinfo->offnum = offnum;
errinfo->phase = phase;
}
@@ -3632,5 +3661,6 @@ static void
restore_vacuum_error_info(LVRelStats *errinfo, const LVSavedErrInfo *saved_err_info)
{
errinfo->blkno = saved_err_info->blkno;
+ errinfo->offnum = saved_err_info->offnum;
errinfo->phase = saved_err_info->phase;
}
--
2.17.0
--OQhbRXNHSL5w/5po
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0002-fix.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Statistics Import and Export
@ 2025-04-01 22:05 Jeff Davis <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Davis @ 2025-04-01 22:05 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; Corey Huinker <[email protected]>; +Cc: Robert Treat <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Michael Paquier <[email protected]>; jian he <[email protected]>; Bruce Momjian <[email protected]>; Matthias van de Meent <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Ashutosh Bapat <[email protected]>; Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]
On Tue, 2025-04-01 at 13:44 -0500, Nathan Bossart wrote:
> Apologies for the noise. I noticed one more way to simplify 0002.
> As
> before, there should be no functional differences.
To restate the problem: one of the problems being solved here is that
the existing code for custom-format dumps calls WriteToc twice. That
was not a big problem before this patch, when the contents of the
entries was easily accessible in memory. But the point of 0002 is to
avoid keeping all of the stats in memory at once, because that causes
bloat; and instead to query it on demand.
In theory, we could fix the pre-existing code by making the second pass
able to jump over the other contents of the entry and just update the
data offsets. But that seems invasive, at least to do it properly.
0001 sidesteps the problem by skipping the second pass if data's not
being dumped (because there are no offsets that need updating). The
worst case is when there are a lot of objects with a small amount of
data. But that's a worst case for stats in general, so I don't think
that needs to be solved here.
Issuing the stats queries twice is not great, though. If there's any
non-deterministic output in the query, that could lead to strangeness.
How bad can that be? If the results change in some way that looks
benign, but changes the length of the definition string, can it lead to
corruption of a ToC entry? I'm not saying there's a problem, but trying
to understand the risk of future problems.
For 0003, it makes an assumption about the way the scan happens in
WriteToc(). Can you add some additional sanity checks to verify that
something doesn't happen in a different order than we expect?
Also, why do we need the clause "WHERE s.tablename = ANY($2)"? Isn't
that already implied by "JOIN unnest($1, $2) ... s.tablename =
u.tablename"?
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Statistics Import and Export
@ 2025-04-02 03:21 Nathan Bossart <[email protected]>
parent: Jeff Davis <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Bossart @ 2025-04-02 03:21 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Corey Huinker <[email protected]>; Robert Treat <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Michael Paquier <[email protected]>; jian he <[email protected]>; Bruce Momjian <[email protected]>; Matthias van de Meent <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Ashutosh Bapat <[email protected]>; Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]
On Tue, Apr 01, 2025 at 03:05:59PM -0700, Jeff Davis wrote:
> To restate the problem: one of the problems being solved here is that
> the existing code for custom-format dumps calls WriteToc twice. That
> was not a big problem before this patch, when the contents of the
> entries was easily accessible in memory. But the point of 0002 is to
> avoid keeping all of the stats in memory at once, because that causes
> bloat; and instead to query it on demand.
>
> In theory, we could fix the pre-existing code by making the second pass
> able to jump over the other contents of the entry and just update the
> data offsets. But that seems invasive, at least to do it properly.
>
> 0001 sidesteps the problem by skipping the second pass if data's not
> being dumped (because there are no offsets that need updating). The
> worst case is when there are a lot of objects with a small amount of
> data. But that's a worst case for stats in general, so I don't think
> that needs to be solved here.
>
> Issuing the stats queries twice is not great, though. If there's any
> non-deterministic output in the query, that could lead to strangeness.
> How bad can that be? If the results change in some way that looks
> benign, but changes the length of the definition string, can it lead to
> corruption of a ToC entry? I'm not saying there's a problem, but trying
> to understand the risk of future problems.
It certainly feels risky. I was able to avoid executing the queries twice
in all cases by saving the definition length in the TOC entry and skipping
that many bytes the second time round. That's simple enough, but it relies
on various assumptions such as fseeko() being available (IIUC the file will
only be open for writing so we cannot fall back on fread()) and WriteStr()
returning an accurate value (which I'm skeptical of because some formats
compress this data). But AFAICT custom format is the only format that does
a second WriteToc() pass at the moment, and it only does so when fseeko()
is usable. Plus, custom format doesn't appear to compress anything written
via WriteStr().
We might be able to improve this by inventing a new callback that fails for
all formats except for custom with feesko() available. That would at least
ensure hard failures if these assumptions change. That problably wouldn't
be terribly invasive. I'm curious what you think.
> For 0003, it makes an assumption about the way the scan happens in
> WriteToc(). Can you add some additional sanity checks to verify that
> something doesn't happen in a different order than we expect?
Hm. One thing we could do is to send the TocEntry to the callback and
verify that matches the one we were expecting to see next (as set by a
previous call). Does that sound like a strong enough check? FWIW the
pg_dump tests failed miserably until Corey and I got this part right, so
our usual tests should also offer some assurance.
> Also, why do we need the clause "WHERE s.tablename = ANY($2)"? Isn't
> that already implied by "JOIN unnest($1, $2) ... s.tablename =
> u.tablename"?
Good question. Corey, do you recall why this was needed?
--
nathan
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Statistics Import and Export
@ 2025-04-02 05:44 Jeff Davis <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 2 replies; 6+ messages in thread
From: Jeff Davis @ 2025-04-02 05:44 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Corey Huinker <[email protected]>; Robert Treat <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Michael Paquier <[email protected]>; jian he <[email protected]>; Bruce Momjian <[email protected]>; Matthias van de Meent <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Ashutosh Bapat <[email protected]>; Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]
On Tue, 2025-04-01 at 22:21 -0500, Nathan Bossart wrote:
> It certainly feels risky. I was able to avoid executing the queries
> twice
> in all cases by saving the definition length in the TOC entry and
> skipping
> that many bytes the second time round.
That feels like a better approach.
> That's simple enough, but it relies
> on various assumptions such as fseeko() being available (IIUC the
> file will
> only be open for writing so we cannot fall back on fread()) and
> WriteStr()
> returning an accurate value (which I'm skeptical of because some
> formats
> compress this data). But AFAICT custom format is the only format
> that does
> a second WriteToc() pass at the moment, and it only does so when
> fseeko()
> is usable.
Even with those assumptions, I think it's much better than querying
twice and assuming that the results are the same.
> Plus, custom format doesn't appear to compress anything written
> via WriteStr().
If WriteStr() was doing compression, that would make the second
WriteToc() pass to update the data offsets scary even in the existing
code.
> We might be able to improve this by inventing a new callback that
> fails for
> all formats except for custom with feesko() available. That would at
> least
> ensure hard failures if these assumptions change. That problably
> wouldn't
> be terribly invasive. I'm curious what you think.
That sounds fine, I'd say do that if it feels reasonable, and if the
extra callbacks get too messy, we can just document the assumptions
instead.
>
> Hm. One thing we could do is to send the TocEntry to the callback
> and
> verify that matches the one we were expecting to see next (as set by
> a
> previous call). Does that sound like a strong enough check?
Again, I'd just be practical here and do the check if it feels natural,
and if not, improve the comments so that someone modifying the code
would know where to look.
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Statistics Import and Export
@ 2025-04-02 16:42 Andres Freund <[email protected]>
parent: Jeff Davis <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Andres Freund @ 2025-04-02 16:42 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Corey Huinker <[email protected]>; Robert Treat <[email protected]>; Robert Haas <[email protected]>; Tom Lane <[email protected]>; Michael Paquier <[email protected]>; jian he <[email protected]>; Bruce Momjian <[email protected]>; Matthias van de Meent <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Ashutosh Bapat <[email protected]>; Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]
Hi,
https://commitfest.postgresql.org/patch/4538/ is still in "needs review", even
though the feature really has been committed. Is that intention, e.g. to
track pending changes that we're planning to make?
Greetings,
Andres
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Statistics Import and Export
@ 2025-04-03 02:26 Nathan Bossart <[email protected]>
parent: Jeff Davis <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Nathan Bossart @ 2025-04-03 02:26 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: Corey Huinker <[email protected]>; Robert Treat <[email protected]>; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Tom Lane <[email protected]>; Michael Paquier <[email protected]>; jian he <[email protected]>; Bruce Momjian <[email protected]>; Matthias van de Meent <[email protected]>; Magnus Hagander <[email protected]>; Stephen Frost <[email protected]>; Ashutosh Bapat <[email protected]>; Peter Smith <[email protected]>; PostgreSQL Hackers <[email protected]>; [email protected]
On Tue, Apr 01, 2025 at 10:44:19PM -0700, Jeff Davis wrote:
> On Tue, 2025-04-01 at 22:21 -0500, Nathan Bossart wrote:
>> We might be able to improve this by inventing a new callback that fails for
>> all formats except for custom with feesko() available. That would at least
>> ensure hard failures if these assumptions change. That problably wouldn't
>> be terribly invasive. I'm curious what you think.
>
> That sounds fine, I'd say do that if it feels reasonable, and if the
> extra callbacks get too messy, we can just document the assumptions
> instead.
I did write a version with callbacks, but it felt a bit silly because it is
very obviously intended for this one case. So, I removed them in the
attached patch set.
>> Hm. One thing we could do is to send the TocEntry to the callback and
>> verify that matches the one we were expecting to see next (as set by a
>> previous call). Does that sound like a strong enough check?
>
> Again, I'd just be practical here and do the check if it feels natural,
> and if not, improve the comments so that someone modifying the code
> would know where to look.
Okay, here is an updated patch set. I did add some verification code,
which ended up being a really good idea because it revealed a couple of
cases we weren't handling:
* Besides custom format calling WriteToc() twice to update the data
offsets, tar format calls WriteToc() followed by RestoreArchive() to
write restore.sql. I couldn't think of a great way to avoid executing
the queries twice in this case, so I settled on allowing it for only that
mode. While we don't expect the second set of queries to result in
different stats definitions, even if it did, the worst case is that the
content of restore.sql (which isn't used by pg_restore) would be
different. I noticed some past discussion that seems to suggest that
this format might be a candidate for deprecation [0], so I'm not sure
it's worth doing anything fancier.
* Our batching code assumes that stats entries are dumped in TOC order,
which unfortunately wasn't true for formats that use RestoreArchive() for
dumping. This is because RestoreArchive() does multiple passes through
the TOC and selectively dumps certain entries each time. This is
particularly troublesome for index stats and a subset of matview stats;
both are in SECTION_POST_DATA, but matview stats that depend on matview
data are dumped in RESTORE_PASS_POST_ACL, while all other stats data is
dumped in RESTORE_PASS_MAIN. To deal with this, I propose moving all
stats entries in SECTION_POST_DATA to RESTORE_PASS_POST_ACL, which
ensures that we always dump stats in TOC order. One convenient side
effect of this change is that we can revert a decent chunk of commit
a0a4601765. It might be possible to do better via smarter lookahead code
or a more sophisticated cache, but it's a bit late in the game for that.
[0] https://postgr.es/m/20180727015306.fzlo4inv5i3zqr2c%40alap3.anarazel.de
--
nathan
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2025-04-03 02:26 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 18:53 [PATCH 1/2] Added offset with block number in vacuum errcontext Mahendra Singh Thalor <[email protected]>
2025-04-01 22:05 Re: Statistics Import and Export Jeff Davis <[email protected]>
2025-04-02 03:21 ` Re: Statistics Import and Export Nathan Bossart <[email protected]>
2025-04-02 05:44 ` Re: Statistics Import and Export Jeff Davis <[email protected]>
2025-04-02 16:42 ` Re: Statistics Import and Export Andres Freund <[email protected]>
2025-04-03 02:26 ` Re: Statistics Import and Export Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox