RE: Slow catchup of 2PC (twophase) transactions on replica in LR

[Vitaly Davydov]

Hi Kuroda-san,

Thank you very much for the patch. In general, it seem to work well for me, but 
there seems to be a memory access problem in libpqrcv_alter_slot -> 
quote_identifier in case of NULL slot_name. It happens, if the two_phase option 
is altered on a subscription without slot. I think, a simple check for NULL may 
fix the problem. I guess, the same problem may be for failover option.

Another possible problem is related to my use case. I haven't reproduced this 
case, just some thoughts. I guess, when two_phase is ON, the PREPARE statement 
may be truncated from the WAL at checkpoint, but COMMIT PREPARED is still kept 
in the WAL. On catchup, I would ask the master to send transactions from some 
restart LSN. I would like to get all such transactions competely, with theirs 
bodies, not only COMMIT PREPARED messages. One of the solutions is to have an 
option for the slot to keep the WAL like with two_phase = OFF independently on 
its two_phase option. It is just an idea.

With best regards,
Vitaly

RE: Slow catchup of 2PC (twophase) transactions on replica in LR

[Vitaly Davydov]

Dear Hayato,

On Monday, April 22, 2024 15:54 MSK, "Hayato Kuroda (Fujitsu)" 
 wrote:
 > Dear Vitaly,
>
> > I looked at the patch and realized that I can't try it easily in the near 
> > future
> > because the solution I'm working on is based on PG16 or earlier. This patch 
> > is
> > not easily applicable to the older releases. I have to port my solution to 
> > the
> > master, which is not done yet.
>
> We also tried to port our patch for PG16, but the largest barrier was that a
> replication command ALTER_SLOT is not supported. Since the slot option
> two_phase
> can't be modified, it is difficult to skip decoding PREPARE command even when
> altering the option from true to false.
> IIUC, Adding a new feature (e.g., replication command) for minor updates is
> generally
> prohibited
>
...

Attached patch set is a ported version for PG16, which breaks ABI. This can be 
used
for testing purpose, but it won't be pushed to REL_16_STABLE.
At least, this patchset can pass my github CI.

Can you apply and check whether your issue is solved?​​It is fantastic. 
Thank you for your help! I will definitely try your patch. I need some time to 
test and incorporate it. I also plan to port my stuff to the master branch to 
simplify testing of patches.

With best regards,
​Vitaly Davydov

 

Re: How to accurately determine when a relation should use local buffers?

[Vitaly Davydov]

Hi Aleksander,

Thank you for the reply.

> Could you please provide a specific example when the current code willdo
> something wrong/unintended?

I can't say that something is wrong in vanilla. But if you decide to
replicate DDL in some solutions like multimaster, you might want to
replicate CREATE TEMPORARY TABLE. Furthermore, there is some possible
inconsistency in the code show below (REL_16_STABLE) in bufmgr.c file:

   - FlushRelationBuffers, PrefetchBuffer uses
   RelationUsesLocalBuffers(rel).
   - ExtendBufferedRel_common finally use
   BufferManagerRelation.relpersistence which is actually
   rd_rel->relpersistence, works like RelationUsesLocalBuffers.
   - ReadBuffer_common uses isLocalBuf = SmgrIsTemp(smgr), that checks
   rlocator.backend for InvalidBackendId.

I would like to clarify, do we completely refuse the use of temporary
tables in other contexts than in backends or there is some work-in-progress
to allow some other usage contexts? If so, the check of
rd_rel->relpersistence is enough. Not sure why we use SmgrIsTemp instead of
RelationUsesLocalBuffers in ReadBuffer_common.


With best regards,

Vitaly Davydov

вт, 21 нояб. 2023 г. в 11:52, Aleksander Alekseev :

> Hi,
>
> > I would like to clarify, what the correct way is to determine that a
> given relation is using local buffers. Local buffers, as far as I know, are
> used for temporary tables in backends. There are two functions/macros
> (bufmgr.c): SmgrIsTemp, RelationUsesLocalBuffers. The first function
> verifies that the current process is a regular session backend, while the
> other macro verifies the relation persistence characteristic. It seems, the
> use of each function independently is not correct. I think, these functions
> should be applied in pair to check for local buffers use, but, it seems,
> these functions are used independently. It works until temporary tables are
> allowed only in session backends.
>
> Could you please provide a specific example when the current code will
> do something wrong/unintended?
>
> > I'm concerned, how to determine the use of local buffers in some other
> theoretical cases? For example, if we decide to replicate temporary tables?
> Are there the other cases, when local buffers can be used with relations in
> the Vanilla? Do we allow the use of relations with RELPERSISTENCE_TEMP not
> only in session backends?
>
> Temporary tables, by definition, are visible only within one session.
> I can't imagine how and why they would be replicated.
>
> --
> Best regards,
> Aleksander Alekseev
>


-- 
С уважением,
Давыдов Виталий
http://www.vdavydov.ru