From: Andres Freund <andres@anarazel.de>
Date: Mon, 25 May 2015 01:43:15 +0000 (+0200)
Subject: bdr: Improve sequencer locking and pgstat reporting.
X-Git-Url: http://proxy-ga.blitzz.co/gitweb/%22tables.php?a=commitdiff_plain;h=303a03bd363dc038396342fbdd6392e11c5ee5d1;p=2ndquadrant_bdr.git

bdr: Improve sequencer locking and pgstat reporting.

Up to now the sequencer locked its relations exclusively to prevent an
interlock with apply. That was problematic, because it would often
prevent auto vacuum/analyze from running, or interrupt it.

Instead of locking the underlying relations lock the bdr seqam object
and add code to apply to also acquire that lock. That allow autovacuum
to run undisturbed.

Testing brought an additional problem to light, namely that pgstat
records weren't sent in the sequencer until shutdown, which means that
in many situations autovacuum won't be trigger in time. This is a more
widespread problem than just the sequencer, but here only the sequencer
is fixed.
---

diff --git a/bdr.c b/bdr.c
index fd9cc1aa2b..e72f94644d 100644
--- a/bdr.c
+++ b/bdr.c
@@ -26,6 +26,7 @@
 
 #ifdef BUILDING_BDR
 #include "access/committs.h"
+#include "access/seqam.h"
 #endif
 #include "access/heapam.h"
 #include "access/xact.h"
@@ -77,6 +78,7 @@ Oid   BdrConflictHistoryRelId;
 Oid   BdrLocksRelid;
 Oid   BdrLocksByOwnerRelid;
 Oid   BdrReplicationSetConfigRelid;
+Oid   BdrSeqamOid;
 
 /* GUC storage */
 static bool bdr_synchronous_commit;
@@ -894,6 +896,7 @@ bdr_maintain_schema(bool update_extensions)
 		bdr_lookup_relid("bdr_global_locks", schema_oid);
 	BdrLocksByOwnerRelid =
 		bdr_lookup_relid("bdr_global_locks_byowner", schema_oid);
+	BdrSeqamOid = get_seqam_oid("bdr", false);
 #endif
 
 	bdr_conflict_handlers_init();
diff --git a/bdr.h b/bdr.h
index ffda80cf97..9bb8f994a7 100644
--- a/bdr.h
+++ b/bdr.h
@@ -318,6 +318,7 @@ extern Oid	QueuedDropsRelid;
 extern Oid	BdrSequenceValuesRelid;
 extern Oid	BdrSequenceElectionsRelid;
 extern Oid	BdrVotesRelid;
+extern Oid	BdrSeqamOid;
 #endif
 
 /* Structure representing bdr_nodes record */
@@ -415,6 +416,7 @@ extern void tuple_to_stringinfo(StringInfo s, TupleDesc tupdesc, HeapTuple tuple
 /* sequence support */
 extern void bdr_sequencer_shmem_init(int sequencers);
 extern void bdr_sequencer_init(int seq_slot, Size nnodes);
+extern void bdr_sequencer_lock(void);
 extern bool bdr_sequencer_vote(void);
 extern void bdr_sequencer_tally(void);
 extern bool bdr_sequencer_start_elections(void);
diff --git a/bdr_apply.c b/bdr_apply.c
index 624fe75049..ff16d37b1f 100644
--- a/bdr_apply.c
+++ b/bdr_apply.c
@@ -1986,6 +1986,18 @@ read_rel(StringInfo s, LOCKMODE mode)
 
 	relid = RangeVarGetRelidExtended(rv, mode, false, false, NULL, NULL);
 
+#ifdef BUILDING_BDR
+	/*
+	 * Acquire sequencer lock if any of the sequencer relations are
+	 * modified. We used to rely on relation locks, but that had problems with
+	 * deadlocks and interrupting auto-analyze/vacuum.
+	 */
+	if (relid == BdrSequenceValuesRelid ||
+		relid == BdrSequenceElectionsRelid ||
+		relid == BdrVotesRelid)
+		bdr_sequencer_lock();
+#endif
+
 	return bdr_heap_open(relid, NoLock);
 }
 
diff --git a/bdr_seq.c b/bdr_seq.c
index cfe64fc3f3..c89fbbbd1d 100644
--- a/bdr_seq.c
+++ b/bdr_seq.c
@@ -544,20 +544,21 @@ bdr_sequencer_init(int new_seq_slot, Size nnodes)
 	slot->nnodes = nnodes;
 }
 
-static void
-bdr_sequencer_lock_rel(char *relname, Oid relid)
-{
-	SetCurrentStatementStartTimestamp();
-	pgstat_report_activity(STATE_RUNNING, relname);
-	LockRelationOid(relid, ExclusiveLock);
-}
-
-static void
+/*
+ * Acquire sequencer lock.
+ *
+ * We lock on on our seqam instead of the underlying relations. That's
+ * advantageous because we only want to prevent modifications by the sequencer
+ * or apply processes, it's perfectly fine for auto-analyze/vacuum to process
+ * the relation.
+ */
+void
 bdr_sequencer_lock(void)
 {
-	bdr_sequencer_lock_rel("bdr_votes", BdrVotesRelid);
-	bdr_sequencer_lock_rel("bdr_sequence_elections", BdrSequenceElectionsRelid);
-	bdr_sequencer_lock_rel("bdr_sequence_values", BdrSequenceValuesRelid);
+	SetCurrentStatementStartTimestamp();
+	pgstat_report_activity(STATE_RUNNING, "acquiring sequencer lock");
+	LockDatabaseObject(SeqAccessMethodRelationId, BdrSeqamOid, InvalidOid,
+					   ExclusiveLock);
 }
 
 bool
@@ -625,6 +626,7 @@ bdr_sequencer_vote(void)
 	PopActiveSnapshot();
 	SPI_finish();
 	CommitTransactionCommand();
+	pgstat_report_stat(false);
 
 	elog(DEBUG1, "started %d votes", processed);
 
@@ -691,6 +693,7 @@ bdr_sequencer_start_elections(void)
 	PopActiveSnapshot();
 	SPI_finish();
 	CommitTransactionCommand();
+	pgstat_report_stat(false);
 
 	return processed > 0;
 }
@@ -757,6 +760,7 @@ bdr_sequencer_tally(void)
 	PopActiveSnapshot();
 	SPI_finish();
 	CommitTransactionCommand();
+	pgstat_report_stat(false);
 }
 
 
@@ -1030,6 +1034,7 @@ bdr_sequencer_fill_sequences(void)
 	PopActiveSnapshot();
 	SPI_finish();
 	CommitTransactionCommand();
+	pgstat_report_stat(false);
 
 	elog(DEBUG1, "checked %d sequences for filling", total);
 }