Re: Problem with synchronous replication

------------------------------------------------------------------

From:LIU Dongming <lingce.ldm@alibaba-inc.com>

Sent At:2019 Oct. 25 (Fri.) 15:18

To:pgsql-hackers <pgsql-hackers@postgresql.org>

Subject:Problem with synchronous replication

Hi,

I recently discovered two possible bugs about synchronous replication.

1. SyncRepCleanupAtProcExit may delete an element that has been deleted

SyncRepCleanupAtProcExit first checks whether the queue is detached, if it is not detached, 

acquires the SyncRepLock lock and deletes it. If this element has been deleted by walsender, 

it will be deleted repeatedly, SHMQueueDelete will core with a segment fault. 

IMO, like SyncRepCancelWait, we should lock the SyncRepLock first and then check

whether the queue is detached or not.

2. SyncRepWaitForLSN may not call SyncRepCancelWait if ereport check one interrupt.

For SyncRepWaitForLSN, if a query cancel interrupt arrives, we just terminate the wait 

with suitable warning. As follows:

a. set QueryCancelPending to false

b. errport outputs one warning

c. calls SyncRepCancelWait to delete one element from the queue

If another cancel interrupt arrives when we are outputting warning at step b, the errfinish

will call CHECK_FOR_INTERRUPTS that will output an ERROR, such as "canceling autovacuum

task", then the process will jump to the sigsetjmp. Unfortunately, the step c will be skipped

and the element that should be deleted by SyncRepCancelWait is remained.

The easiest way to fix this is to swap the order of step b and step c. On the other hand, 

let sigsetjmp clean up the queue may also be a good choice. What do you think?

Attached the patch, any feedback is greatly appreciated.

Best regards,

--

Dongming Liu