Re: some review comments on logical rep code

On 19/04/17 10:45, Kyotaro HORIGUCHI wrote:
> At Wed, 19 Apr 2017 17:43:17 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20170419(dot)174317(dot)114509231(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
>> At Wed, 19 Apr 2017 10:33:29 +0200, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote in <ed73a706-9e15-f137-2d55-f05361f2de9a(at)2ndquadrant(dot)com>
>>>> Commit has been moved from after to before of the lock section.
>>>> This causes potential race condition. (As the same as the
>>>> potential dead-lock, I'm not sure it can cause realistic problem,
>>>> though..) Isn't it better to be after the lock section?
>>>>
>>>
>>> We just read catalogs so there should not be locking issues.
>>
>> Some other process may modify it then go to there. With a kind of
>> priority inversion, the process may modify the data on the memory
>> *before* we modify it. Of course this is rather unrealistic,
>> though.
>
> A bit short.
>
> Some other process may modify it *after* we read it then go to
> there. With a kind of priority inversion, the process may modify
> the data on the memory *before* we modify it. Of course this is
> rather unrealistic, though.
>

Yeah but I think that's risk anyway in MVCC read committed database, the
only way to protect against that would be to hold lock over the catalogs
access which was what we wanted to get rid of :)

BTW the whole sync coordination is explicitly written in a way that
unless you mess with catalogs manually only the tablesync process should
do UPDATEs to that catalog (with the exception of s->r state switch
which can happen in apply and has no effect on sync because both states
mean that synchronization is done, only makes apply stop tracking the
table individually).

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services