public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Masahiko Sawada <[email protected]>
To: Petr Jelinek <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Rahila Syed <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Column Filtering in Logical Replication
Date: Thu, 14 Apr 2022 09:09:31 +0530
Message-ID: <CAA4eK1LHevgWSsHHD2VHCMNnqqopdyY9fSZFjKPoTwA3vPy81g@mail.gmail.com> (raw)
In-Reply-To: <CAD21AoAS67ppEQ_RPggNBHbfrah2NiO59NPUpomPYRSF_voMJA@mail.gmail.com>
References: <[email protected]>
<OS0PR01MB571614E4A39DD33669D5AFD6940A9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAA4eK1J9UDnM-xN7viu4eO_DDL_PqMG9F6ZsskqEU7snujXi+A@mail.gmail.com>
<[email protected]>
<CAA4eK1KUpOnFam+2bBuGGX-_pZbmO_ShJV6G=GT-RyM7gQSvWg@mail.gmail.com>
<[email protected]>
<OS0PR01MB57164514E1DE1E579B0B6C46940F9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAA4eK1KvFAygPbFu=cUvnqhmTLPpjzqp8p4UNSsLkiZmvFp8oQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAA4eK1LpBFU49Ohbnk=dv_v9YP+Kqh1+Sf8i++_s-QhD1Gy4Qw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAA4eK1JzzoE61CY1qi9Vcdi742JFwG4YA3XpoMHwfKNhbFic6g@mail.gmail.com>
<CAA4eK1JcQRQw0G-U4A+vaGaBWSvggYMMDJH4eDtJ0Yf2eUYXyA@mail.gmail.com>
<CAD21AoAD6qhu1gK=vnzxvNiQpFx1m_wqXOvWqy1g4iNLJWCbnw@mail.gmail.com>
<CAA4eK1J03HJC-6bUsdHnO8O6W89T-CJ1NaqA5HuxTYikwA-HOQ@mail.gmail.com>
<CAD21AoAS67ppEQ_RPggNBHbfrah2NiO59NPUpomPYRSF_voMJA@mail.gmail.com>
On Thu, Apr 14, 2022 at 8:32 AM Masahiko Sawada <[email protected]> wrote:
>
> On Wed, Apr 13, 2022 at 6:45 PM Amit Kapila <[email protected]> wrote:
> >
> > On Wed, Apr 13, 2022 at 1:41 PM Masahiko Sawada <[email protected]> wrote:
> > >
> > > I've looked at this issue and had the same analysis. Also, I could
> > > reproduce this issue with the steps shared by Amit.
> > >
> > > As I mentioned in another thread[1], the fact that the tablesync
> > > worker doesn't check the return value from
> > > wait_for_worker_state_change() seems a bug to me. So my initial
> > > thought of the solution is that we can have the tablesync worker check
> > > the return value and exit if it's false. That way, the apply worker
> > > can restart and request to launch the tablesync worker again. What do
> > > you think?
> > >
> >
> > I think that will fix this symptom but I am not sure if that would be
> > the best way to deal with this because we have a mechanism where the
> > sync worker can continue even if we don't do anything as a result of
> > wait_for_worker_state_change() provided apply worker restarts.
>
> I think we can think this is a separate issue. That is, if tablesync
> worker can start streaming changes even without waiting for the apply
> worker to set SUBREL_STATE_CATCHUP, do we really need the wait? I'm
> not sure it's really safe. If it's safe, the tablesync worker will no
> longer need to wait there.
>
As per my understanding, it is safe, whatever is streamed by tablesync
worker will be skipped later by apply worker. The wait here avoids
streaming the same data both by the apply worker and table sync worker
which I think is good even if it is not a must.
> >
> > The other part of the puzzle is the below check in the code:
> > /*
> > * If we reached the sync worker limit per subscription, just exit
> > * silently as we might get here because of an otherwise harmless race
> > * condition.
> > */
> > if (nsyncworkers >= max_sync_workers_per_subscription)
> >
> > It is not clear to me why this check is there, if this wouldn't be
> > there, the user would have got either a WARNING to increase the
> > max_logical_replication_workers or the apply worker would have been
> > restarted. Do you have any idea about this?
>
> Yeah, I'm also puzzled with this check. It seems that this function
> doesn't work well when the apply worker is not running and some
> tablesync workers are running. I initially thought that the apply
> worker calls to this function as many as tables that needs to be
> synced, but it checks the max_sync_workers_per_subscription limit
> before calling to logicalrep_worker_launch(). So I'm not really sure
> we need this check.
>
I just hope that the original author Petr J. responds to this point. I
have added him to this email. This will help us to find the best
solution for this problem.
Note: I'll be away for the remaining week, so will join the discussion
next week unless we reached the conclusion by that time.
--
With Regards,
Amit Kapila.
view thread (28+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Column Filtering in Logical Replication
In-Reply-To: <CAA4eK1LHevgWSsHHD2VHCMNnqqopdyY9fSZFjKPoTwA3vPy81g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox