Re: some review comments on logical rep code
Petr Jelinek <petr.jelinek@2ndquadrant.com>
From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
To: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: masao.fujii@gmail.com, sawada.mshk@gmail.com, pgsql-hackers@postgresql.org
Date: 2017-04-19T08:59:00Z
Lists: pgsql-hackers
On 19/04/17 10:45, Kyotaro HORIGUCHI wrote: > At Wed, 19 Apr 2017 17:43:17 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in <20170419.174317.114509231.horiguchi.kyotaro@lab.ntt.co.jp> >> At Wed, 19 Apr 2017 10:33:29 +0200, Petr Jelinek <petr.jelinek@2ndquadrant.com> wrote in <ed73a706-9e15-f137-2d55-f05361f2de9a@2ndquadrant.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
Commits
-
Fix bug so logical rep launcher saves correctly time of last startup of worker.
- 9f11fcec6624 10.0 landed
-
Prevent log_replication_commands from causing SQL commands to be logged.
- 3a66581dd12b 10.0 landed
-
Mark some columns in pg_subscription as NOT NULL.
- 88b0a3192633 10.0 landed
-
Don't call the function that may raise an error while holding spinlock.
- 8bbc618b4825 10.0 landed
-
A collection of small fixes for logical replication.
- 280c53ecfbca 10.0 landed
-
Use DatumGetInt32() to extract 32-bit integer value from a datum.
- 39a6772d0494 10.0 landed