($INBOX_DIR/description missing)help / color / mirror / Atom feed
[PATCH 3/4] GiST-Optimal-WAL-Usage 13+ messages / 9 participants [nested] [flat]
* [PATCH 3/4] GiST-Optimal-WAL-Usage @ 2019-04-01 04:07 Andrey V. Lepikhov <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Andrey V. Lepikhov @ 2019-04-01 04:07 UTC (permalink / raw) --- src/backend/access/gist/gist.c | 46 ++++++++++++++++++-------- src/backend/access/gist/gistbuild.c | 32 ++++++++++-------- src/backend/access/gist/gistutil.c | 2 +- src/backend/access/gist/gistxlog.c | 22 ------------ src/backend/access/rmgrdesc/gistdesc.c | 5 --- src/include/access/gist_private.h | 7 ++-- src/include/access/gistxlog.h | 1 - 7 files changed, 56 insertions(+), 59 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 2fddb23496..0e2b6c3014 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -172,7 +172,7 @@ gistinsert(Relation r, Datum *values, bool *isnull, values, isnull, true /* size is currently bogus */ ); itup->t_tid = *ht_ctid; - gistdoinsert(r, itup, 0, giststate, heapRel); + gistdoinsert(r, itup, 0, giststate, heapRel, false); /* cleanup */ MemoryContextSwitchTo(oldCxt); @@ -219,7 +219,8 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, Buffer leftchildbuf, List **splitinfo, bool markfollowright, - Relation heapRel) + Relation heapRel, + bool is_build) { BlockNumber blkno = BufferGetBlockNumber(buffer); Page page = BufferGetPage(buffer); @@ -458,7 +459,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, * insertion for that. NB: The number of pages and data segments * specified here must match the calculations in gistXLogSplit()! */ - if (RelationNeedsWAL(rel)) + if (RelationNeedsWAL(rel) && !is_build) XLogEnsureRecordSpace(npage, 1 + npage * 2); START_CRIT_SECTION(); @@ -479,18 +480,20 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, PageRestoreTempPage(dist->page, BufferGetPage(dist->buffer)); dist->page = BufferGetPage(dist->buffer); - /* Write the WAL record */ - if (RelationNeedsWAL(rel)) + /* + * Write the WAL record. + * Do not write XLog entry if the insertion is caused by + * index build process. + */ + if (RelationNeedsWAL(rel) && !is_build) recptr = gistXLogSplit(is_leaf, - dist, oldrlink, oldnsn, leftchildbuf, - markfollowright); + dist, oldrlink, oldnsn, leftchildbuf, + markfollowright); else recptr = gistGetFakeLSN(rel); for (ptr = dist; ptr; ptr = ptr->next) - { PageSetLSN(ptr->page, recptr); - } /* * Return the new child buffers to the caller. @@ -544,7 +547,8 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, if (BufferIsValid(leftchildbuf)) MarkBufferDirty(leftchildbuf); - if (RelationNeedsWAL(rel)) + + if (RelationNeedsWAL(rel) && !is_build) { OffsetNumber ndeloffs = 0, deloffs[1]; @@ -567,6 +571,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, PageSetLSN(page, recptr); } + if (newblkno) *newblkno = blkno; } @@ -583,17 +588,28 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, * the full page image. There's a chicken-and-egg problem: if we updated * the child pages first, we wouldn't know the recptr of the WAL record * we're about to write. + * + * We use fakeLSNs for inserions caused by index build. And when it is + * finished, we write generic_xlog entry for each index page and update + * all LSNs. In order to keep NSNs less then LSNs after this update, we + * set NSN to InvalidXLogRecPtr, which is the smallest possible NSN. */ + if (BufferIsValid(leftchildbuf)) { Page leftpg = BufferGetPage(leftchildbuf); + XLogRecPtr fakerecptr = InvalidXLogRecPtr; - GistPageSetNSN(leftpg, recptr); - GistClearFollowRight(leftpg); + if (!is_build) + GistPageSetNSN(leftpg, recptr); + else + GistPageSetNSN(leftpg, fakerecptr); + GistClearFollowRight(leftpg); PageSetLSN(leftpg, recptr); } + END_CRIT_SECTION(); return is_split; @@ -606,7 +622,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, */ void gistdoinsert(Relation r, IndexTuple itup, Size freespace, - GISTSTATE *giststate, Relation heapRel) + GISTSTATE *giststate, Relation heapRel, bool is_build) { ItemId iid; IndexTuple idxtuple; @@ -619,6 +635,7 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, state.freespace = freespace; state.r = r; state.heapRel = heapRel; + state.is_build = is_build; /* Start from the root */ firststack.blkno = GIST_ROOT_BLKNO; @@ -1251,7 +1268,8 @@ gistinserttuples(GISTInsertState *state, GISTInsertStack *stack, leftchild, &splitinfo, true, - state->heapRel); + state->heapRel, + state->is_build); /* * Before recursing up in case the page was split, release locks on the diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c index 3652fde5bb..8d0d285cab 100644 --- a/src/backend/access/gist/gistbuild.c +++ b/src/backend/access/gist/gistbuild.c @@ -17,6 +17,7 @@ #include <math.h> #include "access/genam.h" +#include "access/generic_xlog.h" #include "access/gist_private.h" #include "access/gistxlog.h" #include "access/tableam.h" @@ -181,18 +182,12 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo) MarkBufferDirty(buffer); - if (RelationNeedsWAL(index)) - { - XLogRecPtr recptr; - - XLogBeginInsert(); - XLogRegisterBuffer(0, buffer, REGBUF_WILL_INIT); - - recptr = XLogInsert(RM_GIST_ID, XLOG_GIST_CREATE_INDEX); - PageSetLSN(page, recptr); - } - else - PageSetLSN(page, gistGetFakeLSN(heap)); + /* + * Do not write index pages to WAL unitl index build is finished. + * But we still need increasing LSNs on each page, so use FakeLSN, + * even for relations which eventually need WAL. + */ + PageSetLSN(page, gistGetFakeLSN(heap)); UnlockReleaseBuffer(buffer); @@ -226,6 +221,15 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo) freeGISTstate(buildstate.giststate); + /* + * Create generic wal records for all pages of relation, if necessary. + * It seems reasonable not to generate WAL, if we recieved interrupt + * signal. + */ + CHECK_FOR_INTERRUPTS(); + if (RelationNeedsWAL(index)) + generic_log_relation(index); + /* * Return statistics */ @@ -488,7 +492,7 @@ gistBuildCallback(Relation index, * locked, we call gistdoinsert directly. */ gistdoinsert(index, itup, buildstate->freespace, - buildstate->giststate, buildstate->heaprel); + buildstate->giststate, buildstate->heaprel, true); } /* Update tuple count and total size. */ @@ -695,7 +699,7 @@ gistbufferinginserttuples(GISTBuildState *buildstate, Buffer buffer, int level, InvalidBuffer, &splitinfo, false, - buildstate->heaprel); + buildstate->heaprel, true); /* * If this is a root split, update the root path item kept in memory. This diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c index 2163cc482d..af278e5ded 100644 --- a/src/backend/access/gist/gistutil.c +++ b/src/backend/access/gist/gistutil.c @@ -1004,6 +1004,7 @@ gistproperty(Oid index_oid, int attno, * Temporary and unlogged GiST indexes are not WAL-logged, but we need LSNs * to detect concurrent page splits anyway. This function provides a fake * sequence of LSNs for that purpose. + * Persistent relations are also not WAL-logged while we build index. */ XLogRecPtr gistGetFakeLSN(Relation rel) @@ -1024,7 +1025,6 @@ gistGetFakeLSN(Relation rel) * Unlogged relations are accessible from other backends, and survive * (clean) restarts. GetFakeLSNForUnloggedRel() handles that for us. */ - Assert(rel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED); return GetFakeLSNForUnloggedRel(); } } diff --git a/src/backend/access/gist/gistxlog.c b/src/backend/access/gist/gistxlog.c index cb80ab00cd..4fb1855e89 100644 --- a/src/backend/access/gist/gistxlog.c +++ b/src/backend/access/gist/gistxlog.c @@ -490,25 +490,6 @@ gistRedoPageSplitRecord(XLogReaderState *record) UnlockReleaseBuffer(firstbuffer); } -static void -gistRedoCreateIndex(XLogReaderState *record) -{ - XLogRecPtr lsn = record->EndRecPtr; - Buffer buffer; - Page page; - - buffer = XLogInitBufferForRedo(record, 0); - Assert(BufferGetBlockNumber(buffer) == GIST_ROOT_BLKNO); - page = (Page) BufferGetPage(buffer); - - GISTInitBuffer(buffer, F_LEAF); - - PageSetLSN(page, lsn); - - MarkBufferDirty(buffer); - UnlockReleaseBuffer(buffer); -} - /* redo page deletion */ static void gistRedoPageDelete(XLogReaderState *record) @@ -594,9 +575,6 @@ gist_redo(XLogReaderState *record) case XLOG_GIST_PAGE_SPLIT: gistRedoPageSplitRecord(record); break; - case XLOG_GIST_CREATE_INDEX: - gistRedoCreateIndex(record); - break; case XLOG_GIST_PAGE_DELETE: gistRedoPageDelete(record); break; diff --git a/src/backend/access/rmgrdesc/gistdesc.c b/src/backend/access/rmgrdesc/gistdesc.c index 3ff4f83d38..eb308c72d6 100644 --- a/src/backend/access/rmgrdesc/gistdesc.c +++ b/src/backend/access/rmgrdesc/gistdesc.c @@ -71,8 +71,6 @@ gist_desc(StringInfo buf, XLogReaderState *record) case XLOG_GIST_PAGE_SPLIT: out_gistxlogPageSplit(buf, (gistxlogPageSplit *) rec); break; - case XLOG_GIST_CREATE_INDEX: - break; case XLOG_GIST_PAGE_DELETE: out_gistxlogPageDelete(buf, (gistxlogPageDelete *) rec); break; @@ -98,9 +96,6 @@ gist_identify(uint8 info) case XLOG_GIST_PAGE_SPLIT: id = "PAGE_SPLIT"; break; - case XLOG_GIST_CREATE_INDEX: - id = "CREATE_INDEX"; - break; case XLOG_GIST_PAGE_DELETE: id = "PAGE_DELETE"; break; diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h index 02dc285a78..78e2e3fb31 100644 --- a/src/include/access/gist_private.h +++ b/src/include/access/gist_private.h @@ -244,6 +244,7 @@ typedef struct Relation r; Relation heapRel; Size freespace; /* free space to be left */ + bool is_build; GISTInsertStack *stack; } GISTInsertState; @@ -393,7 +394,8 @@ extern void gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *GISTstate, - Relation heapRel); + Relation heapRel, + bool is_build); /* A List of these is returned from gistplacetopage() in *splitinfo */ typedef struct @@ -409,7 +411,8 @@ extern bool gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, Buffer leftchildbuf, List **splitinfo, bool markleftchild, - Relation heapRel); + Relation heapRel, + bool is_build); extern SplitedPageLayout *gistSplit(Relation r, Page page, IndexTuple *itup, int len, GISTSTATE *giststate); diff --git a/src/include/access/gistxlog.h b/src/include/access/gistxlog.h index 2f87b67a53..80931497ca 100644 --- a/src/include/access/gistxlog.h +++ b/src/include/access/gistxlog.h @@ -23,7 +23,6 @@ * FSM */ #define XLOG_GIST_PAGE_SPLIT 0x30 /* #define XLOG_GIST_INSERT_COMPLETE 0x40 */ /* not used anymore */ -#define XLOG_GIST_CREATE_INDEX 0x50 #define XLOG_GIST_PAGE_DELETE 0x60 /* -- 2.17.1 --------------5B85550EC82B346D9FBFC34E Content-Type: text/x-patch; name="v2_0004-SP-GiST-Optimal-WAL-Usage.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="v2_0004-SP-GiST-Optimal-WAL-Usage.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH 3/4] GiST-Optimal-WAL-Usage @ 2019-04-02 04:43 Andrey V. Lepikhov <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Andrey V. Lepikhov @ 2019-04-02 04:43 UTC (permalink / raw) --- src/backend/access/gist/gist.c | 46 ++++++++++++++++++-------- src/backend/access/gist/gistbuild.c | 32 ++++++++++-------- src/backend/access/gist/gistutil.c | 2 +- src/backend/access/gist/gistxlog.c | 22 ------------ src/backend/access/rmgrdesc/gistdesc.c | 5 --- src/include/access/gist_private.h | 7 ++-- src/include/access/gistxlog.h | 1 - 7 files changed, 56 insertions(+), 59 deletions(-) diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 2fddb23496..0e2b6c3014 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -172,7 +172,7 @@ gistinsert(Relation r, Datum *values, bool *isnull, values, isnull, true /* size is currently bogus */ ); itup->t_tid = *ht_ctid; - gistdoinsert(r, itup, 0, giststate, heapRel); + gistdoinsert(r, itup, 0, giststate, heapRel, false); /* cleanup */ MemoryContextSwitchTo(oldCxt); @@ -219,7 +219,8 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, Buffer leftchildbuf, List **splitinfo, bool markfollowright, - Relation heapRel) + Relation heapRel, + bool is_build) { BlockNumber blkno = BufferGetBlockNumber(buffer); Page page = BufferGetPage(buffer); @@ -458,7 +459,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, * insertion for that. NB: The number of pages and data segments * specified here must match the calculations in gistXLogSplit()! */ - if (RelationNeedsWAL(rel)) + if (RelationNeedsWAL(rel) && !is_build) XLogEnsureRecordSpace(npage, 1 + npage * 2); START_CRIT_SECTION(); @@ -479,18 +480,20 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, PageRestoreTempPage(dist->page, BufferGetPage(dist->buffer)); dist->page = BufferGetPage(dist->buffer); - /* Write the WAL record */ - if (RelationNeedsWAL(rel)) + /* + * Write the WAL record. + * Do not write XLog entry if the insertion is caused by + * index build process. + */ + if (RelationNeedsWAL(rel) && !is_build) recptr = gistXLogSplit(is_leaf, - dist, oldrlink, oldnsn, leftchildbuf, - markfollowright); + dist, oldrlink, oldnsn, leftchildbuf, + markfollowright); else recptr = gistGetFakeLSN(rel); for (ptr = dist; ptr; ptr = ptr->next) - { PageSetLSN(ptr->page, recptr); - } /* * Return the new child buffers to the caller. @@ -544,7 +547,8 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, if (BufferIsValid(leftchildbuf)) MarkBufferDirty(leftchildbuf); - if (RelationNeedsWAL(rel)) + + if (RelationNeedsWAL(rel) && !is_build) { OffsetNumber ndeloffs = 0, deloffs[1]; @@ -567,6 +571,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, PageSetLSN(page, recptr); } + if (newblkno) *newblkno = blkno; } @@ -583,17 +588,28 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, * the full page image. There's a chicken-and-egg problem: if we updated * the child pages first, we wouldn't know the recptr of the WAL record * we're about to write. + * + * We use fakeLSNs for inserions caused by index build. And when it is + * finished, we write generic_xlog entry for each index page and update + * all LSNs. In order to keep NSNs less then LSNs after this update, we + * set NSN to InvalidXLogRecPtr, which is the smallest possible NSN. */ + if (BufferIsValid(leftchildbuf)) { Page leftpg = BufferGetPage(leftchildbuf); + XLogRecPtr fakerecptr = InvalidXLogRecPtr; - GistPageSetNSN(leftpg, recptr); - GistClearFollowRight(leftpg); + if (!is_build) + GistPageSetNSN(leftpg, recptr); + else + GistPageSetNSN(leftpg, fakerecptr); + GistClearFollowRight(leftpg); PageSetLSN(leftpg, recptr); } + END_CRIT_SECTION(); return is_split; @@ -606,7 +622,7 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, */ void gistdoinsert(Relation r, IndexTuple itup, Size freespace, - GISTSTATE *giststate, Relation heapRel) + GISTSTATE *giststate, Relation heapRel, bool is_build) { ItemId iid; IndexTuple idxtuple; @@ -619,6 +635,7 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, state.freespace = freespace; state.r = r; state.heapRel = heapRel; + state.is_build = is_build; /* Start from the root */ firststack.blkno = GIST_ROOT_BLKNO; @@ -1251,7 +1268,8 @@ gistinserttuples(GISTInsertState *state, GISTInsertStack *stack, leftchild, &splitinfo, true, - state->heapRel); + state->heapRel, + state->is_build); /* * Before recursing up in case the page was split, release locks on the diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c index 3652fde5bb..aa05c0a8ee 100644 --- a/src/backend/access/gist/gistbuild.c +++ b/src/backend/access/gist/gistbuild.c @@ -17,6 +17,7 @@ #include <math.h> #include "access/genam.h" +#include "access/generic_xlog.h" #include "access/gist_private.h" #include "access/gistxlog.h" #include "access/tableam.h" @@ -181,18 +182,12 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo) MarkBufferDirty(buffer); - if (RelationNeedsWAL(index)) - { - XLogRecPtr recptr; - - XLogBeginInsert(); - XLogRegisterBuffer(0, buffer, REGBUF_WILL_INIT); - - recptr = XLogInsert(RM_GIST_ID, XLOG_GIST_CREATE_INDEX); - PageSetLSN(page, recptr); - } - else - PageSetLSN(page, gistGetFakeLSN(heap)); + /* + * Do not write index pages to WAL unitl index build is finished. + * But we still need increasing LSNs on each page, so use FakeLSN, + * even for relations which eventually need WAL. + */ + PageSetLSN(page, gistGetFakeLSN(heap)); UnlockReleaseBuffer(buffer); @@ -226,6 +221,15 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo) freeGISTstate(buildstate.giststate); + /* + * Create generic wal records for all pages of relation, if necessary. + * It seems reasonable not to generate WAL, if we recieved interrupt + * signal. + */ + CHECK_FOR_INTERRUPTS(); + if (RelationNeedsWAL(index)) + log_relation(index); + /* * Return statistics */ @@ -488,7 +492,7 @@ gistBuildCallback(Relation index, * locked, we call gistdoinsert directly. */ gistdoinsert(index, itup, buildstate->freespace, - buildstate->giststate, buildstate->heaprel); + buildstate->giststate, buildstate->heaprel, true); } /* Update tuple count and total size. */ @@ -695,7 +699,7 @@ gistbufferinginserttuples(GISTBuildState *buildstate, Buffer buffer, int level, InvalidBuffer, &splitinfo, false, - buildstate->heaprel); + buildstate->heaprel, true); /* * If this is a root split, update the root path item kept in memory. This diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c index 2163cc482d..af278e5ded 100644 --- a/src/backend/access/gist/gistutil.c +++ b/src/backend/access/gist/gistutil.c @@ -1004,6 +1004,7 @@ gistproperty(Oid index_oid, int attno, * Temporary and unlogged GiST indexes are not WAL-logged, but we need LSNs * to detect concurrent page splits anyway. This function provides a fake * sequence of LSNs for that purpose. + * Persistent relations are also not WAL-logged while we build index. */ XLogRecPtr gistGetFakeLSN(Relation rel) @@ -1024,7 +1025,6 @@ gistGetFakeLSN(Relation rel) * Unlogged relations are accessible from other backends, and survive * (clean) restarts. GetFakeLSNForUnloggedRel() handles that for us. */ - Assert(rel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED); return GetFakeLSNForUnloggedRel(); } } diff --git a/src/backend/access/gist/gistxlog.c b/src/backend/access/gist/gistxlog.c index cb80ab00cd..4fb1855e89 100644 --- a/src/backend/access/gist/gistxlog.c +++ b/src/backend/access/gist/gistxlog.c @@ -490,25 +490,6 @@ gistRedoPageSplitRecord(XLogReaderState *record) UnlockReleaseBuffer(firstbuffer); } -static void -gistRedoCreateIndex(XLogReaderState *record) -{ - XLogRecPtr lsn = record->EndRecPtr; - Buffer buffer; - Page page; - - buffer = XLogInitBufferForRedo(record, 0); - Assert(BufferGetBlockNumber(buffer) == GIST_ROOT_BLKNO); - page = (Page) BufferGetPage(buffer); - - GISTInitBuffer(buffer, F_LEAF); - - PageSetLSN(page, lsn); - - MarkBufferDirty(buffer); - UnlockReleaseBuffer(buffer); -} - /* redo page deletion */ static void gistRedoPageDelete(XLogReaderState *record) @@ -594,9 +575,6 @@ gist_redo(XLogReaderState *record) case XLOG_GIST_PAGE_SPLIT: gistRedoPageSplitRecord(record); break; - case XLOG_GIST_CREATE_INDEX: - gistRedoCreateIndex(record); - break; case XLOG_GIST_PAGE_DELETE: gistRedoPageDelete(record); break; diff --git a/src/backend/access/rmgrdesc/gistdesc.c b/src/backend/access/rmgrdesc/gistdesc.c index 3ff4f83d38..eb308c72d6 100644 --- a/src/backend/access/rmgrdesc/gistdesc.c +++ b/src/backend/access/rmgrdesc/gistdesc.c @@ -71,8 +71,6 @@ gist_desc(StringInfo buf, XLogReaderState *record) case XLOG_GIST_PAGE_SPLIT: out_gistxlogPageSplit(buf, (gistxlogPageSplit *) rec); break; - case XLOG_GIST_CREATE_INDEX: - break; case XLOG_GIST_PAGE_DELETE: out_gistxlogPageDelete(buf, (gistxlogPageDelete *) rec); break; @@ -98,9 +96,6 @@ gist_identify(uint8 info) case XLOG_GIST_PAGE_SPLIT: id = "PAGE_SPLIT"; break; - case XLOG_GIST_CREATE_INDEX: - id = "CREATE_INDEX"; - break; case XLOG_GIST_PAGE_DELETE: id = "PAGE_DELETE"; break; diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h index 02dc285a78..78e2e3fb31 100644 --- a/src/include/access/gist_private.h +++ b/src/include/access/gist_private.h @@ -244,6 +244,7 @@ typedef struct Relation r; Relation heapRel; Size freespace; /* free space to be left */ + bool is_build; GISTInsertStack *stack; } GISTInsertState; @@ -393,7 +394,8 @@ extern void gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *GISTstate, - Relation heapRel); + Relation heapRel, + bool is_build); /* A List of these is returned from gistplacetopage() in *splitinfo */ typedef struct @@ -409,7 +411,8 @@ extern bool gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, Buffer leftchildbuf, List **splitinfo, bool markleftchild, - Relation heapRel); + Relation heapRel, + bool is_build); extern SplitedPageLayout *gistSplit(Relation r, Page page, IndexTuple *itup, int len, GISTSTATE *giststate); diff --git a/src/include/access/gistxlog.h b/src/include/access/gistxlog.h index 2f87b67a53..80931497ca 100644 --- a/src/include/access/gistxlog.h +++ b/src/include/access/gistxlog.h @@ -23,7 +23,6 @@ * FSM */ #define XLOG_GIST_PAGE_SPLIT 0x30 /* #define XLOG_GIST_INSERT_COMPLETE 0x40 */ /* not used anymore */ -#define XLOG_GIST_CREATE_INDEX 0x50 #define XLOG_GIST_PAGE_DELETE 0x60 /* -- 2.17.1 --------------58E4AA393B5F48ED79D1D18E Content-Type: text/x-patch; name="v3_0004-SP-GiST-Optimal-WAL-Usage.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="v3_0004-SP-GiST-Optimal-WAL-Usage.patch" ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v4 1/1] add to_binary() and to_oct() @ 2023-07-25 23:09 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Nathan Bossart @ 2023-07-25 23:09 UTC (permalink / raw) --- doc/src/sgml/func.sgml | 42 +++++++++++++ src/backend/utils/adt/varlena.c | 86 +++++++++++++++++++-------- src/include/catalog/pg_proc.dat | 12 ++++ src/test/regress/expected/strings.out | 26 +++++++- src/test/regress/sql/strings.sql | 9 ++- 5 files changed, 148 insertions(+), 27 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index b94827674c..79334ea145 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -3737,6 +3737,48 @@ repeat('Pg', 4) <returnvalue>PgPgPgPg</returnvalue> </para></entry> </row> + <row> + <entry role="func_table_entry"><para role="func_signature"> + <indexterm> + <primary>to_binary</primary> + </indexterm> + <function>to_binary</function> ( <type>integer</type> ) + <returnvalue>text</returnvalue> + </para> + <para role="func_signature"> + <function>to_binary</function> ( <type>bigint</type> ) + <returnvalue>text</returnvalue> + </para> + <para> + Converts the number to its equivalent binary representation. + </para> + <para> + <literal>to_binary(2147483647)</literal> + <returnvalue>1111111111111111111111111111111</returnvalue> + </para></entry> + </row> + + <row> + <entry role="func_table_entry"><para role="func_signature"> + <indexterm> + <primary>to_oct</primary> + </indexterm> + <function>to_oct</function> ( <type>integer</type> ) + <returnvalue>text</returnvalue> + </para> + <para role="func_signature"> + <function>to_oct</function> ( <type>bigint</type> ) + <returnvalue>text</returnvalue> + </para> + <para> + Converts the number to its equivalent octal representation. + </para> + <para> + <literal>to_oct(2147483647)</literal> + <returnvalue>17777777777</returnvalue> + </para></entry> + </row> + <row> <entry role="func_table_entry"><para role="func_signature"> <indexterm> diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index b1ec5c32ce..b8903f338f 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -165,6 +165,7 @@ static void text_format_string_conversion(StringInfo buf, char conversion, int flags, int width); static void text_format_append_string(StringInfo buf, const char *str, int flags, int width); +static char *convert_to_base(uint64 value, int base); /***************************************************************************** @@ -4919,53 +4920,90 @@ array_to_text_internal(FunctionCallInfo fcinfo, ArrayType *v, return result; } -#define HEXBASE 16 /* - * Convert an int32 to a string containing a base 16 (hex) representation of - * the number. + * Convert an integer to a string containing a base-2 (binary) representation + * of the number. */ Datum -to_hex32(PG_FUNCTION_ARGS) +to_binary32(PG_FUNCTION_ARGS) { - uint32 value = (uint32) PG_GETARG_INT32(0); - char *ptr; - const char *digits = "0123456789abcdef"; - char buf[32]; /* bigger than needed, but reasonable */ + uint64 value = (uint64) PG_GETARG_INT32(0); + char *result = convert_to_base(value, 2); - ptr = buf + sizeof(buf) - 1; - *ptr = '\0'; + PG_RETURN_TEXT_P(cstring_to_text(result)); +} +Datum +to_binary64(PG_FUNCTION_ARGS) +{ + uint64 value = (uint64) PG_GETARG_INT64(0); + char *result = convert_to_base(value, 2); - do - { - *--ptr = digits[value % HEXBASE]; - value /= HEXBASE; - } while (ptr > buf && value); + PG_RETURN_TEXT_P(cstring_to_text(result)); +} - PG_RETURN_TEXT_P(cstring_to_text(ptr)); +/* + * Convert an integer to a string containing a base-8 (oct) representation of + * the number. + */ +Datum +to_oct32(PG_FUNCTION_ARGS) +{ + uint64 value = (uint64) PG_GETARG_INT32(0); + char *result = convert_to_base(value, 8); + + PG_RETURN_TEXT_P(cstring_to_text(result)); +} +Datum +to_oct64(PG_FUNCTION_ARGS) +{ + uint64 value = (uint64) PG_GETARG_INT64(0); + char *result = convert_to_base(value, 8); + + PG_RETURN_TEXT_P(cstring_to_text(result)); } /* - * Convert an int64 to a string containing a base 16 (hex) representation of + * Convert an integer to a string containing a base-16 (hex) representation of * the number. */ Datum +to_hex32(PG_FUNCTION_ARGS) +{ + uint64 value = (uint64) PG_GETARG_INT32(0); + char *result = convert_to_base(value, 16); + + PG_RETURN_TEXT_P(cstring_to_text(result)); +} +Datum to_hex64(PG_FUNCTION_ARGS) { uint64 value = (uint64) PG_GETARG_INT64(0); - char *ptr; + char *result = convert_to_base(value, 16); + + PG_RETURN_TEXT_P(cstring_to_text(result)); +} + +/* + * Workhorse for to_binary, to_oct, and to_hex. Note that base must be either + * 2, 8, or 16. + */ +static char * +convert_to_base(uint64 value, int base) +{ const char *digits = "0123456789abcdef"; - char buf[32]; /* bigger than needed, but reasonable */ + char *buf = palloc(sizeof(uint64) * BITS_PER_BYTE + 1); + char *ptr = buf + (sizeof(uint64) * BITS_PER_BYTE); - ptr = buf + sizeof(buf) - 1; - *ptr = '\0'; + Assert(base == 2 || base == 8 || base == 16); + *ptr = '\0'; do { - *--ptr = digits[value % HEXBASE]; - value /= HEXBASE; + *--ptr = digits[value % base]; + value /= base; } while (ptr > buf && value); - PG_RETURN_TEXT_P(cstring_to_text(ptr)); + return ptr; } /* diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 6996073989..eca2b420e0 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -3707,6 +3707,18 @@ { oid => '2768', descr => 'split string by pattern', proname => 'regexp_split_to_array', prorettype => '_text', proargtypes => 'text text text', prosrc => 'regexp_split_to_array' }, +{ oid => '5101', descr => 'convert int4 number to binary', + proname => 'to_binary', prorettype => 'text', proargtypes => 'int4', + prosrc => 'to_binary32' }, +{ oid => '5102', descr => 'convert int8 number to binary', + proname => 'to_binary', prorettype => 'text', proargtypes => 'int8', + prosrc => 'to_binary64' }, +{ oid => '5103', descr => 'convert int4 number to oct', + proname => 'to_oct', prorettype => 'text', proargtypes => 'int4', + prosrc => 'to_oct32' }, +{ oid => '5104', descr => 'convert int8 number to oct', + proname => 'to_oct', prorettype => 'text', proargtypes => 'int8', + prosrc => 'to_oct64' }, { oid => '2089', descr => 'convert int4 number to hex', proname => 'to_hex', prorettype => 'text', proargtypes => 'int4', prosrc => 'to_hex32' }, diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out index 62698569e1..28c81ea6e4 100644 --- a/src/test/regress/expected/strings.out +++ b/src/test/regress/expected/strings.out @@ -2129,8 +2129,32 @@ select split_part('@joeuser@mydatabase@','@',-2) AS "mydatabase"; (1 row) -- --- test to_hex +-- test to_binary, to_oct, and to_hex -- +select to_binary(256*256*256 - 1) AS "111111111111111111111111"; + 111111111111111111111111 +-------------------------- + 111111111111111111111111 +(1 row) + +select to_binary(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "11111111111111111111111111111111"; + 11111111111111111111111111111111 +---------------------------------- + 11111111111111111111111111111111 +(1 row) + +select to_oct(256*256*256 - 1) AS "77777777"; + 77777777 +---------- + 77777777 +(1 row) + +select to_oct(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "37777777777"; + 37777777777 +------------- + 37777777777 +(1 row) + select to_hex(256*256*256 - 1) AS "ffffff"; ffffff -------- diff --git a/src/test/regress/sql/strings.sql b/src/test/regress/sql/strings.sql index ca32f6bba5..f7e35bfb41 100644 --- a/src/test/regress/sql/strings.sql +++ b/src/test/regress/sql/strings.sql @@ -685,10 +685,15 @@ select split_part('joeuser@mydatabase','@',-3) AS "empty string"; select split_part('@joeuser@mydatabase@','@',-2) AS "mydatabase"; -- --- test to_hex +-- test to_binary, to_oct, and to_hex -- -select to_hex(256*256*256 - 1) AS "ffffff"; +select to_binary(256*256*256 - 1) AS "111111111111111111111111"; +select to_binary(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "11111111111111111111111111111111"; + +select to_oct(256*256*256 - 1) AS "77777777"; +select to_oct(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "37777777777"; +select to_hex(256*256*256 - 1) AS "ffffff"; select to_hex(256::bigint*256::bigint*256::bigint*256::bigint - 1) AS "ffffffff"; -- -- 2.25.1 --y0ulUmNC+osPPQO6-- ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables @ 2025-02-05 22:08 Melanie Plageman <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Melanie Plageman @ 2025-02-05 22:08 UTC (permalink / raw) To: Greg Sabino Mullane <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]> On Thu, Jan 16, 2025 at 5:50 PM Melanie Plageman <[email protected]> wrote: > > On Thu, Jan 16, 2025 at 4:43 PM Melanie Plageman > <[email protected]> wrote: > > > > On Fri, Oct 25, 2024 at 11:14 AM Melanie Plageman > > <[email protected]> wrote: > > > > > > I've done something similar to this in attached v2. > > > > This needed a rebase. See attached v4. > > Whoops -- docs didn't build. Attached v5. Outside of the positive performance impact of vacuuming pages before they go cold (detailed in my first email [1]), there is also a substantial positive effect with this patch for large tables with substantial cold regions: fewer anti-wraparound vacuums and more frequent normal/aggressive vacuums With the default vacuum settings, you often see an append-only table devolve to _only_ anti-wraparound vacuums after the first aggressive vacuum. I ran an insert-only workload for an hour (with 32 clients and synchronous commit off to maximize the amount of data inserted) with the default vacuum settings. On master, after the first aggressive vacuum, we do only anti-wraparound vacuums (and only two of these are triggered). With the patch, after the first aggressive vacuum, 10 more vacuums are triggered -- none of which are anti-wraparound vacuums. I attached a chart comparing the autovacuums triggered on master vs with the patch. Besides the performance benefit of spreading the freezing work over more normal vacuums (thereby disrupting foreground workloads less), anti-wraparound vacuums are not auto canceled by DDL -- making them more of a nuisance to users. [1] https://www.postgresql.org/message-id/CAAKRu_aj-P7YyBz_cPNwztz6ohP%2BvWis%3Diz3YcomkB3NpYA--w%40mail... Attachments: [image/png] autovacuums.png (69.3K, ../../CAAKRu_b=QZHNA1setZHBX+VjJRhwus9gM5cG5n10gaqYgF+QaA@mail.gmail.com/2-autovacuums.png) download | view image ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables @ 2025-02-06 01:54 wenhui qiu <[email protected]> parent: Melanie Plageman <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: wenhui qiu @ 2025-02-06 01:54 UTC (permalink / raw) To: Melanie Plageman <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]> Hi Melanie Plageman Thank you for working on this ,Actually, there were two patches aimed at optimizing vacuum-triggered processes, and one of them reached a consensus and has been committed:https://commitfest.postgresql.org/52/5046/ , https://commitfest.postgresql.org/51/5395/, Maybe referring to the already committed patch and setting a maximum value for vacuum_max_ins_threshold would be more acceptable. Thanks On Thu, Feb 6, 2025 at 6:08 AM Melanie Plageman <[email protected]> wrote: > On Thu, Jan 16, 2025 at 5:50 PM Melanie Plageman > <[email protected]> wrote: > > > > On Thu, Jan 16, 2025 at 4:43 PM Melanie Plageman > > <[email protected]> wrote: > > > > > > On Fri, Oct 25, 2024 at 11:14 AM Melanie Plageman > > > <[email protected]> wrote: > > > > > > > > I've done something similar to this in attached v2. > > > > > > This needed a rebase. See attached v4. > > > > Whoops -- docs didn't build. Attached v5. > > Outside of the positive performance impact of vacuuming pages before > they go cold (detailed in my first email [1]), there is also a > substantial positive effect with this patch for large tables with > substantial cold regions: fewer anti-wraparound vacuums and more > frequent normal/aggressive vacuums > > With the default vacuum settings, you often see an append-only table > devolve to _only_ anti-wraparound vacuums after the first aggressive > vacuum. I ran an insert-only workload for an hour (with 32 clients and > synchronous commit off to maximize the amount of data inserted) with > the default vacuum settings. On master, after the first aggressive > vacuum, we do only anti-wraparound vacuums (and only two of these are > triggered). With the patch, after the first aggressive vacuum, 10 more > vacuums are triggered -- none of which are anti-wraparound vacuums. > > I attached a chart comparing the autovacuums triggered on master vs > with the patch. > > Besides the performance benefit of spreading the freezing work over > more normal vacuums (thereby disrupting foreground workloads less), > anti-wraparound vacuums are not auto canceled by DDL -- making them > more of a nuisance to users. > > [1] > https://www.postgresql.org/message-id/CAAKRu_aj-P7YyBz_cPNwztz6ohP%2BvWis%3Diz3YcomkB3NpYA--w%40mail... > ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables @ 2025-02-06 15:42 Melanie Plageman <[email protected]> parent: wenhui qiu <[email protected]> 0 siblings, 2 replies; 13+ messages in thread From: Melanie Plageman @ 2025-02-06 15:42 UTC (permalink / raw) To: wenhui qiu <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]> Attached v6 is rebased over 306dc520b9dfd60 On Wed, Feb 5, 2025 at 8:54 PM wenhui qiu <[email protected]> wrote: > > Hi Melanie Plageman > Thank you for working on this ,Actually, there were two patches aimed at optimizing vacuum-triggered processes, and one of them reached a consensus and has been committed:https://commitfest.postgresql.org/52/5046/ , https://commitfest.postgresql.org/51/5395/, Maybe referring to the already committed patch and setting a maximum value for vacuum_max_ins_threshold would be more acceptable. We could add autovacuum_vacuum_insert_max_threshold, but with an insert-only workload, we can expect that the cold data is being frozen. By calculating the threshold based on unfrozen data, we are effectively capping the threshold for inserted data without adding another guc. If any of that data is being unfrozen via updates or deletes, then the autovacuum_vacuum_max_threshold would apply. Perhaps I'm missing a case where calculating the insert threshold on unfrozen data would not act as a cap, in which case I could get on board with a guc. - Melanie Attachments: [application/x-patch] v6-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch (5.4K, ../../CAAKRu_bRnvkAaCuim6KBi8Sti+uHApMSrO8ug-VBpGzUZ7T=HQ@mail.gmail.com/2-v6-0002-Trigger-more-frequent-autovacuums-with-relallfroz.patch) download | inline diff: From 6048afbedf1590c3669c9f952fa70b244839c987 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Thu, 16 Jan 2025 16:31:55 -0500 Subject: [PATCH v6 2/2] Trigger more frequent autovacuums with relallfrozen Calculate the insert threshold for triggering an autovacuum of a relation based on the number of unfrozen pages. By only considering the "active" (unfrozen) portion of the table when calculating how many tuples to add to the insert threshold, we can trigger more frequent vacuums of insert-heavy tables and increase the chances of vacuuming those pages when they still reside in shared buffers. Reviewed-by: Greg Sabino Mullane --- doc/src/sgml/config.sgml | 16 +++++----- src/backend/postmaster/autovacuum.c | 32 +++++++++++++++++-- src/backend/utils/misc/postgresql.conf.sample | 4 +-- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 38244409e3c..8cef54bf183 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -8652,14 +8652,14 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; </term> <listitem> <para> - Specifies a fraction of the table size to add to - <varname>autovacuum_vacuum_insert_threshold</varname> - when deciding whether to trigger a <command>VACUUM</command>. - The default is <literal>0.2</literal> (20% of table size). - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line; - but the setting can be overridden for individual tables by - changing table storage parameters. + Specifies a fraction of the active (unfrozen) table size to add to + <varname>autovacuum_vacuum_insert_threshold</varname> + when deciding whether to trigger a <command>VACUUM</command>. + The default is 0.2 (20% of active table size). + This parameter can only be set in the <filename>postgresql.conf</filename> + file or on the server command line; + but the setting can be overridden for individual tables by + changing table storage parameters. </para> </listitem> </varlistentry> diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 09ec9bb6990..250de2bb8ae 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2932,7 +2932,6 @@ relation_needs_vacanalyze(Oid relid, { bool force_vacuum; bool av_enabled; - float4 reltuples; /* pg_class.reltuples */ /* constants from reloptions or GUC variables */ int vac_base_thresh, @@ -3046,7 +3045,12 @@ relation_needs_vacanalyze(Oid relid, */ if (PointerIsValid(tabentry) && AutoVacuumingActive()) { - reltuples = classForm->reltuples; + float4 pcnt_unfrozen = 1; + float4 reltuples = classForm->reltuples; + int32 relpages = classForm->relpages; + int32 relallfrozen = classForm->relallfrozen; + int32 relallvisible = classForm->relallvisible; + vactuples = tabentry->dead_tuples; instuples = tabentry->ins_since_vacuum; anltuples = tabentry->mod_since_analyze; @@ -3055,11 +3059,33 @@ relation_needs_vacanalyze(Oid relid, if (reltuples < 0) reltuples = 0; + /* + * If the table has been vacuumed and we have reliable data for + * relallfrozen and relallvisible, calculate the unfrozen percentage + * of the table to modify insert scale factor. This helps us decide + * whether or not to vacuum an insert-heavy table based on the number + * of inserts to the "active" part of the table. + * + * If relallfrozen is -1, that means relallvisible was updated + * manually and we can't rely on relallfrozen. + */ + if (relpages > 0 && reltuples > 0 && relallfrozen > -1) + { + if (relallvisible > relpages) + relallvisible = relpages; + + if (relallfrozen > relallvisible) + relallfrozen = relallvisible; + + pcnt_unfrozen = 1 - ((float4) relallfrozen / relpages); + } + vacthresh = (float4) vac_base_thresh + vac_scale_factor * reltuples; if (vac_max_thresh >= 0 && vacthresh > (float4) vac_max_thresh) vacthresh = (float4) vac_max_thresh; - vacinsthresh = (float4) vac_ins_base_thresh + vac_ins_scale_factor * reltuples; + vacinsthresh = (float4) vac_ins_base_thresh + + vac_ins_scale_factor * reltuples * pcnt_unfrozen; anlthresh = (float4) anl_base_thresh + anl_scale_factor * reltuples; /* diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index c40b7a3121e..543e365adb6 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -667,8 +667,8 @@ autovacuum_worker_slots = 16 # autovacuum worker slots to allocate #autovacuum_analyze_threshold = 50 # min number of row updates before # analyze #autovacuum_vacuum_scale_factor = 0.2 # fraction of table size before vacuum -#autovacuum_vacuum_insert_scale_factor = 0.2 # fraction of inserts over table - # size before insert vacuum +#autovacuum_vacuum_insert_scale_factor = 0.2 # fraction of inserts over active + # table size before insert vacuum #autovacuum_analyze_scale_factor = 0.1 # fraction of table size before analyze #autovacuum_vacuum_max_threshold = 100000000 # max number of row updates # before vacuum; -1 disables max -- 2.34.1 [application/x-patch] v6-0001-Add-relallfrozen-to-pg_class.patch (16.0K, ../../CAAKRu_bRnvkAaCuim6KBi8Sti+uHApMSrO8ug-VBpGzUZ7T=HQ@mail.gmail.com/3-v6-0001-Add-relallfrozen-to-pg_class.patch) download | inline diff: From f32c398378285d76ded53e90f19231dc20f960bb Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Thu, 16 Jan 2025 16:19:57 -0500 Subject: [PATCH v6 1/2] Add relallfrozen to pg_class Add relallfrozen, an estimate of the number of pages marked all-frozen in the visibility map. pg_class already has relallvisible, an estimate of the number of pages in the relation marked all-visible in the visibility map. This is used primarily for planning. relallfrozen, however, is useful for estimating the outstanding number of all-visible but not all-frozen pages in the relation for the purposes of scheduling manual VACUUMs and tuning vacuum freeze parameters. In the future, this field could be used to trigger more frequent vacuums on insert-focused workloads with significant volume of frozen data. Reviewed-by: Greg Sabino Mullane --- doc/src/sgml/catalogs.sgml | 20 ++++++++++++++++++++ src/backend/access/heap/vacuumlazy.c | 17 +++++++++++++---- src/backend/catalog/heap.c | 2 ++ src/backend/catalog/index.c | 22 +++++++++++++++++----- src/backend/commands/analyze.c | 12 ++++++------ src/backend/commands/cluster.c | 5 +++++ src/backend/commands/vacuum.c | 15 ++++++++++++++- src/backend/statistics/relation_stats.c | 23 ++++++++++++++++++++--- src/backend/utils/cache/relcache.c | 2 ++ src/include/catalog/pg_class.h | 8 ++++++++ src/include/commands/vacuum.h | 1 + 11 files changed, 108 insertions(+), 19 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 088fb175cce..ba0ba0f55a3 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -2064,6 +2064,26 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>relallfrozen</structfield> <type>int4</type> + </para> + <para> + Number of pages that are marked all-frozen in the table's visibility + map. This is only an estimate used for triggering autovacuums. It is + updated by <link linkend="sql-vacuum"><command>VACUUM</command></link>, + <link linkend="sql-analyze"><command>ANALYZE</command></link>, + and a few DDL commands such as + <link linkend="sql-createindex"><command>CREATE INDEX</command></link>. + <structfield>relallfrozen</structfield> must be less than or equal to + <structfield>relallvisible</structfield> as an all-frozen page must be + all-visible. If <structfield>relallvisible</structfield> was updated + manually, <structfield>relallfrozen</structfield> will be -1 until the + next time they are updated. + </para></entry> + </row> + + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>reltoastrelid</structfield> <type>oid</type> diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 075af385cd1..d7048fdfb71 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -370,7 +370,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, minmulti_updated; BlockNumber orig_rel_pages, new_rel_pages, - new_rel_allvisible; + new_rel_allvisible, + new_rel_allfrozen; PGRUsage ru0; TimestampTz starttime = 0; PgStat_Counter startreadtime = 0, @@ -631,10 +632,17 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, * pg_class.relpages to */ new_rel_pages = vacrel->rel_pages; /* After possible rel truncation */ - visibilitymap_count(rel, &new_rel_allvisible, NULL); + visibilitymap_count(rel, &new_rel_allvisible, &new_rel_allfrozen); if (new_rel_allvisible > new_rel_pages) new_rel_allvisible = new_rel_pages; + /* + * An all-frozen block _must_ be all-visible. As such, clamp the count of + * all-frozen blocks to the count of all-visible blocks. + */ + if (new_rel_allfrozen > new_rel_allvisible) + new_rel_allfrozen = new_rel_allvisible; + /* * Now actually update rel's pg_class entry. * @@ -643,7 +651,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, * scan every page that isn't skipped using the visibility map. */ vac_update_relstats(rel, new_rel_pages, vacrel->new_live_tuples, - new_rel_allvisible, vacrel->nindexes > 0, + new_rel_allvisible, new_rel_allfrozen, + vacrel->nindexes > 0, vacrel->NewRelfrozenXid, vacrel->NewRelminMxid, &frozenxid_updated, &minmulti_updated, false); @@ -3251,7 +3260,7 @@ update_relstats_all_indexes(LVRelState *vacrel) vac_update_relstats(indrel, istat->num_pages, istat->num_index_tuples, - 0, + 0, 0, false, InvalidTransactionId, InvalidMultiXactId, diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 57ef466acce..5ab75d5c977 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -913,6 +913,7 @@ InsertPgClassTuple(Relation pg_class_desc, values[Anum_pg_class_relpages - 1] = Int32GetDatum(rd_rel->relpages); values[Anum_pg_class_reltuples - 1] = Float4GetDatum(rd_rel->reltuples); values[Anum_pg_class_relallvisible - 1] = Int32GetDatum(rd_rel->relallvisible); + values[Anum_pg_class_relallfrozen - 1] = Int32GetDatum(rd_rel->relallfrozen); values[Anum_pg_class_reltoastrelid - 1] = ObjectIdGetDatum(rd_rel->reltoastrelid); values[Anum_pg_class_relhasindex - 1] = BoolGetDatum(rd_rel->relhasindex); values[Anum_pg_class_relisshared - 1] = BoolGetDatum(rd_rel->relisshared); @@ -983,6 +984,7 @@ AddNewRelationTuple(Relation pg_class_desc, new_rel_reltup->relpages = 0; new_rel_reltup->reltuples = -1; new_rel_reltup->relallvisible = 0; + new_rel_reltup->relallfrozen = 0; /* Sequences always have a known size */ if (relkind == RELKIND_SEQUENCE) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 7377912b41e..ee72a1e5e41 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2792,8 +2792,8 @@ FormIndexDatum(IndexInfo *indexInfo, * hasindex: set relhasindex to this value * reltuples: if >= 0, set reltuples to this value; else no change * - * If reltuples >= 0, relpages and relallvisible are also updated (using - * RelationGetNumberOfBlocks() and visibilitymap_count()). + * If reltuples >= 0, relpages, relallvisible, and relallfrozen are also + * updated (using RelationGetNumberOfBlocks() and visibilitymap_count()). * * NOTE: an important side-effect of this operation is that an SI invalidation * message is sent out to all backends --- including me --- causing relcache @@ -2811,6 +2811,7 @@ index_update_stats(Relation rel, bool update_stats; BlockNumber relpages = 0; /* keep compiler quiet */ BlockNumber relallvisible = 0; + BlockNumber relallfrozen = 0; Oid relid = RelationGetRelid(rel); Relation pg_class; ScanKeyData key[1]; @@ -2850,7 +2851,13 @@ index_update_stats(Relation rel, relpages = RelationGetNumberOfBlocks(rel); if (rel->rd_rel->relkind != RELKIND_INDEX) - visibilitymap_count(rel, &relallvisible, NULL); + { + visibilitymap_count(rel, &relallvisible, &relallfrozen); + + /* An all-frozen block must be all-visible in the VM */ + if (relallfrozen > relallvisible) + relallfrozen = relallvisible; + } } /* @@ -2875,8 +2882,8 @@ index_update_stats(Relation rel, * transaction could still fail before committing. Setting relhasindex * true is safe even if there are no indexes (VACUUM will eventually fix * it). And of course the new relpages and reltuples counts are correct - * regardless. However, we don't want to change relpages (or - * relallvisible) if the caller isn't providing an updated reltuples + * regardless. However, we don't want to change relpages (or relallvisible + * and relallfrozen) if the caller isn't providing an updated reltuples * count, because that would bollix the reltuples/relpages ratio which is * what's really important. */ @@ -2923,6 +2930,11 @@ index_update_stats(Relation rel, rd_rel->relallvisible = (int32) relallvisible; dirty = true; } + if (rd_rel->relallfrozen != (int32) relallfrozen) + { + rd_rel->relallfrozen = (int32) relallfrozen; + dirty = true; + } } /* diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index f8da32e9aef..b9f17bf4212 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -630,12 +630,11 @@ do_analyze_rel(Relation onerel, VacuumParams *params, */ if (!inh) { - BlockNumber relallvisible; + BlockNumber relallvisible = 0; + BlockNumber relallfrozen = 0; if (RELKIND_HAS_STORAGE(onerel->rd_rel->relkind)) - visibilitymap_count(onerel, &relallvisible, NULL); - else - relallvisible = 0; + visibilitymap_count(onerel, &relallvisible, &relallfrozen); /* * Update pg_class for table relation. CCI first, in case acquirefunc @@ -646,6 +645,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params, relpages, totalrows, relallvisible, + relallfrozen, hasindex, InvalidTransactionId, InvalidMultiXactId, @@ -662,7 +662,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params, vac_update_relstats(Irel[ind], RelationGetNumberOfBlocks(Irel[ind]), totalindexrows, - 0, + 0, 0, false, InvalidTransactionId, InvalidMultiXactId, @@ -678,7 +678,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params, */ CommandCounterIncrement(); vac_update_relstats(onerel, -1, totalrows, - 0, hasindex, InvalidTransactionId, + 0, 0, hasindex, InvalidTransactionId, InvalidMultiXactId, NULL, NULL, in_outer_xact); diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 99193f5c886..54a08e4102e 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1226,6 +1226,7 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class, int32 swap_pages; float4 swap_tuples; int32 swap_allvisible; + int32 swap_allfrozen; swap_pages = relform1->relpages; relform1->relpages = relform2->relpages; @@ -1238,6 +1239,10 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class, swap_allvisible = relform1->relallvisible; relform1->relallvisible = relform2->relallvisible; relform2->relallvisible = swap_allvisible; + + swap_allfrozen = relform1->relallfrozen; + relform1->relallfrozen = relform2->relallfrozen; + relform2->relallfrozen = swap_allfrozen; } /* diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index e6745e6145c..376ce2e489a 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1410,6 +1410,7 @@ void vac_update_relstats(Relation relation, BlockNumber num_pages, double num_tuples, BlockNumber num_all_visible_pages, + BlockNumber num_all_frozen_pages, bool hasindex, TransactionId frozenxid, MultiXactId minmulti, bool *frozenxid_updated, bool *minmulti_updated, @@ -1441,8 +1442,15 @@ vac_update_relstats(Relation relation, relid); pgcform = (Form_pg_class) GETSTRUCT(ctup); - /* Apply statistical updates, if any, to copied tuple */ + /* + * An all-frozen block must be marked all-visible in the VM. While callers + * are likely to check this themselves, it is worth ensuring we don't put + * incorrect information in pg_class. + */ + if (num_all_frozen_pages > num_all_visible_pages) + num_all_frozen_pages = num_all_visible_pages; + /* Apply statistical updates, if any, to copied tuple */ dirty = false; if (pgcform->relpages != (int32) num_pages) { @@ -1459,6 +1467,11 @@ vac_update_relstats(Relation relation, pgcform->relallvisible = (int32) num_all_visible_pages; dirty = true; } + if (pgcform->relallfrozen != (int32) num_all_frozen_pages) + { + pgcform->relallfrozen = (int32) num_all_frozen_pages; + dirty = true; + } /* Apply DDL updates, but not inside an outer transaction (see above) */ diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c index 046661d7c3f..758a56f8a46 100644 --- a/src/backend/statistics/relation_stats.c +++ b/src/backend/statistics/relation_stats.c @@ -56,6 +56,10 @@ static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel, /* * Internal function for modifying statistics for a relation. + * + * Up to four pg_class columns may be updated even though only three relation + * statistics may be modified; relallfrozen is always set to -1 when + * relallvisible is updated manually. */ static bool relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace) @@ -167,6 +171,14 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace) if (update_relallvisible && pgcform->relallvisible != relallvisible) { pgcform->relallvisible = relallvisible; + + /* + * If we are modifying relallvisible manually, it is not clear + * what relallfrozen value would make sense. Therefore, set it to + * -1 ("unknown"). It will be updated the next time these fields + * are updated. + */ + pgcform->relallfrozen = -1; dirty = true; } @@ -182,9 +194,9 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace) TupleDesc tupdesc = RelationGetDescr(crel); HeapTuple ctup; Form_pg_class pgcform; - int replaces[3] = {0}; - Datum values[3] = {0}; - bool nulls[3] = {0}; + int replaces[4] = {0}; + Datum values[4] = {0}; + bool nulls[4] = {0}; int nreplaces = 0; ctup = SearchSysCache1(RELOID, ObjectIdGetDatum(reloid)); @@ -217,6 +229,11 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel, bool inplace) replaces[nreplaces] = Anum_pg_class_relallvisible; values[nreplaces] = Int32GetDatum(relallvisible); nreplaces++; + + /* See comment above in-place update of relallfrozen */ + replaces[nreplaces] = Anum_pg_class_relallfrozen; + values[nreplaces] = Int32GetDatum(-1); + nreplaces++; } if (nreplaces > 0) diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 43219a9629c..efd6bcb3860 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1925,6 +1925,7 @@ formrdesc(const char *relationName, Oid relationReltype, relation->rd_rel->relpages = 0; relation->rd_rel->reltuples = -1; relation->rd_rel->relallvisible = 0; + relation->rd_rel->relallfrozen = 0; relation->rd_rel->relkind = RELKIND_RELATION; relation->rd_rel->relnatts = (int16) natts; @@ -3882,6 +3883,7 @@ RelationSetNewRelfilenumber(Relation relation, char persistence) classform->relpages = 0; /* it's empty until further notice */ classform->reltuples = -1; classform->relallvisible = 0; + classform->relallfrozen = 0; } classform->relfrozenxid = freezeXid; classform->relminmxid = minmulti; diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h index f0d612ca487..fbf42b0e195 100644 --- a/src/include/catalog/pg_class.h +++ b/src/include/catalog/pg_class.h @@ -68,6 +68,14 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat /* # of all-visible blocks (not always up-to-date) */ int32 relallvisible BKI_DEFAULT(0); + /* + * # of all-frozen blocks (not always up-to-date) + * Starts as 0 (if it has never been updated). + * If relallvisible is manually updated, relallfrozen will be + * -1, meaning "unknown" + */ + int32 relallfrozen BKI_DEFAULT(0); + /* OID of toast table; 0 if none */ Oid reltoastrelid BKI_DEFAULT(0) BKI_LOOKUP_OPT(pg_class); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 12d0b61950d..b2a678df09e 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -329,6 +329,7 @@ extern void vac_update_relstats(Relation relation, BlockNumber num_pages, double num_tuples, BlockNumber num_all_visible_pages, + BlockNumber num_all_frozen_pages, bool hasindex, TransactionId frozenxid, MultiXactId minmulti, -- 2.34.1 ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables @ 2025-02-07 06:40 wenhui qiu <[email protected]> parent: Melanie Plageman <[email protected]> 1 sibling, 0 replies; 13+ messages in thread From: wenhui qiu @ 2025-02-07 06:40 UTC (permalink / raw) To: Melanie Plageman <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]> Hi > We could add autovacuum_vacuum_insert_max_threshold, but with an > insert-only workload, we can expect that the cold data is being > frozen. By calculating the threshold based on unfrozen data, we are > effectively capping the threshold for inserted data without adding > another guc. If any of that data is being unfrozen via updates or > deletes, then the autovacuum_vacuum_max_threshold would apply. > Perhaps I'm missing a case where calculating the insert threshold on > unfrozen data would not act as a cap, in which case I could get on > board with a guc. Actually ,I like your solution. Even I think this formula could use that pcnt_unfrozen parameter vacthresh = (float4) vac_base_thresh + vac_scale_factor * reltuples * pcnt_unfrozen; Thanks On Thu, Feb 6, 2025 at 11:42 PM Melanie Plageman <[email protected]> wrote: > Attached v6 is rebased over 306dc520b9dfd60 > > On Wed, Feb 5, 2025 at 8:54 PM wenhui qiu <[email protected]> wrote: > > > > Hi Melanie Plageman > > Thank you for working on this ,Actually, there were two patches aimed > at optimizing vacuum-triggered processes, and one of them reached a > consensus and has been committed: > https://commitfest.postgresql.org/52/5046/ , > https://commitfest.postgresql.org/51/5395/, Maybe referring to the > already committed patch and setting a maximum value for > vacuum_max_ins_threshold would be more acceptable. > > We could add autovacuum_vacuum_insert_max_threshold, but with an > insert-only workload, we can expect that the cold data is being > frozen. By calculating the threshold based on unfrozen data, we are > effectively capping the threshold for inserted data without adding > another guc. If any of that data is being unfrozen via updates or > deletes, then the autovacuum_vacuum_max_threshold would apply. > > Perhaps I'm missing a case where calculating the insert threshold on > unfrozen data would not act as a cap, in which case I could get on > board with a guc. > > > - Melanie > ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Trigger more frequent autovacuums of heavy insert tables @ 2025-02-07 07:54 Japin Li <[email protected]> parent: Melanie Plageman <[email protected]> 1 sibling, 0 replies; 13+ messages in thread From: Japin Li @ 2025-02-07 07:54 UTC (permalink / raw) To: Melanie Plageman <[email protected]>; +Cc: wenhui qiu <[email protected]>; Greg Sabino Mullane <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Andres Freund <[email protected]>; Peter Geoghegan <[email protected]>; David Rowley <[email protected]> On Thu, 06 Feb 2025 at 10:42, Melanie Plageman <[email protected]> wrote: > Attached v6 is rebased over 306dc520b9dfd60 > > On Wed, Feb 5, 2025 at 8:54 PM wenhui qiu <[email protected]> wrote: >> >> Hi Melanie Plageman >> Thank you for working on this ,Actually, there were two patches >> aimed at optimizing vacuum-triggered processes, and one of them >> reached a consensus and has been >> committed:https://commitfest.postgresql.org/52/5046/ , >> https://commitfest.postgresql.org/51/5395/, Maybe referring to the >> already committed patch and setting a maximum value for >> vacuum_max_ins_threshold would be more acceptable. > > We could add autovacuum_vacuum_insert_max_threshold, but with an > insert-only workload, we can expect that the cold data is being > frozen. By calculating the threshold based on unfrozen data, we are > effectively capping the threshold for inserted data without adding > another guc. If any of that data is being unfrozen via updates or > deletes, then the autovacuum_vacuum_max_threshold would apply. > > Perhaps I'm missing a case where calculating the insert threshold on > unfrozen data would not act as a cap, in which case I could get on > board with a guc. > Make sense. It appears that there is an incorrect indentation in the config.sgml file. The <literal> is accidentally removed. diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 38244409e3c..571c73668f9 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -8652,10 +8652,10 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; </term> <listitem> <para> - Specifies a fraction of the table size to add to + Specifies a fraction of the active (unfrozen) table size to add to <varname>autovacuum_vacuum_insert_threshold</varname> when deciding whether to trigger a <command>VACUUM</command>. - The default is <literal>0.2</literal> (20% of table size). + The default is <literal>0.2</literal> (20% of active table size). This parameter can only be set in the <filename>postgresql.conf</filename> file or on the server command line; but the setting can be overridden for individual tables by -- Regrads, Japin Li ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: extension_control_path and "directory" @ 2025-04-28 20:49 David E. Wheeler <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: David E. Wheeler @ 2025-04-28 20:49 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; +Cc: Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; [email protected] On Apr 25, 2025, at 17:18, Matheus Alcantara <[email protected]> wrote: > Ok, I was testing using extension_control_path = '$system:/my/custom/path' > (starting with the macro) and it was working as expected, testing with > the macro at the end does not work. Great example of why it’s useful to do as much testing as possible! That’s an entirely reasonable place to start testing :-) > The problem was on find_extension_control_filename() that was appending > the /extension at the end of the entire extension_control_path GUC value > instead of just the custom paths. Oh yeah, lol, that wouldn’t work. > To append the /extension at each path on extension_control_path would > require some changes on find_in_path() that > find_extension_control_filename() calls, which I think that it would > make the function more complicated. I've them created a similar > find_in_paths() function that works in the same way but it receives a > List of paths instead of the string of paths separated by ":". We can > get this List of paths using get_extension_control_directories() that > also handle the macro substitution like find_in_path(). > > Attached v4 with these fixes. I hope that now you should be able to omit > the /extension from the GUC value. Yes! It now works with this configuration: ```ini extension_control_path = '/Users/david/Downloads/share/postgresql:$system' dynamic_library_path = '/Users/david/Downloads/lib/postgresql:$libdir’ ``` Which is nicely more consistent. Kind of want that first one to be called “share_path” now, though, since it’s not just extensions. Although I guess it’s only extension control file searching that uses it (for now). If I understand this bit correctly: ```c /* Substitute the path macro if needed */ mangled = substitute_path_macro(piece, "$system", system_dir); /* * Append "extension" suffix in case is a custom extension control * path. */ if (strcmp(piece, "$system") != 0) mangled = psprintf("%s/extension", mangled); ``` The value of `piece` is a single path from the search path, right? If so, I think it’s either `$system` or something else; I don’t it would ever be that `$system` is a substring of a single path. Is that right? Other than that, I think this patch is good to go. Thanks! Best, David ` Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: extension_control_path and "directory" @ 2025-04-29 14:08 David E. Wheeler <[email protected]> parent: David E. Wheeler <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: David E. Wheeler @ 2025-04-29 14:08 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; +Cc: Christoph Berg <[email protected]>; Peter Eisentraut <[email protected]>; [email protected] On Apr 29, 2025, at 09:49, Matheus Alcantara <[email protected]> wrote: > Yes, it is a single path from the search path, in your case it will be > "/Users/david/Downloads/share/postgresql" and "$system". We split these > paths based on the system path separator and get the next "piece" here: > > char *piece = first_path_var_separator(ecp); > > The first_path_var_separator() changes the "ecp" parameter on every call, > it returns the next path on "ecp" and changes it to have the remaining > paths to iterate over it. Right. My point is a minor one, but I thin you can use an if/ else there: ```c if (strcmp(piece, "$system") == 0) { /* Substitute the path macro if needed */ mangled = substitute_path_macro(piece, "$system", system_dir); } else { /* * Append "extension" suffix in case is a custom extension * control path. */ mangled = psprintf("%s/extension", mangled); } ``` Best, David Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: extension_control_path and "directory" @ 2025-05-02 14:51 Peter Eisentraut <[email protected]> parent: David E. Wheeler <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Peter Eisentraut @ 2025-05-02 14:51 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; David E. Wheeler <[email protected]>; +Cc: Christoph Berg <[email protected]>; [email protected] On 29.04.25 17:06, Matheus Alcantara wrote: > On Tue, Apr 29, 2025 at 11:08 AM David E. Wheeler <[email protected]> wrote: >> Right. My point is a minor one, but I thin you can use an if/ else there: >> >> ```c >> if (strcmp(piece, "$system") == 0) { >> /* Substitute the path macro if needed */ >> mangled = substitute_path_macro(piece, "$system", system_dir); >> } else { >> /* >> * Append "extension" suffix in case is a custom extension >> * control path. >> */ >> mangled = psprintf("%s/extension", mangled); >> } >> ``` >> > > The substitute_path_macro() already handles the if/else on "piece" but I > think that this if/else version looks nicer. Fixed. > > I've also included some documentation changes for this v5 version to > remove the "extension" from the examples and also mention the scenario > when using the "directory" on the .control file. Thanks, I have committed this. I did a bit of code reformatting and adjusted the documentation a bit. It's good to get this in before beta1 so that we don't have to change the valid values of extension_control_path past beta1. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: extension_control_path and "directory" @ 2025-05-02 15:04 Matheus Alcantara <[email protected]> parent: Peter Eisentraut <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Matheus Alcantara @ 2025-05-02 15:04 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: David E. Wheeler <[email protected]>; Christoph Berg <[email protected]>; [email protected] On Fri, May 2, 2025 at 11:51 AM Peter Eisentraut <[email protected]> wrote: > > Thanks, I have committed this. I did a bit of code reformatting and > adjusted the documentation a bit. It's good to get this in before beta1 > so that we don't have to change the valid values of > extension_control_path past beta1. > Thanks Peter! -- Matheus Alcantara ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: extension_control_path and "directory" @ 2025-05-02 15:53 Christoph Berg <[email protected]> parent: Matheus Alcantara <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Christoph Berg @ 2025-05-02 15:53 UTC (permalink / raw) To: Matheus Alcantara <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; David E. Wheeler <[email protected]>; [email protected] Re: Matheus Alcantara > > Thanks, I have committed this. I did a bit of code reformatting and > > adjusted the documentation a bit. It's good to get this in before beta1 > > so that we don't have to change the valid values of > > extension_control_path past beta1. > > Thanks Peter! And thanks everyone! Christoph ^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2025-05-02 15:53 UTC | newest] Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-04-01 04:07 [PATCH 3/4] GiST-Optimal-WAL-Usage Andrey V. Lepikhov <[email protected]> 2019-04-02 04:43 [PATCH 3/4] GiST-Optimal-WAL-Usage Andrey V. Lepikhov <[email protected]> 2023-07-25 23:09 [PATCH v4 1/1] add to_binary() and to_oct() Nathan Bossart <[email protected]> 2025-02-05 22:08 Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]> 2025-02-06 01:54 ` Re: Trigger more frequent autovacuums of heavy insert tables wenhui qiu <[email protected]> 2025-02-06 15:42 ` Re: Trigger more frequent autovacuums of heavy insert tables Melanie Plageman <[email protected]> 2025-02-07 06:40 ` Re: Trigger more frequent autovacuums of heavy insert tables wenhui qiu <[email protected]> 2025-02-07 07:54 ` Re: Trigger more frequent autovacuums of heavy insert tables Japin Li <[email protected]> 2025-04-28 20:49 Re: extension_control_path and "directory" David E. Wheeler <[email protected]> 2025-04-29 14:08 ` Re: extension_control_path and "directory" David E. Wheeler <[email protected]> 2025-05-02 14:51 ` Re: extension_control_path and "directory" Peter Eisentraut <[email protected]> 2025-05-02 15:04 ` Re: extension_control_path and "directory" Matheus Alcantara <[email protected]> 2025-05-02 15:53 ` Re: extension_control_path and "directory" Christoph Berg <[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