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 fd9cc1aa2bcf536028aa655f649c859e0327b07b..e72f94644de879579080f7e067906974c03c5420 100644 (file)
--- 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 ffda80cf9779593ff13d458f57740c2cb8779d88..9bb8f994a7c0a9301cce85c32aa9f8a64c5bcde9 100644 (file)
--- 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 624fe7504921af00433ae0c429db1ed794880a61..ff16d37b1f560d7f8b9c5205c86844f66c1fc71b 100644 (file)
--- 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 cfe64fc3f31948724e125f9e842143b40744c31a..c89fbbbd1d4db63c5d3a1770390bd4fa956fc5f0 100644 (file)
--- 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);
 }