Alexander Korotkov писал 2017-09-05 00:33:
I've following comments on this patch:
1) You shouldn't use ">=" to compare xids. You should use
TransactionIdFollowsOrEquals() function which handles transaction id
wraparound correctly.
I fixed it and as an additional I add GUC parameter that could turn off
this behavior. These parameter can be set in the postgresql.conf
configuration file, or with the help of the ALTER SYSTEM command. For
the changes to take effect, call the pg_reload_conf() function or reload
the database server.
Changes are in vacuum_lazy_truncate_v3.patch
2) As I understood, this patch makes heap truncate only when no
concurrent transactions are running on both master and replicas with
hot_standby_feedback enabled. For busy system, it would be literally
"never do heap truncate after vacuum".
Yes, the difficulty is that if we have a lot of replicas and requests
for them will overlap each other, then truncation will not occur on the
loaded system
Perhaps, it's certainly bad idea to disable replaying
AccessExclusiveLock *every time*, we should skip taking
AccessExclusiveLock only while writing XLOG_SMGR_TRUNCATE record.
Besides that, this code violates out coding style.
In patch (standby_truncate_skip_v2.patch) fixed this behaviour and now
it skip writing XLOG_STANDBY_LOCK records (with in AccessExclusiveLock)
only while autovacuum truncations the table.
Amit Kapila писал 2017-09-05 12:52:
I think one deficiency of this solution is that it will keep on
generating extra WAL even if standby doesn't need it (as standby has
successfully truncated the relation). I don't know if we can just get
away by saying that an additional WAL record per vacuum cycle is
harmless especially when that doesn't serve any purpose (like for the
cases when standby is always able to truncate the heap on first WAL
record). Am I missing some part of solution which avoids it?
You are right, and this implementation generating extra WALs and it
could have some cases. If you have any solutions to avoid it, I`ll be
glade to hear them.
Simon Riggs писал 2017-09-05 14:44:
If we defer an action on standby replay, when and who will we apply
it? What happens if the standby is shutdown or crashes while an action
is pending.
Perhaps altering the way truncation requires an AccessExclusiveLock
would help workloads on both master and standby? If a Relation knew it
had a pending truncation then scans wouldn't need to go past newsize.
Once older lockers have gone we could simply truncate without the
lock. Would need a few pushups but seems less scary then trying to
make pending standby actions work well enough to commit.
Yes, it is the first idea that come to my mind and the most difficult
one. I think that in a few days I'll finish the patch with such ideas of
implementation.
------
Ivan Kartyshov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index 9a5fde0..a6dc369 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -31,6 +31,9 @@
#include "storage/smgr.h"
#include "utils/memutils.h"
#include "utils/rel.h"
+#include "storage/lmgr.h"
+#include "storage/procarray.h"
+#include "access/transam.h"
/*
* We keep a list of all relations (represented as RelFileNode values)
@@ -495,9 +498,21 @@ smgr_redo(XLogReaderState *record)
xl_smgr_truncate *xlrec = (xl_smgr_truncate *) XLogRecGetData(record);
SMgrRelation reln;
Relation rel;
+ LOCKTAG locktag;
+ VirtualTransactionId *backends;
+ VirtualTransactionId *backends2;
reln = smgropen(xlrec->rnode, InvalidBackendId);
+ SET_LOCKTAG_RELATION(locktag, reln->smgr_rnode.node.dbNode,
+ reln->smgr_rnode.node.relNode);
+
+ backends = GetLockConflicts(&locktag, AccessExclusiveLock);
+ backends2 = GetConflictingVirtualXIDs(InvalidTransactionId, InvalidOid);
+
+ if (!VirtualTransactionIdIsValid(*backends) && !VirtualTransactionIdIsValid(*backends2))
+ {
+
/*
* Forcibly create relation if it doesn't exist (which suggests that
* it was dropped somewhere later in the WAL sequence). As in
@@ -542,6 +557,10 @@ smgr_redo(XLogReaderState *record)
visibilitymap_truncate(rel, xlrec->blkno);
FreeFakeRelcacheEntry(rel);
+ } else
+ {
+ XLogFlush(lsn);
+ }
}
else
elog(PANIC, "smgr_redo: unknown op code %u", info);
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 45b1859..3c25907 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -61,6 +61,8 @@
#include "utils/pg_rusage.h"
#include "utils/timestamp.h"
#include "utils/tqual.h"
+#include "catalog/storage_xlog.h"
+#include "access/rmgr.h"
/*
@@ -317,6 +319,32 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams *params,
new_rel_pages = vacrelstats->old_rel_pages;
new_rel_tuples = vacrelstats->old_rel_tuples;
}
+ else
+ {
+ if (RelationNeedsWAL(onerel))
+ {
+ /* Get the current relation length */
+ LockRelationForExtension(onerel, ExclusiveLock);
+
+ /*
+ * Make an XLOG entry reporting the file truncation.
+ */
+ XLogRecPtr lsn;
+ xl_smgr_truncate xlrec;
+
+ xlrec.blkno = vacrelstats->rel_pages;
+ xlrec.rnode = onerel->rd_node;
+
+ XLogBeginInsert();
+ XLogRegisterData((char *) &xlrec, sizeof(xlrec));
+
+ lsn = XLogInsert(RM_SMGR_ID,
+ XLOG_SMGR_TRUNCATE | XLR_SPECIAL_REL_UPDATE);
+ UnlockRelationForExtension(onerel, ExclusiveLock);
+ XLogFlush(lsn);
+ }
+
+ }
visibilitymap_count(onerel, &new_rel_allvisible, NULL);
if (new_rel_allvisible > new_rel_pages)
@@ -397,7 +425,6 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams *params,
appendStringInfo(&buf, _("avg read rate: %.3f MB/s, avg write rate: %.3f MB/s\n"),
read_rate, write_rate);
appendStringInfo(&buf, _("system usage: %s"), pg_rusage_show(&ru0));
-
ereport(LOG,
(errmsg_internal("%s", buf.data)));
pfree(buf.data);
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index fe98898..3259cd2 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -228,7 +228,7 @@ ConditionalLockRelation(Relation relation, LOCKMODE lockmode)
LOCKTAG tag;
LockAcquireResult res;
- SET_LOCKTAG_RELATION(tag,
+ SET_LOCKTAG_RELATION_TRUNCATE(tag,
relation->rd_lockInfo.lockRelId.dbId,
relation->rd_lockInfo.lockRelId.relId);
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 2b26173..ccfc5ab 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -816,7 +816,8 @@ LockAcquireExtended(const LOCKTAG *locktag,
XLogStandbyInfoActive())
{
LogAccessExclusiveLockPrepare();
- log_lock = true;
+ if (!locktag->locktag_field4)
+ log_lock = true;
}
/*
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 765431e..125a75a 100644
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -198,6 +198,14 @@ typedef struct LOCKTAG
(locktag).locktag_type = LOCKTAG_RELATION, \
(locktag).locktag_lockmethodid = DEFAULT_LOCKMETHOD)
+#define SET_LOCKTAG_RELATION_TRUNCATE(locktag,dboid,reloid) \
+ ((locktag).locktag_field1 = (dboid), \
+ (locktag).locktag_field2 = (reloid), \
+ (locktag).locktag_field3 = 0, \
+ (locktag).locktag_field4 = 1, \
+ (locktag).locktag_type = LOCKTAG_RELATION, \
+ (locktag).locktag_lockmethodid = DEFAULT_LOCKMETHOD)
+
#define SET_LOCKTAG_RELATION_EXTEND(locktag,dboid,reloid) \
((locktag).locktag_field1 = (dboid), \
(locktag).locktag_field2 = (reloid), \
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 45b1859..72e7e77 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -61,6 +61,7 @@
#include "utils/pg_rusage.h"
#include "utils/timestamp.h"
#include "utils/tqual.h"
+#include "utils/guc.h"
/*
@@ -167,7 +168,8 @@ static bool lazy_tid_reaped(ItemPointer itemptr, void *state);
static int vac_cmp_itemptr(const void *left, const void *right);
static bool heap_page_is_all_visible(Relation rel, Buffer buf,
TransactionId *visibility_cutoff_xid, bool *all_frozen);
-
+/* GUC variables */
+bool autovacuum_truncate_wait_standby;
/*
* lazy_vacuum_rel() -- perform LAZY VACUUM for one heap relation
@@ -280,7 +282,12 @@ lazy_vacuum_rel(Relation onerel, int options, VacuumParams *params,
* Optionally truncate the relation.
*/
if (should_attempt_truncation(vacrelstats))
- lazy_truncate_heap(onerel, vacrelstats);
+ {
+ if (!autovacuum_truncate_wait_standby)
+ lazy_truncate_heap(onerel, vacrelstats);
+ else if (TransactionIdFollowsOrEquals(OldestXmin, ShmemVariableCache->latestCompletedXid))
+ lazy_truncate_heap(onerel, vacrelstats);
+ }
/* Report that we are now doing final cleanup */
pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 246fea8..52c63c8 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1582,6 +1582,16 @@ static struct config_bool ConfigureNamesBool[] =
},
{
+ {"autovacuum_truncate_wait_standby", PGC_SIGHUP, REPLICATION_MASTER,
+ gettext_noop("Make autovacuum truncate to wait till all hot_standby finish there transactions"),
+ NULL
+ },
+ &autovacuum_truncate_wait_standby,
+ false,
+ NULL, NULL, NULL
+ },
+
+ {
{"allow_system_table_mods", PGC_POSTMASTER, DEVELOPER_OPTIONS,
gettext_noop("Allows modifications of the structure of system tables."),
NULL,
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index df5d2f3..5bdc045 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -531,6 +531,8 @@
#autovacuum_vacuum_cost_limit = -1 # default vacuum cost limit for
# autovacuum, -1 means use
# vacuum_cost_limit
+#autovacuum_truncate_wait_standby = off # Make autovacuum truncate to wait till all
+ # hot_standby finish there transactions
#------------------------------------------------------------------------------
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index a903511..87d4d86 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -190,6 +190,8 @@ extern void vacuum_delay_point(void);
/* in commands/vacuumlazy.c */
extern void lazy_vacuum_rel(Relation onerel, int options,
VacuumParams *params, BufferAccessStrategy bstrategy);
+ /* user-settable parameters */
+extern bool autovacuum_truncate_wait_standby;
/* in commands/analyze.c */
extern void analyze_rel(Oid relid, RangeVar *relation, int options,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
