agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH 04/10] Make XLogReadRecord a state machine 4+ messages / 3 participants [nested] [flat]
* [PATCH 04/10] Make XLogReadRecord a state machine @ 2019-05-24 00:33 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Kyotaro Horiguchi @ 2019-05-24 00:33 UTC (permalink / raw) This patch moves the caller sites of the callback above XLogReadRecord by making XLogReadRecord a state machine. --- src/backend/access/transam/twophase.c | 8 +- src/backend/access/transam/xlog.c | 8 +- src/backend/access/transam/xlogreader.c | 108 ++++++++++++++++--------- src/backend/replication/logical/logical.c | 10 ++- src/backend/replication/logical/logicalfuncs.c | 10 ++- src/backend/replication/slotfuncs.c | 10 ++- src/backend/replication/walsender.c | 10 ++- src/bin/pg_rewind/parsexlog.c | 28 ++++++- src/bin/pg_waldump/pg_waldump.c | 11 ++- src/include/access/xlogreader.h | 12 +++ 10 files changed, 165 insertions(+), 50 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 653f685870..6feca69126 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1394,7 +1394,13 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len) errmsg("out of memory"), errdetail("Failed while allocating a WAL reading processor."))); - XLogReadRecord(xlogreader, lsn, &record, &errormsg); + while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) == + XLREAD_NEED_DATA) + xlogreader->read_page(xlogreader, + xlogreader->loadPagePtr, xlogreader->loadLen, + xlogreader->currRecPtr, xlogreader->readBuf, + &xlogreader->readPageTLI); + if (record == NULL) ereport(ERROR, (errcode_for_file_access(), diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 27ab6cc815..573b49050d 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -4261,7 +4261,13 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode, { char *errormsg; - XLogReadRecord(xlogreader, RecPtr, &record, &errormsg); + while (XLogReadRecord(xlogreader, RecPtr, &record, &errormsg) + == XLREAD_NEED_DATA) + xlogreader->read_page(xlogreader, + xlogreader->loadPagePtr, xlogreader->loadLen, + xlogreader->currRecPtr, xlogreader->readBuf, + &xlogreader->readPageTLI); + ReadRecPtr = xlogreader->ReadRecPtr; EndRecPtr = xlogreader->EndRecPtr; if (record == NULL) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 8bd0e2925d..4ab0655af5 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -282,31 +282,57 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength) /* * Attempt to read an XLOG record. * - * If RecPtr is valid, try to read a record at that position. Otherwise - * try to read a record just after the last one previously read. + * This function runs a state machine and may need to call several times until + * a record is read. * - * If the read_page callback fails to read the requested data, *record is set - * to NULL and XLREAD_FAIL is returned. The callback is expected to have - * reported the error; errormsg is set to NULL. + * At the initial state, if called with valid pRecPtr, try to read a record at + * that position. If invalid pRecPtr is given try to read a record just after + * the last one previously read. * - * If the reading fails for some other reason, *record is also set to NULL and - * XLREAD_FAIL is returned. *errormsg is set to a string with details of the - * failure. + * When a record is successfully read, returns XLREAD_SUCCESS with result + * record being stored in *record then the state machine is reset to initial + * state. + * + * Returns XLREAD_NEED_DATA if needs more data fed. In that case loadPagePtr + * and loadLen in state is set to inform the required WAL data. The caller + * shall read in the requested data into readBuf and set readLen and + * readPageTLI to the length of the data actually read and the TLI for the + * data read in respectively. In case of failure the caller shall call the + * function setting readLen to -1 and storing error message in errormsg_buf to + * inform error. + * + * If the reading fails for some reasons including caller-side error mentioned + * above, returns XLREAD_FAIL with *record being set to NULL. *errormsg is set + * to a string with details of the failure. The state machine is reset to + * initial state. * * The returned pointer (or *errormsg) points to an internal buffer that's * valid until the next call to XLogReadRecord. + * + * Note: This function is not reentrant. The state is maintained internally in + * the function. DO NOT non-local exit (ereport) from inside of this function. */ XLogReadRecordResult -XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, +XLogReadRecord(XLogReaderState *state, XLogRecPtr pRecPtr, XLogRecord **record, char **errormsg) { - XLogRecPtr targetPagePtr; - bool randAccess; - uint32 len, - total_len; - uint32 targetRecOff; - uint32 pageHeaderSize; - bool gotheader; + /* + * This function is a state machine that can exit and reenter at any place + * marked as XLR_LEAVE. All internal state needs to be preserved through + * multiple calls. + */ + static XLogRecPtr targetPagePtr; + static bool randAccess; + static uint32 len, + total_len; + static uint32 targetRecOff; + static uint32 pageHeaderSize; + static bool gotheader; + static XLogRecPtr RecPtr; + + XLR_SWITCH(XLREAD_STATE_INIT); + + RecPtr = pRecPtr; /* * randAccess indicates whether to verify the previous-record pointer of @@ -360,10 +386,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, */ while (XLogNeedData(state, targetPagePtr, Min(targetRecOff + SizeOfXLogRecord, XLOG_BLCKSZ))) - state->read_page(state, state->loadPagePtr, state->loadLen, - state->currRecPtr, state->readBuf, - &state->readPageTLI); - + XLR_LEAVE(XLREAD_STATE_PAGE, XLREAD_NEED_DATA); + if (state->readLen < 0) goto err; @@ -442,10 +466,10 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, if (total_len > len) { /* Need to reassemble record */ - char *contdata; - XLogPageHeader pageHeader; - char *buffer; - uint32 gotlen; + static char *contdata; + static XLogPageHeader pageHeader; + static char *buffer; + static uint32 gotlen; /* * Enlarge readRecordBuf as needed. @@ -475,9 +499,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, while (XLogNeedData(state, targetPagePtr, Min(total_len - gotlen + SizeOfXLogShortPHD, XLOG_BLCKSZ))) - state->read_page(state, state->loadPagePtr, state->loadLen, - state->currRecPtr, state->readBuf, - &state->readPageTLI); + XLR_LEAVE(XLREAD_STATE_CONTPAGE, XLREAD_NEED_DATA); if (state->readLen < 0) goto err; @@ -514,9 +536,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, if (state->readLen < pageHeaderSize) { while (XLogNeedData(state, targetPagePtr, pageHeaderSize)) - state->read_page(state, state->loadPagePtr, state->loadLen, - state->currRecPtr, state->readBuf, - &state->readPageTLI); + XLR_LEAVE(XLREAD_STATE_CONTPAGE_HEADER, XLREAD_NEED_DATA); } Assert(pageHeaderSize <= state->readLen); @@ -529,9 +549,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, if (state->readLen < pageHeaderSize + len) { if (XLogNeedData(state, targetPagePtr, pageHeaderSize + len)) - state->read_page(state, state->loadPagePtr, state->loadLen, - state->currRecPtr, state->readBuf, - &state->readPageTLI); + XLR_LEAVE(XLREAD_STATE_CONTRECORD, XLREAD_NEED_DATA); } memcpy(buffer, (char *) contdata, len); @@ -565,9 +583,7 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, /* Wait for the record data to become available */ while (XLogNeedData(state, targetPagePtr, Min(targetRecOff + total_len, XLOG_BLCKSZ))) - state->read_page(state, state->loadPagePtr, state->loadLen, - state->currRecPtr, state->readBuf, - &state->readPageTLI); + XLR_LEAVE(XLREAD_STATE_RECORD, XLREAD_NEED_DATA); if (state->readLen < 0) goto err; @@ -581,6 +597,8 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, state->ReadRecPtr = RecPtr; } + XLR_SWITCH_END(); + /* * Special processing if it's an XLOG SWITCH record */ @@ -593,10 +611,10 @@ XLogReadRecord(XLogReaderState *state, XLogRecPtr RecPtr, } if (DecodeXLogRecord(state, *record, errormsg)) - return XLREAD_SUCCESS; + XLR_RETURN(XLREAD_SUCCESS); *record = NULL; - return XLREAD_FAIL; + XLR_RETURN(XLREAD_FAIL); err: @@ -610,7 +628,7 @@ err: *errormsg = state->errormsg_buf; *record = NULL; - return XLREAD_FAIL; + XLR_RETURN(XLREAD_FAIL); } /* @@ -1005,6 +1023,7 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr) XLogRecPtr found = InvalidXLogRecPtr; XLogPageHeader header; XLogRecord *record; + XLogReadRecordResult result; char *errormsg; Assert(!XLogRecPtrIsInvalid(RecPtr)); @@ -1093,8 +1112,17 @@ XLogFindNextRecord(XLogReaderState *state, XLogRecPtr RecPtr) * because either we're at the first record after the beginning of a page * or we just jumped over the remaining data of a continuation. */ - while (XLogReadRecord(state, tmpRecPtr, &record, &errormsg) == XLREAD_SUCCESS) + while ((result = XLogReadRecord(state, tmpRecPtr, &record, &errormsg)) != + XLREAD_FAIL) { + if (result == XLREAD_NEED_DATA) + { + state->read_page(state, state->loadPagePtr, state->loadLen, + state->currRecPtr, state->readBuf, + &state->readPageTLI); + continue; + } + /* continue after the record */ tmpRecPtr = InvalidXLogRecPtr; diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 4f383721eb..06200ea2e9 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -481,7 +481,15 @@ DecodingContextFindStartpoint(LogicalDecodingContext *ctx) char *err = NULL; /* the read_page callback waits for new WAL */ - XLogReadRecord(ctx->reader, startptr, &record, &err); + while (XLogReadRecord(ctx->reader, startptr, &record, &err) == + XLREAD_NEED_DATA) + ctx->reader->read_page(ctx->reader, + ctx->reader->loadPagePtr, + ctx->reader->loadLen, + ctx->reader->currRecPtr, + ctx->reader->readBuf, + &ctx->reader->readPageTLI); + if (err) elog(ERROR, "%s", err); if (!record) diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c index 6fc78d7445..4d09255504 100644 --- a/src/backend/replication/logical/logicalfuncs.c +++ b/src/backend/replication/logical/logicalfuncs.c @@ -289,7 +289,15 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin XLogRecord *record; char *errm = NULL; - XLogReadRecord(ctx->reader, startptr, &record, &errm); + while (XLogReadRecord(ctx->reader, startptr, &record, &errm) == + XLREAD_NEED_DATA) + ctx->reader->read_page(ctx->reader, + ctx->reader->loadPagePtr, + ctx->reader->loadLen, + ctx->reader->currRecPtr, + ctx->reader->readBuf, + &ctx->reader->readPageTLI); + if (errm) elog(ERROR, "%s", errm); diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c index 7db8e0d044..f4f4a907ad 100644 --- a/src/backend/replication/slotfuncs.c +++ b/src/backend/replication/slotfuncs.c @@ -433,7 +433,15 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto) * Read records. No changes are generated in fast_forward mode, * but snapbuilder/slot statuses are updated properly. */ - XLogReadRecord(ctx->reader, startlsn, &record, &errm); + while (XLogReadRecord(ctx->reader, startlsn, &record, &errm) == + XLREAD_NEED_DATA) + ctx->reader->read_page(ctx->reader, + ctx->reader->loadPagePtr, + ctx->reader->loadLen, + ctx->reader->currRecPtr, + ctx->reader->readBuf, + &ctx->reader->readPageTLI); + if (errm) elog(ERROR, "%s", errm); diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 4ba43592ca..36e14ab822 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -2827,7 +2827,15 @@ XLogSendLogical(void) */ WalSndCaughtUp = false; - XLogReadRecord(logical_decoding_ctx->reader, logical_startptr, &record, &errm); + while (XLogReadRecord(logical_decoding_ctx->reader, + logical_startptr, &record, &errm) == XLREAD_NEED_DATA) + logical_decoding_ctx->reader->read_page(logical_decoding_ctx->reader, + logical_decoding_ctx->reader->loadPagePtr, + logical_decoding_ctx->reader->loadLen, + logical_decoding_ctx->reader->currRecPtr, + logical_decoding_ctx->reader->readBuf, + &logical_decoding_ctx->reader->readPageTLI); + logical_startptr = InvalidXLogRecPtr; /* xlog record was invalid */ diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c index 512005de1c..e26127206c 100644 --- a/src/bin/pg_rewind/parsexlog.c +++ b/src/bin/pg_rewind/parsexlog.c @@ -75,7 +75,14 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex, do { - XLogReadRecord(xlogreader, startpoint, &record, &errormsg); + while (XLogReadRecord(xlogreader, startpoint, &record, &errormsg) == + XLREAD_NEED_DATA) + xlogreader->read_page(xlogreader, + xlogreader->loadPagePtr, + xlogreader->loadLen, + xlogreader->currRecPtr, + xlogreader->readBuf, + &xlogreader->readPageTLI); if (record == NULL) { @@ -127,7 +134,15 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex) if (xlogreader == NULL) pg_fatal("out of memory"); - XLogReadRecord(xlogreader, ptr, &record, &errormsg); + while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) == + XLREAD_NEED_DATA) + xlogreader->read_page(xlogreader, + xlogreader->loadPagePtr, + xlogreader->loadLen, + xlogreader->currRecPtr, + xlogreader->readBuf, + &xlogreader->readPageTLI); + if (record == NULL) { if (errormsg) @@ -190,7 +205,14 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex, { uint8 info; - XLogReadRecord(xlogreader, searchptr, &record, &errormsg); + while (XLogReadRecord(xlogreader, searchptr, &record, &errormsg) == + XLREAD_NEED_DATA) + xlogreader->read_page(xlogreader, + xlogreader->loadPagePtr, + xlogreader->loadLen, + xlogreader->currRecPtr, + xlogreader->readBuf, + &xlogreader->readPageTLI); if (record == NULL) { diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index 41aa108215..e2e93f144a 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -1132,7 +1132,16 @@ main(int argc, char **argv) for (;;) { /* try to read the next record */ - XLogReadRecord(xlogreader_state, first_record, &record, &errormsg); + while (XLogReadRecord(xlogreader_state, + first_record, &record, &errormsg) == + XLREAD_NEED_DATA) + xlogreader_state->read_page(xlogreader_state, + xlogreader_state->loadPagePtr, + xlogreader_state->loadLen, + xlogreader_state->currRecPtr, + xlogreader_state->readBuf, + &xlogreader_state->readPageTLI); + if (!record) { if (!config.follow || private.endptr_reached) diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 0e734d27f1..bc0c642906 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -70,6 +70,7 @@ typedef struct typedef enum XLogReadRecordResult { XLREAD_SUCCESS, /* record is successfully read */ + XLREAD_NEED_DATA, /* need more data. see XLogReadRecord. */ XLREAD_FAIL /* failed during reading a record */ } XLogReadRecordResult; @@ -82,6 +83,17 @@ typedef enum xlnd_stateid XLND_STATE_PAGELONGHEADER } xlnd_stateid; +/* internal state of XLogReadRecord() */ +typedef enum xlread_stateid +{ + XLREAD_STATE_INIT, + XLREAD_STATE_PAGE, + XLREAD_STATE_CONTPAGE, + XLREAD_STATE_CONTPAGE_HEADER, + XLREAD_STATE_CONTRECORD, + XLREAD_STATE_RECORD +} xlread_stateid; + struct XLogReaderState { /* ---------------------------------------- -- 2.16.3 ----Next_Part(Wed_Jul_10_13_18_10_2019_842)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v4-0005-Make-XLogPageRead-not-use-callback-but-call-the-func.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v1] ci: macos: used cached macports install @ 2023-08-04 06:29 Andres Freund <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Andres Freund @ 2023-08-04 06:29 UTC (permalink / raw) This substantially speeds up the mac CI time. Author: Reviewed-by: Discussion: https://postgr.es/m/ Backpatch: --- .cirrus.yml | 63 +++++++----------- src/tools/ci/ci_macports_packages.sh | 97 ++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 38 deletions(-) create mode 100755 src/tools/ci/ci_macports_packages.sh diff --git a/.cirrus.yml b/.cirrus.yml index d260f15c4e2..e9cfc542cfe 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -429,8 +429,7 @@ task: CIRRUS_WORKING_DIR: ${HOME}/pgsql/ CCACHE_DIR: ${HOME}/ccache - HOMEBREW_CACHE: ${HOME}/homebrew-cache - PERL5LIB: ${HOME}/perl5/lib/perl5 + MACPORTS_CACHE: ${HOME}/macports-cache CC: ccache cc CXX: ccache c++ @@ -454,55 +453,43 @@ task: - mkdir ${HOME}/cores - sudo sysctl kern.corefile="${HOME}/cores/core.%P" - perl_cache: - folder: ~/perl5 - cpan_install_script: - - perl -mIPC::Run -e 1 || cpan -T IPC::Run - - perl -mIO::Pty -e 1 || cpan -T IO::Pty - upload_caches: perl - - - # XXX: Could we instead install homebrew into a cached directory? The - # homebrew installation takes a good bit of time every time, even if the - # packages do not need to be downloaded. - homebrew_cache: - folder: $HOMEBREW_CACHE + # Use macports, even though homebrew is installed. The installation + # of the additional packages we need would take quite a while with + # homebrew, even if we cache the downloads. We can't cache all of + # homebrew, because it's already large. So we use macports. To cache + # the installation we create a .dmg file that we mount if it already + # exists. + # XXX: The reason for the direct p5.34* references is that we'd need + # the large macport tree around to figure out that p5-io-tty is + # actually p5.34-io-tty. Using the unversioned name works, but + # updates macports every time. + macports_cache: + folder: ${MACPORTS_CACHE} setup_additional_packages_script: | - brew install \ + sh src/tools/ci/ci_macports_packages.sh \ ccache \ - icu4c \ - krb5 \ - llvm \ + icu \ + kerberos5 \ lz4 \ - make \ meson \ openldap \ openssl \ - python \ - tcl-tk \ + p5.34-io-tty \ + p5.34-ipc-run \ + tcl \ zstd - - brew cleanup -s # to reduce cache size - upload_caches: homebrew + # Make macports install visible for subsequent steps + echo PATH=/opt/local/sbin/:/opt/local/bin/:$PATH >> $CIRRUS_ENV + upload_caches: macports ccache_cache: folder: $CCACHE_DIR configure_script: | - brewpath="/opt/homebrew" - PKG_CONFIG_PATH="${brewpath}/lib/pkgconfig:${PKG_CONFIG_PATH}" - - for pkg in icu4c krb5 openldap openssl zstd ; do - pkgpath="${brewpath}/opt/${pkg}" - PKG_CONFIG_PATH="${pkgpath}/lib/pkgconfig:${PKG_CONFIG_PATH}" - PATH="${pkgpath}/bin:${pkgpath}/sbin:$PATH" - done - - export PKG_CONFIG_PATH PATH - + export PKG_CONFIG_PATH="/opt/local/lib/pkgconfig/" meson setup \ --buildtype=debug \ - -Dextra_include_dirs=${brewpath}/include \ - -Dextra_lib_dirs=${brewpath}/lib \ + -Dextra_include_dirs=/opt/local/include \ + -Dextra_lib_dirs=/opt/local/lib \ -Dcassert=true \ -Duuid=e2fs -Ddtrace=auto \ -Dsegsize_blocks=6 \ diff --git a/src/tools/ci/ci_macports_packages.sh b/src/tools/ci/ci_macports_packages.sh new file mode 100755 index 00000000000..5f5d3027760 --- /dev/null +++ b/src/tools/ci/ci_macports_packages.sh @@ -0,0 +1,97 @@ +#!/bin/sh + +# Installs the passed in packages via macports. To make it fast enough +# for CI, cache the installation as a .dmg file. To avoid +# unnecessarily updating the cache, the cached image is only modified +# when packages are installed or removed. Any package this script is +# not instructed to install, will be removed again. +# +# This currently expects to be run in a macos cirrus-ci environment. + +set -e +set -x + +packages="$@" + +macports_url="https://github.com/macports/macports-base/releases/download/v2.8.1/MacPorts-2.8.1-13-Ventura.pkg"; +cache_dmg="macports.hfs.dmg" + +if [ "$CIRRUS_CI" != "true" ]; then + echo "expect to be called within cirrus-ci" 1>2 + exit 1 +fi + +sudo mkdir -p /opt/local +mkdir -p ${MACPORTS_CACHE}/ + +# If we are starting from clean cache, perform a fresh macports +# install. Otherwise decompress the .dmg we created previously. +# +# After this we have a working macports installation, with an unknown set of +# packages installed. +new_install=0 +update_cached_image=0 +if [ -e ${MACPORTS_CACHE}/${cache_dmg}.zstd ]; then + time zstd -T0 -d ${MACPORTS_CACHE}/${cache_dmg}.zstd -o ${cache_dmg} + time sudo hdiutil attach -kernel ${cache_dmg} -owners on -shadow ${cache_dmg}.shadow -mountpoint /opt/local +else + new_install=1 + curl -fsSL -o macports.pkg "$macports_url" + time sudo installer -pkg macports.pkg -target / + # this is a throwaway environment, and it'd be a few lines to gin + # up a correct user / group when using the cache. + echo macportsuser root | sudo tee -a /opt/local/etc/macports/macports.conf +fi +export PATH=/opt/local/sbin/:/opt/local/bin/:$PATH + +# mark all installed packages unrequested, that allows us to detect +# packages that aren't needed anymore +if [ -n "$(port -q installed installed)" ] ; then + sudo port unsetrequested installed +fi + +# if setting all the required packages as requested fails, we need +# to install at least one of them +if ! sudo port setrequested $packages > /dev/null 2>&1 ; then + echo not all required packages installed, doing so now + update_cached_image=1 + # to keep the image small, we deleted the ports tree from the image... + sudo port selfupdate + # XXX likely we'll need some other way to force an upgrade at some + # point... + sudo port upgrade outdated + sudo port install -N $packages + sudo port setrequested $packages +fi + +# check if any ports should be uninstalled +if [ -n "$(port -q installed rleaves)" ] ; then + echo superflous packages installed + update_cached_image=1 + sudo port uninstall --follow-dependencies rleaves + + # remove prior cache contents, don't want to increase size + rm -f ${MACPORTS_CACHE}/* +fi + +# Shrink installation if we created / modified it +if [ "$new_install" -eq 1 -o "$update_cached_image" -eq 1 ]; then + sudo /opt/local/bin/port clean --all installed + sudo rm -rf /opt/local/var/macports/{software,sources}/* +fi + +# If we're starting from a clean cache, start a new image. If we have +# an image, but the contents changed, update the image in the cache +# location. +if [ "$new_install" -eq 1 ]; then + # use a generous size, so additional software can be installed later + time sudo hdiutil create -fs HFS+ -format UDRO -size 10g -layout NONE -srcfolder /opt/local/ ${cache_dmg} + time zstd -T -10 -z ${cache_dmg} -o ${MACPORTS_CACHE}/${cache_dmg}.zstd +elif [ "$update_cached_image" -eq 1 ]; then + sudo hdiutil detach /opt/local/ + time hdiutil convert -format UDRO ${cache_dmg} -shadow ${cache_dmg}.shadow -o updated.hfs.dmg + rm ${cache_dmg}.shadow + mv updated.hfs.dmg ${cache_dmg} + time zstd -T -10 -z ${cache_dmg} -o ${MACPORTS_CACHE}/${cache_dmg}.zstd + time sudo hdiutil attach -kernel ${cache_dmg} -owners on -shadow ${cache_dmg}.shadow -mountpoint /opt/local +fi -- 2.38.0 --daajsth7ljgil4ye-- ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v9 02/10] heapam: Use exclusive lock on old page in CLUSTER @ 2025-01-26 20:18 Andres Freund <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Andres Freund @ 2025-01-26 20:18 UTC (permalink / raw) To be able to guarantee that we can set the hint bit, acquire an exclusive lock on the old buffer. This is required as a future commit will only allow hint bits to be set with a new lock level, which is acquired as-needed in a non-blocking fashion. We need the hint bits, set in heapam_relation_copy_for_cluster() -> HeapTupleSatisfiesVacuum(), to be set, as otherwise reform_and_rewrite_tuple() -> rewrite_heap_tuple() will get confused. Specifically, rewrite_heap_tuple() checks for HEAP_XMAX_INVALID in the old tuple to determine whether to check the old-to-new mapping hash table. It'd be better if we somehow could avoid setting hint bits on the old page. A common reason to use VACUUM FULL are very bloated tables - rewriting most of the old table before during VACUUM FULL doesn't exactly help. Author: Reviewed-by: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/access/heap/heapam_handler.c | 16 +++++++++++++++- src/backend/access/heap/heapam_visibility.c | 7 +++++++ src/backend/access/heap/rewriteheap.c | 3 +++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 09a456e9966..505faaaf9d2 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -837,7 +837,21 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap, tuple = ExecFetchSlotHeapTuple(slot, false, NULL); buf = hslot->buffer; - LockBuffer(buf, BUFFER_LOCK_SHARE); + /* + * To be able to guarantee that we can set the hint bit, acquire an + * exclusive lock on the old buffer. We need the hint bits, set in + * heapam_relation_copy_for_cluster() -> HeapTupleSatisfiesVacuum(), + * to be set, as otherwise reform_and_rewrite_tuple() -> + * rewrite_heap_tuple() will get confused. Specifically, + * rewrite_heap_tuple() checks for HEAP_XMAX_INVALID in the old tuple + * to determine whether to check the old-to-new mapping hash table. + * + * It'd be better if we somehow could avoid setting hint bits on the + * old page. One reason to use VACUUM FULL are very bloated tables - + * rewriting most of the old table before during VACUUM FULL doesn't + * exactly help... + */ + LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE); switch (HeapTupleSatisfiesVacuum(tuple, OldestXmin, buf)) { diff --git a/src/backend/access/heap/heapam_visibility.c b/src/backend/access/heap/heapam_visibility.c index 05e70b7d92a..9a034d5c9e8 100644 --- a/src/backend/access/heap/heapam_visibility.c +++ b/src/backend/access/heap/heapam_visibility.c @@ -141,6 +141,13 @@ void HeapTupleSetHintBits(HeapTupleHeader tuple, Buffer buffer, uint16 infomask, TransactionId xid) { + /* + * The uses from heapam.c rely on being able to perform the hint bit + * updates, which can only be guaranteed if we are holding an exclusive + * lock on the buffer - which all callers are doing. + */ + Assert(BufferIsLockedByMeInMode(buffer, BUFFER_LOCK_EXCLUSIVE)); + SetHintBits(tuple, buffer, infomask, xid); } diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index bae3a2da77a..77fd48eb59e 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -382,6 +382,9 @@ rewrite_heap_tuple(RewriteState state, /* * If the tuple has been updated, check the old-to-new mapping hash table. + * + * Note that this check relies on the HeapTupleSatisfiesVacuum() in + * heapam_relation_copy_for_cluster() to have set hint bits. */ if (!((old_tuple->t_data->t_infomask & HEAP_XMAX_INVALID) || HeapTupleHeaderIsOnlyLocked(old_tuple->t_data)) && -- 2.48.1.76.g4e746b1a31.dirty --rkiyqpij3ajqn7ww Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0003-heapam-Add-batch-mode-mvcc-check-and-use-it-in-pa.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH] Allow progress tracking of sub-commands. @ 2026-05-04 09:32 Antonin Houska <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Antonin Houska @ 2026-05-04 09:32 UTC (permalink / raw) Some commands that support progress reporting run sub-commands, which also report their progress. The typical case is that REPACK builds indexes. Instead of disabling the progress tracking of the sub-commands, we can allow both the "parent" command and the sub-command to report their progress at the same time. --- contrib/file_fdw/file_fdw.c | 44 +++++++++++ src/backend/commands/indexcmds.c | 10 ++- src/backend/commands/repack.c | 14 +++- src/backend/utils/activity/backend_progress.c | 76 ++++++++++++++++--- src/backend/utils/activity/backend_status.c | 1 + src/backend/utils/adt/pgstatfuncs.c | 9 ++- src/include/utils/backend_status.h | 7 ++ 7 files changed, 142 insertions(+), 19 deletions(-) diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index 33a37d832ce..a60fb226320 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -36,6 +36,7 @@ #include "optimizer/pathnode.h" #include "optimizer/planmain.h" #include "optimizer/restrictinfo.h" +#include "utils/backend_status.h" #include "utils/acl.h" #include "utils/memutils.h" #include "utils/rel.h" @@ -119,6 +120,16 @@ typedef struct FileFdwExecutionState CopyFromState cstate; /* COPY execution state */ } FileFdwExecutionState; +/* + * Since progress tracking of multiple COPY commands is not supported, the + * first file_fdw node of the plan needs to set pgstat_track_activities to + * false during startup, and the last active node needs to restore the + * original value during shutdown. + */ +static bool save_pgstat_track_activities = false; +static int fdw_nodes = 0; +static int active_fdw_nodes = 0; + /* * SQL functions */ @@ -616,6 +627,12 @@ fileGetForeignPlan(PlannerInfo *root, { Index scan_relid = baserel->relid; + /* + * This seems to be the appropriate place to count file_fdw nodes in the + * plan. + */ + fdw_nodes++; + /* * We have no native ability to evaluate restriction clauses, so we just * put all the scan_clauses into the plan node's qual list for the @@ -695,6 +712,18 @@ fileBeginForeignScan(ForeignScanState *node, int eflags) /* Add any options from the plan (currently only convert_selectively) */ options = list_concat(options, plan->fdw_private); + /* + * Save the value of pgstat_track_activities if this is the first file_fdw + * node of a plan containing multiple file_fdw nodes, and disable the + * progress tracking. The monitoring infrastructure currently does not + * support monitoring of multiple COPY commands. + */ + if (fdw_nodes > 1 && active_fdw_nodes++ == 0) + { + save_pgstat_track_activities = pgstat_track_activities; + pgstat_track_activities = false; + } + /* * Create CopyState from FDW options. We always acquire all columns, so * as to match the expected ScanTupleSlot signature. @@ -861,6 +890,21 @@ fileEndForeignScan(ForeignScanState *node) festate->cstate->num_errors)); EndCopyFrom(festate->cstate); + + + /* + * Restore the value of pgstat_track_activities if this is the last + * file_fdw node of a plan containing multiple file_fdw nodes, and enable + * progress tracking if we disabled it earlier. + */ + if (active_fdw_nodes > 0) + { + if (--active_fdw_nodes == 0) + { + pgstat_track_activities = save_pgstat_track_activities; + fdw_nodes = 0; + } + } } /* diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 9ab74c8df0a..7280e64d118 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3918,6 +3918,12 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein * more detailed comments. */ + /* + * XXX Is there a reason not to start progress reporting here? If it's ok, + * then INDEX_CREATE_SUPPRESS_PROGRESS below is probably not needed. + */ + pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, relationOid); + foreach(lc, indexIds) { char *concurrentName; @@ -3966,8 +3972,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP) elog(ERROR, "cannot reindex a temporary table concurrently"); - pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, idx->tableId); - progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY; progress_vals[1] = 0; /* initializing */ progress_vals[2] = idx->indexId; @@ -4144,7 +4148,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein * Update progress for the index to build, with the correct parent * table involved. */ - pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, newidx->tableId); progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY; progress_vals[1] = PROGRESS_CREATEIDX_PHASE_BUILD; progress_vals[2] = newidx->indexId; @@ -4208,7 +4211,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein * Update progress for the index to build, with the correct parent * table involved. */ - pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, newidx->tableId); progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY; progress_vals[1] = PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN; progress_vals[2] = newidx->indexId; diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index 9d162957bc3..f360f37a9da 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -1937,6 +1937,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, pgstat_progress_update_param(PROGRESS_REPACK_PHASE, PROGRESS_REPACK_PHASE_REBUILD_INDEX); + reindex_params.options |= REINDEXOPT_REPORT_PROGRESS; reindex_relation(NULL, OIDOldHeap, reindex_flags, &reindex_params); } @@ -3204,9 +3205,20 @@ build_new_indexes(Relation NewHeap, Relation OldHeap, List *OldIndexes) "repacknew", get_rel_namespace(ind->rd_index->indrelid), false); - newindex = index_create_copy(NewHeap, INDEX_CREATE_SUPPRESS_PROGRESS, + + /* + * We build the index on the new heap, but after the swap phase it'll + * become an index on the old heap. It makes more sense to report the + * progress this way. (The reporting API expects that both command and + * subcommand deal with the same target.) + */ + pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, + RelationGetRelid(OldHeap)); + newindex = index_create_copy(NewHeap, 0, oldindex, ind->rd_rel->reltablespace, newName); + pgstat_progress_end_command(); + copy_index_constraints(ind, newindex, RelationGetRelid(NewHeap)); result = lappend_oid(result, newindex); diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c index b0359771de5..97965a6973c 100644 --- a/src/backend/utils/activity/backend_progress.c +++ b/src/backend/utils/activity/backend_progress.c @@ -22,6 +22,10 @@ * * Set st_progress_command (and st_progress_command_target) in own backend * entry. Also, zero-initialize st_progress_param array. + * + * If command has already been started, start a sub-command. Only parameters + * of the sub-command are updated until pgstat_progress_end_command() is + * called. (Target relation must be the same for both commands.) *----------- */ void @@ -33,9 +37,30 @@ pgstat_progress_start_command(ProgressCommandType cmdtype, Oid relid) return; PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); - beentry->st_progress_command = cmdtype; - beentry->st_progress_command_target = relid; - MemSet(&beentry->st_progress_param, 0, sizeof(beentry->st_progress_param)); + /* Sub-command should not be started w/o parent command. */ + if (beentry->st_progress_command == PROGRESS_COMMAND_INVALID) + { + Assert(beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID); + + beentry->st_progress_command = cmdtype; + beentry->st_progress_command_target = relid; + MemSet(&beentry->st_progress_param, 0, + sizeof(beentry->st_progress_param)); + } + else if (beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID) + { + Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID); + Assert(beentry->st_progress_command_target == relid); + + beentry->st_progress_command2 = cmdtype; + MemSet(&beentry->st_progress_param2, 0, + sizeof(beentry->st_progress_param2)); + } + else + { + /* Only one level of nesting is supported. */ + Assert(false); + } PGSTAT_END_WRITE_ACTIVITY(beentry); } @@ -49,14 +74,20 @@ void pgstat_progress_update_param(int index, int64 val) { volatile PgBackendStatus *beentry = MyBEEntry; + volatile int64 *params; Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM); + Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID || + beentry->st_progress_command2 != PROGRESS_COMMAND_INVALID); if (!beentry || !pgstat_track_activities) return; + params = (beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID) ? + beentry->st_progress_param : beentry->st_progress_param2; + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); - beentry->st_progress_param[index] = val; + params[index] = val; PGSTAT_END_WRITE_ACTIVITY(beentry); } @@ -70,14 +101,20 @@ void pgstat_progress_incr_param(int index, int64 incr) { volatile PgBackendStatus *beentry = MyBEEntry; + volatile int64 *params; Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM); + Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID || + beentry->st_progress_command2 != PROGRESS_COMMAND_INVALID); if (!beentry || !pgstat_track_activities) return; + params = (beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID) ? + beentry->st_progress_param : beentry->st_progress_param2; + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); - beentry->st_progress_param[index] += incr; + params[index] += incr; PGSTAT_END_WRITE_ACTIVITY(beentry); } @@ -124,17 +161,24 @@ pgstat_progress_update_multi_param(int nparam, const int *index, { volatile PgBackendStatus *beentry = MyBEEntry; int i; + volatile int64 *params; if (!beentry || !pgstat_track_activities || nparam == 0) return; + Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID || + beentry->st_progress_command2 != PROGRESS_COMMAND_INVALID); + + params = (beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID) ? + beentry->st_progress_param : beentry->st_progress_param2; + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); for (i = 0; i < nparam; ++i) { Assert(index[i] >= 0 && index[i] < PGSTAT_NUM_PROGRESS_PARAM); - beentry->st_progress_param[index[i]] = val[i]; + params[index[i]] = val[i]; } PGSTAT_END_WRITE_ACTIVITY(beentry); @@ -144,7 +188,7 @@ pgstat_progress_update_multi_param(int nparam, const int *index, * pgstat_progress_end_command() - * * Reset st_progress_command (and st_progress_command_target) in own backend - * entry. This signals the end of the command. + * entry. This signals the end of the command (or a sub-command). *----------- */ void @@ -155,11 +199,19 @@ pgstat_progress_end_command(void) if (!beentry || !pgstat_track_activities) return; - if (beentry->st_progress_command == PROGRESS_COMMAND_INVALID) - return; - PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); - beentry->st_progress_command = PROGRESS_COMMAND_INVALID; - beentry->st_progress_command_target = InvalidOid; + + if (beentry->st_progress_command2 != PROGRESS_COMMAND_INVALID) + { + Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID); + + beentry->st_progress_command2 = PROGRESS_COMMAND_INVALID; + } + else + { + beentry->st_progress_command = PROGRESS_COMMAND_INVALID; + beentry->st_progress_command_target = InvalidOid; + } + PGSTAT_END_WRITE_ACTIVITY(beentry); } diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index d685fc5cd87..15675992415 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -284,6 +284,7 @@ pgstat_bestart_initial(void) lbeentry.st_state = STATE_STARTING; lbeentry.st_progress_command = PROGRESS_COMMAND_INVALID; + lbeentry.st_progress_command2 = PROGRESS_COMMAND_INVALID; lbeentry.st_progress_command_target = InvalidOid; lbeentry.st_query_id = INT64CONST(0); lbeentry.st_plan_id = INT64CONST(0); diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index 7a9dfa9ba3b..c2257d900af 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -314,6 +314,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) Datum values[PG_STAT_GET_PROGRESS_COLS] = {0}; bool nulls[PG_STAT_GET_PROGRESS_COLS] = {0}; int i; + volatile int64 *params; local_beentry = pgstat_get_local_beentry_by_index(curr_backend); beentry = &local_beentry->backendStatus; @@ -322,7 +323,11 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) * Report values for only those backends which are running the given * command. */ - if (beentry->st_progress_command != cmdtype) + if (beentry->st_progress_command == cmdtype) + params = beentry->st_progress_param; + else if (beentry->st_progress_command2 == cmdtype) + params = beentry->st_progress_param2; + else continue; /* Value available to all callers */ @@ -334,7 +339,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS) { values[2] = ObjectIdGetDatum(beentry->st_progress_command_target); for (i = 0; i < PGSTAT_NUM_PROGRESS_PARAM; i++) - values[i + 3] = Int64GetDatum(beentry->st_progress_param[i]); + values[i + 3] = Int64GetDatum(params[i]); } else { diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h index a334e096e4a..f528f7abeec 100644 --- a/src/include/utils/backend_status.h +++ b/src/include/utils/backend_status.h @@ -169,6 +169,13 @@ typedef struct PgBackendStatus Oid st_progress_command_target; int64 st_progress_param[PGSTAT_NUM_PROGRESS_PARAM]; + /* + * Some commands have a sub-command, e.g. REPACK (re)builds indexes. The + * subcommands are supposed to have the same target. + */ + ProgressCommandType st_progress_command2; + int64 st_progress_param2[PGSTAT_NUM_PROGRESS_PARAM]; + /* query identifier, optionally computed using post_parse_analyze_hook */ int64 st_query_id; -- 2.47.3 --=-=-=-- ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-05-04 09:32 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-05-24 00:33 [PATCH 04/10] Make XLogReadRecord a state machine Kyotaro Horiguchi <[email protected]> 2023-08-04 06:29 [PATCH v1] ci: macos: used cached macports install Andres Freund <[email protected]> 2025-01-26 20:18 [PATCH v9 02/10] heapam: Use exclusive lock on old page in CLUSTER Andres Freund <[email protected]> 2026-05-04 09:32 [PATCH] Allow progress tracking of sub-commands. Antonin Houska <[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