Re: Synchronizing slots from primary to standby - Mailing list pgsql-hackers
From | Bharath Rupireddy |
---|---|
Subject | Re: Synchronizing slots from primary to standby |
Date | |
Msg-id | CALj2ACVJv97y-wMYSdfZz-W1uHbSUXXeu_r5m7gj1uSQO5HJHQ@mail.gmail.com Whole thread Raw |
In response to | Re: Synchronizing slots from primary to standby (Amit Kapila <amit.kapila16@gmail.com>) |
Responses |
Re: Synchronizing slots from primary to standby
|
List | pgsql-hackers |
On Wed, Apr 3, 2024 at 9:04 AM Amit Kapila <amit.kapila16@gmail.com> wrote: > > > I'd just rename LogicalSlotAdvanceAndCheckSnapState(XLogRecPtr > > moveto, bool *found_consistent_snapshot) to > > pg_logical_replication_slot_advance(XLogRecPtr moveto, bool > > *found_consistent_snapshot) and use it. If others don't like this, I'd > > at least turn pg_logical_replication_slot_advance(XLogRecPtr moveto) a > > static inline function. > > > Yeah, we can do that but it is not a performance-sensitive routine so > don't know if it is worth it. Okay for what the patch has right now. No more bikeshedding from me on this. > > > The slotsync worker needs to advance the slots from different databases in > > > fast_forward. So, we need to skip this check in fast_forward mode. The analysis can > > > be found in [3]. > > - if (slot->data.database != MyDatabaseId) > > + /* > > + * We need to access the system tables during decoding to build the > > + * logical changes unless we are in fast_forward mode where no changes are > > + * generated. > > + */ > > + if (slot->data.database != MyDatabaseId && !fast_forward) > > ereport(ERROR, > > > > It's not clear from the comment that we need it for a slotsync worker > > to advance the slots from different databases. Can this be put into > > the comment? Also, specify in the comment, why this is safe? > > > It is not specific to slot sync worker but specific to fast_forward > mode. There is already a comment "We need to access the system tables > during decoding to build the logical changes unless we are in > fast_forward mode where no changes are generated." telling why it is > safe. The point is we need database access to access system tables > while generating the logical changes and in fast-forward mode, we > don't generate logical changes so this check is not required. Do let > me if you have a different understanding or if my understanding is > incorrect. Understood. Thanks. Just curious, why isn't a problem for the existing fast_forward mode callers pg_replication_slot_advance and LogicalReplicationSlotHasPendingWal? I quickly looked at v8, and have a nit, rest all looks good. + if (DecodingContextReady(ctx) && found_consistent_snapshot) + *found_consistent_snapshot = true; Can the found_consistent_snapshot be checked first to help avoid the function call DecodingContextReady() for pg_replication_slot_advance callers? -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
pgsql-hackers by date: