public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 2/8] Pass all scan keys to BRIN consistent function at once
13+ messages / 9 participants
[nested] [flat]
* [PATCH 2/8] Pass all scan keys to BRIN consistent function at once
@ 2020-09-12 13:07 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw)
Passing all scan keys to the BRIN consistent function at once may allow
elimination of additional ranges, which would be impossible when only
passing individual scan keys.
The code continues to support both the original (one scan key at a time)
and new (all scan keys at once) approaches, depending on whether the
consistent function accepts three or four arguments.
Author: Tomas Vondra <[email protected]>
Reviewed-by: Alvaro Herrera <[email protected]>
Reviewed-by: Mark Dilger <[email protected]>
Reviewed-by: Alexander Korotkov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/brin/brin.c | 158 +++++++++++++++++++-----
src/backend/access/brin/brin_validate.c | 4 +-
2 files changed, 126 insertions(+), 36 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 27ba596c6e..f9a0476024 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -390,6 +390,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
BrinMemTuple *dtup;
BrinTuple *btup = NULL;
Size btupsz = 0;
+ ScanKey **keys;
+ int *nkeys;
+ int keyno;
opaque = (BrinOpaque *) scan->opaque;
bdesc = opaque->bo_bdesc;
@@ -411,6 +414,66 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
*/
consistentFn = palloc0(sizeof(FmgrInfo) * bdesc->bd_tupdesc->natts);
+ /*
+ * Make room for per-attribute lists of scan keys that we'll pass to the
+ * consistent support procedure. We allocate space for all attributes, so
+ * that we don't have to bother determining which attributes are used.
+ *
+ * XXX The widest table can have ~1600 attributes, so this may allocate a
+ * couple kilobytes of memory). We could invent a more compact approach
+ * (with just space for used attributes) but that would make the matching
+ * more complicated, so it may not be a win.
+ */
+ keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+ nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ /*
+ * Preprocess the scan keys - split them into per-attribute arrays.
+ */
+ for (keyno = 0; keyno < scan->numberOfKeys; keyno++)
+ {
+ ScanKey key = &scan->keyData[keyno];
+ AttrNumber keyattno = key->sk_attno;
+
+ /*
+ * The collation of the scan key must match the collation used in the
+ * index column (but only if the search is not IS NULL/ IS NOT NULL).
+ * Otherwise we shouldn't be using this index ...
+ */
+ Assert((key->sk_flags & SK_ISNULL) ||
+ (key->sk_collation ==
+ TupleDescAttr(bdesc->bd_tupdesc,
+ keyattno - 1)->attcollation));
+
+ /* First time we see this index attribute, so init as needed. */
+ if (!keys[keyattno - 1])
+ {
+ FmgrInfo *tmp;
+
+ /*
+ * This is a bit of an overkill - we don't know how many scan keys
+ * are there for this attribute, so we simply allocate the largest
+ * number possible. This may waste a bit of memory, but we only
+ * expect small number of scan keys in general, so this should be
+ * negligible, and it's cheaper than having to repalloc
+ * repeatedly.
+ */
+ keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
+
+ /* First time this column, so look up consistent function */
+ Assert(consistentFn[keyattno - 1].fn_oid == InvalidOid);
+
+ tmp = index_getprocinfo(idxRel, keyattno,
+ BRIN_PROCNUM_CONSISTENT);
+ fmgr_info_copy(&consistentFn[keyattno - 1], tmp,
+ CurrentMemoryContext);
+ }
+
+ /* Add key to the per-attribute array. */
+ keys[keyattno - 1][nkeys[keyattno - 1]] = key;
+ nkeys[keyattno - 1]++;
+ }
+
/* allocate an initial in-memory tuple, out of the per-range memcxt */
dtup = brin_new_memtuple(bdesc);
@@ -471,7 +534,7 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
}
else
{
- int keyno;
+ int attno;
/*
* Compare scan keys with summary values stored for the range.
@@ -481,51 +544,78 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
* no keys.
*/
addrange = true;
- for (keyno = 0; keyno < scan->numberOfKeys; keyno++)
+ for (attno = 1; attno <= bdesc->bd_tupdesc->natts; attno++)
{
- ScanKey key = &scan->keyData[keyno];
- AttrNumber keyattno = key->sk_attno;
- BrinValues *bval = &dtup->bt_columns[keyattno - 1];
+ BrinValues *bval;
Datum add;
- /*
- * The collation of the scan key must match the collation
- * used in the index column (but only if the search is not
- * IS NULL/ IS NOT NULL). Otherwise we shouldn't be using
- * this index ...
- */
- Assert((key->sk_flags & SK_ISNULL) ||
- (key->sk_collation ==
- TupleDescAttr(bdesc->bd_tupdesc,
- keyattno - 1)->attcollation));
+ /* skip attributes without any scan keys */
+ if (nkeys[attno - 1] == 0)
+ continue;
- /* First time this column? look up consistent function */
- if (consistentFn[keyattno - 1].fn_oid == InvalidOid)
- {
- FmgrInfo *tmp;
+ bval = &dtup->bt_columns[attno - 1];
- tmp = index_getprocinfo(idxRel, keyattno,
- BRIN_PROCNUM_CONSISTENT);
- fmgr_info_copy(&consistentFn[keyattno - 1], tmp,
- CurrentMemoryContext);
- }
+ Assert((nkeys[attno - 1] > 0) &&
+ (nkeys[attno - 1] <= scan->numberOfKeys));
/*
* Check whether the scan key is consistent with the page
* range values; if so, have the pages in the range added
* to the output bitmap.
*
- * When there are multiple scan keys, failure to meet the
- * criteria for a single one of them is enough to discard
- * the range as a whole, so break out of the loop as soon
- * as a false return value is obtained.
+ * The opclass may or may not support processing of
+ * multiple scan keys. We can determine that based on the
+ * number of arguments - functions with extra parameter
+ * (number of scan keys) do support this, otherwise we
+ * have to simply pass the scan keys one by one, as
+ * before.
*/
- add = FunctionCall3Coll(&consistentFn[keyattno - 1],
- key->sk_collation,
- PointerGetDatum(bdesc),
- PointerGetDatum(bval),
- PointerGetDatum(key));
- addrange = DatumGetBool(add);
+ if (consistentFn[attno - 1].fn_nargs >= 4)
+ {
+ Oid collation;
+
+ /*
+ * Collation from the first key (has to be the same
+ * for all keys for the same attribue).
+ */
+ collation = keys[attno - 1][0]->sk_collation;
+
+ /* Check all keys at once */
+ add = FunctionCall4Coll(&consistentFn[attno - 1],
+ collation,
+ PointerGetDatum(bdesc),
+ PointerGetDatum(bval),
+ PointerGetDatum(keys[attno - 1]),
+ Int32GetDatum(nkeys[attno - 1]));
+ addrange = DatumGetBool(add);
+ }
+ else
+ {
+ /*
+ * Check keys one by one
+ *
+ * When there are multiple scan keys, failure to meet
+ * the criteria for a single one of them is enough to
+ * discard the range as a whole, so break out of the
+ * loop as soon as a false return value is obtained.
+ */
+ int keyno;
+
+ for (keyno = 0; keyno < nkeys[attno - 1]; keyno++)
+ {
+ add = FunctionCall3Coll(&consistentFn[attno - 1],
+ keys[attno - 1][keyno]->sk_collation,
+ PointerGetDatum(bdesc),
+ PointerGetDatum(bval),
+ PointerGetDatum(keys[attno - 1][keyno]));
+ addrange = DatumGetBool(add);
+
+ /* mismatching key, no need to look further */
+ if (!addrange)
+ break;
+ }
+ }
+
if (!addrange)
break;
}
diff --git a/src/backend/access/brin/brin_validate.c b/src/backend/access/brin/brin_validate.c
index 6d4253c05e..11835d85cd 100644
--- a/src/backend/access/brin/brin_validate.c
+++ b/src/backend/access/brin/brin_validate.c
@@ -97,8 +97,8 @@ brinvalidate(Oid opclassoid)
break;
case BRIN_PROCNUM_CONSISTENT:
ok = check_amproc_signature(procform->amproc, BOOLOID, true,
- 3, 3, INTERNALOID, INTERNALOID,
- INTERNALOID);
+ 3, 4, INTERNALOID, INTERNALOID,
+ INTERNALOID, INT4OID);
break;
case BRIN_PROCNUM_UNION:
ok = check_amproc_signature(procform->amproc, BOOLOID, true,
--
2.26.2
--------------82D677E9F94AC7574E460205
Content-Type: text/x-patch; charset=UTF-8;
name="0003-Process-all-scan-keys-in-existing-BRIN-opcl-20210308.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0003-Process-all-scan-keys-in-existing-BRIN-opcl-20210308.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 04:07 Andrey Borodin <[email protected]>
0 siblings, 2 replies; 13+ messages in thread
From: Andrey Borodin @ 2025-04-16 04:07 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; [email protected]
> On 16 Apr 2025, at 04:19, Tom Lane <[email protected]> wrote:
>
> feebly, and seems to have a bus factor of 1. Another example is the
> Spencer regex engine; we thought we could depend on Tcl to be the
> upstream for that, but for a decade or more they've acted as though
> *we* are the upstream.
I think it's what Konstantin is proposing. To have our own Raft implementation, without dependencies.
IMO to better understand what is proposed we need some more description of proposed systems. How the new system will be configured? initdb and what than? How new node joins cluster? What is running pg_rewind when necessary?
Some time ago Peter E proposed to be able to start replication atop of empty directory, so that initial sync would be more straightforward. And also Heikki proposed to remove archive race condition when choosing new timeline. I think this steps are gradual movement in the same direction.
My view is what Konstantin wants is automatic replication topology management. For some reason this technology is called HA, DCS, Raft, Paxos and many other scary words. But basically it manages primary_conn_info of some nodes to provide some fault-tolerance properties. I'd start to design from here, not from Raft paper.
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 04:26 Tom Lane <[email protected]>
parent: Andrey Borodin <[email protected]>
1 sibling, 2 replies; 13+ messages in thread
From: Tom Lane @ 2025-04-16 04:26 UTC (permalink / raw)
To: Andrey Borodin <[email protected]>; +Cc: Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; [email protected]
Andrey Borodin <[email protected]> writes:
> I think it's what Konstantin is proposing. To have our own Raft implementation, without dependencies.
Hmm, OK. I thought that the proposal involved relying on some existing
code, but re-reading the thread that was said nowhere. Still, that
moves it from a large project to a really large project :-(
I continue to think that it'd be best to try to implement it as
an extension, at least up till the point of finding show-stopping
reasons why it cannot be that.
regards, tom lane
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 04:33 Ashutosh Bapat <[email protected]>
parent: Andrey Borodin <[email protected]>
1 sibling, 1 reply; 13+ messages in thread
From: Ashutosh Bapat @ 2025-04-16 04:33 UTC (permalink / raw)
To: Andrey Borodin <[email protected]>; +Cc: Tom Lane <[email protected]>; Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; [email protected]
On Wed, Apr 16, 2025 at 9:37 AM Andrey Borodin <[email protected]> wrote:
>
> My view is what Konstantin wants is automatic replication topology management. For some reason this technology is called HA, DCS, Raft, Paxos and many other scary words. But basically it manages primary_conn_info of some nodes to provide some fault-tolerance properties. I'd start to design from here, not from Raft paper.
>
In my experience, the load of managing hundreds of replicas which all
participate in RAFT protocol becomes more than regular transaction
load. So making every replica a RAFT participant will affect the
ability to deploy hundreds of replica. We may build an extension which
has a similar role in PostgreSQL world as zookeeper in Hadoop. It can
be then used for other distributed systems as well - like shared
nothing clusters based on FDW. There's already a proposal to bring
CREATE SERVER to the world of logical replication - so I see these two
worlds uniting in future. The way I imagine it is some PostgreSQL
instances, which have this extension installed, will act as a RAFT
cluster (similar to Zookeeper ensemble or etcd cluster). The
distributed system based on logical replication or FDW or both will
use this ensemble to manage its shared state. The same ensemble can be
shared across multiple distributed clusters if it has scaling
capabilities.
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 05:24 Andrey Borodin <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 4 replies; 13+ messages in thread
From: Andrey Borodin @ 2025-04-16 05:24 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]>
> On 16 Apr 2025, at 09:26, Tom Lane <[email protected]> wrote:
>
> Andrey Borodin <[email protected]> writes:
>> I think it's what Konstantin is proposing. To have our own Raft implementation, without dependencies.
>
> Hmm, OK. I thought that the proposal involved relying on some existing
> code, but re-reading the thread that was said nowhere. Still, that
> moves it from a large project to a really large project :-(
>
> I continue to think that it'd be best to try to implement it as
> an extension, at least up till the point of finding show-stopping
> reasons why it cannot be that.
I think I can provide some reasons why it cannot be neither extension, nor any part running within postmaster reign.
1. When joining cluster, there’s not PGDATA to run postmaster on top of it.
2. After failover, old Primary node must rejoin cluster by running pg_rewind and following timeline switch.
The system in hand must be able to manipulate with PGDATA without starting Postgres.
My question to Konstantin is Why wouldn’t you just add Raft to Patroni? Is there a reason why something like Patroni is not in core and noone rushes to get it in?
Everyone is using it, or system like it.
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 05:39 Kirill Reshke <[email protected]>
parent: Andrey Borodin <[email protected]>
3 siblings, 1 reply; 13+ messages in thread
From: Kirill Reshke @ 2025-04-16 05:39 UTC (permalink / raw)
To: Andrey Borodin <[email protected]>; +Cc: Tom Lane <[email protected]>; Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]>
On Wed, 16 Apr 2025 at 10:25, Andrey Borodin <[email protected]> wrote:
>
> I think I can provide some reasons why it cannot be neither extension, nor any part running within postmaster reign.
>
> 1. When joining cluster, there’s not PGDATA to run postmaster on top of it.
You can join the cluster on pg_basebackup of its master; So I dont get
why this is an anti-extension restriction.
> 2. After failover, old Primary node must rejoin cluster by running pg_rewind and following timeline switch.
You can run bash from extension, what's the point?
> The system in hand must be able to manipulate with PGDATA without starting Postgres.
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 05:44 Andrey Borodin <[email protected]>
parent: Kirill Reshke <[email protected]>
0 siblings, 1 reply; 13+ messages in thread
From: Andrey Borodin @ 2025-04-16 05:44 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: Tom Lane <[email protected]>; Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]>
> On 16 Apr 2025, at 10:39, Kirill Reshke <[email protected]> wrote:
>
> You can run bash from extension, what's the point?
You cannot run bash that will stop backend running bash.
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 07:50 Michael Banck <[email protected]>
parent: Andrey Borodin <[email protected]>
3 siblings, 0 replies; 13+ messages in thread
From: Michael Banck @ 2025-04-16 07:50 UTC (permalink / raw)
To: Andrey Borodin <[email protected]>; +Cc: Tom Lane <[email protected]>; Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Wed, Apr 16, 2025 at 10:24:48AM +0500, Andrey Borodin wrote:
> I think I can provide some reasons why it cannot be neither extension,
> nor any part running within postmaster reign.
>
> 1. When joining cluster, there’s not PGDATA to run postmaster on top
> of it.
>
> 2. After failover, old Primary node must rejoin cluster by running
> pg_rewind and following timeline switch.
>
> The system in hand must be able to manipulate with PGDATA without
> starting Postgres.
Yeah, while you could maybe implement some/all of the RAFT protocol in
an extension, actually building something useful on top with regards to
high availability or distributed whatever does not look feasible.
> My question to Konstantin is Why wouldn’t you just add Raft to
> Patroni?
Patroni can use pysyncobj, which is a Python implementation of RAFT, so
then you do not need an external RAFT provider like etcd, consul or
zookeeper. However, it is deemed deprecated by the Patroni authors due
to being difficult to debug when it breaks.
I guess a better Python implementation of RAFT for Patroni to use or
Patroni to implement it itself would help, but I believe nobody is
working on the latter right now, nor has any plans to do so. And there
also does not seem to be anybody working on a better pysyncobj.
> Is there a reason why something like Patroni is not in core and noone
> rushes to get it in? Everyone is using it, or system like it.
Well, Patroni is written in Python, for starters. It also does a lot
more than just leader election / cluster config. So I think nobody
seriously thought about proposing to put Patroni into core so far.
I guess the current proposal tries to be a step into the "something like
Patroni in core" if you tilt your head a little. It's just that the
whole thing would be a really big step for Postgres, maybe similar to
deciding we want in-core replication way back when.
Michael
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 09:53 Konstantin Osipov <[email protected]>
parent: Ashutosh Bapat <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Konstantin Osipov @ 2025-04-16 09:53 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; +Cc: Andrey Borodin <[email protected]>; Tom Lane <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; [email protected]
* Ashutosh Bapat <[email protected]> [25/04/16 11:06]:
> > My view is what Konstantin wants is automatic replication topology management. For some reason this technology is called HA, DCS, Raft, Paxos and many other scary words. But basically it manages primary_conn_info of some nodes to provide some fault-tolerance properties. I'd start to design from here, not from Raft paper.
> >
> In my experience, the load of managing hundreds of replicas which all
> participate in RAFT protocol becomes more than regular transaction
> load. So making every replica a RAFT participant will affect the
> ability to deploy hundreds of replica.
I think this experience needs to be detailed out. There are
implementations in the field that are less efficient than others.
Early etcd-raft didn't have pre-voting and had "bastardized"
(their own definition) implementation of configuration changes
which didn't use joint consensus.
Then there is a liveness issue if leader election is implemented
in a straightforward way in large clusters. But this is addressed:
scaling up the randomized election timeout with the cluster size,
converting most of participants to non-voters in large clusters.
Raft replication, again, if implemented in a naive way, would
require a O(outstanding transaction) * number of replicas amount of
RAM. But that doesn't have to be naive.
To sum up, I am not aware of any principal limitations in this
area.
--
Konstantin Osipov, Moscow, Russia
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 09:58 Konstantin Osipov <[email protected]>
parent: Andrey Borodin <[email protected]>
3 siblings, 0 replies; 13+ messages in thread
From: Konstantin Osipov @ 2025-04-16 09:58 UTC (permalink / raw)
To: Andrey Borodin <[email protected]>; +Cc: Tom Lane <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]>
* Andrey Borodin <[email protected]> [25/04/16 11:06]:
> > Andrey Borodin <[email protected]> writes:
> >> I think it's what Konstantin is proposing. To have our own Raft implementation, without dependencies.
> >
> > Hmm, OK. I thought that the proposal involved relying on some existing
> > code, but re-reading the thread that was said nowhere. Still, that
> > moves it from a large project to a really large project :-(
> >
> > I continue to think that it'd be best to try to implement it as
> > an extension, at least up till the point of finding show-stopping
> > reasons why it cannot be that.
>
> I think I can provide some reasons why it cannot be neither extension, nor any part running within postmaster reign.
>
> 1. When joining cluster, there’s not PGDATA to run postmaster on top of it.
>
> 2. After failover, old Primary node must rejoin cluster by running pg_rewind and following timeline switch.
>
> The system in hand must be able to manipulate with PGDATA without starting Postgres.
>
> My question to Konstantin is Why wouldn’t you just add Raft to Patroni? Is there a reason why something like Patroni is not in core and noone rushes to get it in?
> Everyone is using it, or system like it.
Raft uses the same WAL to store configuration change records as is used
for commit records. This is at the core of the correctness of the
algorithm. This is also my biggest concern with correctness of
Patroni - but to the best of my knowledge 's 90%+ of
use cases of Patroni use a "fixed" quorum size, that's defined at
start of the deployment and never/rarely changes.
Contrast to that being able to a replica to the quorum at any
time, and all it takes is just starting this replica and pointing
it at the existing cluster. This greatly simplifies UX.
--
Konstantin Osipov, Moscow, Russia
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 10:02 Konstantin Osipov <[email protected]>
parent: Andrey Borodin <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Konstantin Osipov @ 2025-04-16 10:02 UTC (permalink / raw)
To: Andrey Borodin <[email protected]>; +Cc: Kirill Reshke <[email protected]>; Tom Lane <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]>
* Andrey Borodin <[email protected]> [25/04/16 11:06]:
> > You can run bash from extension, what's the point?
>
> You cannot run bash that will stop backend running bash.
You're right there is a chicken and egg problem when you add
Raft to an existing project, and rebootstrap
becomes a trick, but it's a plumbing trick.
The new member needs to generate and persist a globally unique
identifier as the first step. Later it can
reintroduce itself to the cluster given this identifier
can be preserved in the new incarnation (popen + fork).
--
Konstantin Osipov, Moscow, Russia
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 14:29 Yura Sokolov <[email protected]>
parent: Andrey Borodin <[email protected]>
3 siblings, 0 replies; 13+ messages in thread
From: Yura Sokolov @ 2025-04-16 14:29 UTC (permalink / raw)
To: Andrey Borodin <[email protected]>; Tom Lane <[email protected]>; +Cc: Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; PostgreSQL Hackers <[email protected]>
16.04.2025 08:24, Andrey Borodin пишет:
> 2. After failover, old Primary node must rejoin cluster by running pg_rewind and following timeline switch.
It is really do-able: BiHA already does it. And BiHA runs as a child
process of postmaster, ie both postmaster and BiHA doesn't restart when
PostgreSQL needs to rewind and restart.
Yes, there are non-trivial amount of changes made into postmaster
machinery. But it is doable.
--
regards
Yura Sokolov aka funny-falcon
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: Built-in Raft replication
@ 2025-04-16 21:24 Hannu Krosing <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 0 replies; 13+ messages in thread
From: Hannu Krosing @ 2025-04-16 21:24 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andrey Borodin <[email protected]>; Konstantin Osipov <[email protected]>; Greg Sabino Mullane <[email protected]>; Nikolay Samokhvalov <[email protected]>; [email protected]
On Wed, Apr 16, 2025 at 6:27 AM Tom Lane <[email protected]> wrote:
>
> Andrey Borodin <[email protected]> writes:
> > I think it's what Konstantin is proposing. To have our own Raft implementation, without dependencies.
>
> Hmm, OK. I thought that the proposal involved relying on some existing
> code, but re-reading the thread that was said nowhere. Still, that
> moves it from a large project to a really large project :-(
My understanding is that RAFT is a fancy name for what PostgreSQL is
largely already doing - "electing" a leader and then doing all the
changes through that leader (a.k.a. WAL streaming)
The thing that needs adding - and which makes it "RAFT" instead of
just a streaming replication with a failover - is what happens when
the leader goes away and one of the replicas needs to become a new
leader.
We have ways to do rollbacks and roll-forwards but the main tricky
part is "how do you know that you have not lost some changes" and here
we must have some place to store the info about at which LSN the
failover happened, so that we know to run pg_rewind if any losts hosts
come back and want to join.
And of course we need to have a way to communicate this "who is the
new leader" to clients which needs new libpgq functionality of
"failover connection pools" which hide the failovers from old clients.
The RAFT protocol could be a provider of "who is current leader info"
and optionally cache the LSN the switch happened.
> I continue to think that it'd be best to try to implement it as
> an extension, at least up till the point of finding show-stopping
> reasons why it cannot be that.
The main thing I would like to see in core is ability to do clean
*switchovers* (not failovers) by sending a magic WAL record with a
message "hey node N, please take over the write node role" over WAL so
that node N knows to self-promote and all other nodes know to start
following N starting from the next WAL record either directly or
Why WAL - because it is already is sent to all replicas, it guarantees
continuity as it very clearly states at what LSN the write-head-switch
happens.
We also should be able to send this info to the client libraries
currently connected to the writer. so that they can choose to switch
to the new head.
The rest could be easily an extension.
Mainly we want more than one "coordinators" which can be running in
some or all of the nodes.and who agree on
- which node is current leader
- at which LSN the switch happened (so if some node coming back
discovers that it has magicall moved ahead it knows to rewind to that
LSN and then re-stream it from commonly agreed place.
It would also be nice to have some agreed, hopefully lightweight,
notion of node identity, which we could then use for many things,
including stamping it in WAL records to guarantee / verify that all
the nodes have been on the same WAL stream all the time
But regarding weather to use RAFT I would just define a "coordinator
API" and leave it up to the specific coordinator/consensus extension
to decide how the consensus is achieved
So to summarize:
# Core should provide
- way tomove to new node,
- for switchover a WAL-based switchover
- for failover something similar which also writes the WAL record so
all histories are synced
- a libpq message informing clients about "new write head node"
- node IDs and more general c;luster-awareness inside the PostgreSQL
node (I had a shoutout about this in a recent pgconf.dev unconference
talk)
- a new write-node field in WAL to track write head movement
- API for a joining node to find out which cluster it joins and the
switchover history
- in WAL it is always switchover, maybe with some info saying that
it was a forces switchover because we lost old write head
- if some lost node comes back it may need to rewind or
re-initialize if it finds out it had been following a lost timeline
that is not fully part of
NOTE: switchovers in WAL would be very similar to timeline changes. I
am not sure how much extra info is needed there.
# Extension can provide
- agreeing on new leader node in case of failover
- protocol can be RAFT, PAXOS or "the DBA says so" :)
- sharing fresh info about current leader and switch timelines (though
this should more likely be in core)
- anything else ???
# external apps is (likely?) needed for
- setting up cluster, provisioning machines / VMs
- setting up networking
- starting PostgreSQL servers.
- spinning up and down clients,
- communicating current leader and replica set (could be done by DNS
with agreed conventions)
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2025-04-16 21:24 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-12 13:07 [PATCH 2/8] Pass all scan keys to BRIN consistent function at once Tomas Vondra <[email protected]>
2025-04-16 04:07 Re: Built-in Raft replication Andrey Borodin <[email protected]>
2025-04-16 04:26 ` Re: Built-in Raft replication Tom Lane <[email protected]>
2025-04-16 05:24 ` Re: Built-in Raft replication Andrey Borodin <[email protected]>
2025-04-16 05:39 ` Re: Built-in Raft replication Kirill Reshke <[email protected]>
2025-04-16 05:44 ` Re: Built-in Raft replication Andrey Borodin <[email protected]>
2025-04-16 10:02 ` Re: Built-in Raft replication Konstantin Osipov <[email protected]>
2025-04-16 07:50 ` Re: Built-in Raft replication Michael Banck <[email protected]>
2025-04-16 09:58 ` Re: Built-in Raft replication Konstantin Osipov <[email protected]>
2025-04-16 14:29 ` Re: Built-in Raft replication Yura Sokolov <[email protected]>
2025-04-16 21:24 ` Re: Built-in Raft replication Hannu Krosing <[email protected]>
2025-04-16 04:33 ` Re: Built-in Raft replication Ashutosh Bapat <[email protected]>
2025-04-16 09:53 ` Re: Built-in Raft replication Konstantin Osipov <[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