Re: [HACKERS] make async slave to wait for lsn to be replayed

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Kartyshov Ivan <i(dot)kartyshov(at)postgrespro(dot)ru>, Euler Taveira <euler(at)eulerto(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [HACKERS] make async slave to wait for lsn to be replayed
Date: 2024-04-03 06:58:41
Message-ID: 202404030658.hhj3vfxeyhft@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello, I noticed that commit 06c418e163e9 uses waitLSN->mutex (a
spinlock) to protect the contents of waitLSN -- and it's used to walk an
arbitrary long list of processes waiting ... and also, an arbitrary
number of processes could be calling this code. I think using a
spinlock for this is unwise, as it'll cause busy-waiting whenever
there's contention. Wouldn't it be better to use an LWLock for this?
Then the processes would sleep until the lock is freed.

While nosing about the code, other things struck me:

I think there should be more comments about WaitLSNProcInfo and
WaitLSNState in waitlsn.h.

In addLSNWaiter it'd be better to assign 'cur' before acquiring the
lock.

Is a plan array really the most efficient data structure for this,
considering that you have to reorder each time you add an element?
Maybe it is, but wouldn't it make sense to use memmove() when adding one
element rather iterating all the remaining elements to the end of the
queue?

I think the include list in waitlsn.c could be tightened a bit:

@@ -18,28 +18,18 @@
#include <math.h>

#include "pgstat.h"
-#include "fmgr.h"
-#include "access/transam.h"
-#include "access/xact.h"
#include "access/xlog.h"
-#include "access/xlogdefs.h"
#include "access/xlogrecovery.h"
-#include "catalog/pg_type.h"
#include "commands/waitlsn.h"
-#include "executor/spi.h"
#include "funcapi.h"
#include "miscadmin.h"
-#include "storage/ipc.h"
#include "storage/latch.h"
-#include "storage/pmsignal.h"
#include "storage/proc.h"
#include "storage/shmem.h"
-#include "storage/sinvaladt.h"
-#include "utils/builtins.h"
#include "utils/pg_lsn.h"
#include "utils/snapmgr.h"
-#include "utils/timestamp.h"
#include "utils/fmgrprotos.h"
+#include "utils/wait_event_types.h"

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Las cosas son buenas o malas segun las hace nuestra opinión" (Lisias)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2024-04-03 07:13:01 Re: RFC: Additional Directory for Extensions
Previous Message Bertrand Drouvot 2024-04-03 06:50:19 Re: Introduce XID age and inactive timeout based replication slot invalidation