Thread

Commits

  1. Fix logical replication slot initialization

  2. Ability to advance replication slots

  1. patch to ensure logical decoding errors early

    Dave Cramer <davecramer@gmail.com> — 2018-07-31T18:51:12Z

    This patch does 2 things
    
    1) Ensure that when the slot is created
    with pg_create_physical_replication_slot if the output plugin does not
    exist it will error.
    
    2) Currently when the decoding context is created and the output plugin
    does not exist the protocol will respond with CopyDone. This patch will
    return an error instead and abort the copy connection.
    
    
    
    Dave Cramer
    
  2. Re: patch to ensure logical decoding errors early

    Andres Freund <andres@anarazel.de> — 2018-07-31T18:58:13Z

    Hi,
    
    On 2018-07-31 14:51:12 -0400, Dave Cramer wrote:
    > This patch does 2 things
    > 
    > 1) Ensure that when the slot is created
    > with pg_create_physical_replication_slot if the output plugin does not
    > exist it will error.
    
    *logical, I assume?
    
    
    > diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
    > index 3cd4eef..9f883b9 100644
    > --- a/src/backend/replication/logical/logical.c
    > +++ b/src/backend/replication/logical/logical.c
    > @@ -143,8 +143,7 @@ StartupDecodingContext(List *output_plugin_options,
    >  	 * (re-)load output plugins, so we detect a bad (removed) output plugin
    >  	 * now.
    >  	 */
    > -	if (!fast_forward)
    > -		LoadOutputPlugin(&ctx->callbacks, NameStr(slot->data.plugin));
    > +	LoadOutputPlugin(&ctx->callbacks, NameStr(slot->data.plugin));
    
    So this actually was broken by 9c7d06d60680c7f00d931233873dee81fdb311c6
    and worked before? Petr, Simon?  Isn't the actual bug here that
    CreateInitDecodingContext() passes true for fast_forward?  Dave, could
    you confirm this is the case?  If so, this'll end up actually being an
    open items entry...
    
    
    >  	/*
    >  	 * Now that the slot's xmin has been set, we can announce ourselves as a
    > @@ -312,7 +311,7 @@ CreateInitDecodingContext(char *plugin,
    >  	ReplicationSlotSave();
    >  
    >  	ctx = StartupDecodingContext(NIL, InvalidXLogRecPtr, xmin_horizon,
    > -								 need_full_snapshot, true,
    > +								 need_full_snapshot, true, 
    >  								 read_page, prepare_write, do_write,
    >  								 update_progress);
    
    Huh?
    
    
    Greetings,
    
    Andres Freund
    
    
    
  3. Re: patch to ensure logical decoding errors early

    Dave Cramer <davecramer@gmail.com> — 2018-08-01T14:08:38Z

    On 31 July 2018 at 14:58, Andres Freund <andres@anarazel.de> wrote:
    
    > Hi,
    >
    > On 2018-07-31 14:51:12 -0400, Dave Cramer wrote:
    > > This patch does 2 things
    > >
    > > 1) Ensure that when the slot is created
    > > with pg_create_physical_replication_slot if the output plugin does not
    > > exist it will error.
    >
    > *logical, I assume?
    >
    > Yes, logical.
    
    >
    > > diff --git a/src/backend/replication/logical/logical.c
    > b/src/backend/replication/logical/logical.c
    > > index 3cd4eef..9f883b9 100644
    > > --- a/src/backend/replication/logical/logical.c
    > > +++ b/src/backend/replication/logical/logical.c
    > > @@ -143,8 +143,7 @@ StartupDecodingContext(List *output_plugin_options,
    > >        * (re-)load output plugins, so we detect a bad (removed) output
    > plugin
    > >        * now.
    > >        */
    > > -     if (!fast_forward)
    > > -             LoadOutputPlugin(&ctx->callbacks,
    > NameStr(slot->data.plugin));
    > > +     LoadOutputPlugin(&ctx->callbacks, NameStr(slot->data.plugin));
    >
    > So this actually was broken by 9c7d06d60680c7f00d931233873dee81fdb311c6
    > and worked before? Petr, Simon?  Isn't the actual bug here that
    > CreateInitDecodingContext() passes true for fast_forward?  Dave, could
    > you confirm this is the case?  If so, this'll end up actually being an
    > open items entry...
    >
    
    Ya, I think that is really the issue. I will redo my patch
    
    >
    >
    > >       /*
    > >        * Now that the slot's xmin has been set, we can announce
    > ourselves as a
    > > @@ -312,7 +311,7 @@ CreateInitDecodingContext(char *plugin,
    > >       ReplicationSlotSave();
    > >
    > >       ctx = StartupDecodingContext(NIL, InvalidXLogRecPtr, xmin_horizon,
    > > -
    > need_full_snapshot, true,
    > > +
    > need_full_snapshot, true,
    > >
    > read_page, prepare_write, do_write,
    > >
    > update_progress);
    >
    
    Hmmm, I guess I should read my patches more carefully before sending them
    
    > Huh?
    >
    >
    >
    
    
    Dave Cramer
    
  4. Re: patch to ensure logical decoding errors early

    Petr Jelinek <petr.jelinek@2ndquadrant.com> — 2018-08-01T14:13:39Z

    Hi,
    
    On 31/07/18 20:58, Andres Freund wrote>
    >> diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
    >> index 3cd4eef..9f883b9 100644
    >> --- a/src/backend/replication/logical/logical.c
    >> +++ b/src/backend/replication/logical/logical.c
    >> @@ -143,8 +143,7 @@ StartupDecodingContext(List *output_plugin_options,
    >>  	 * (re-)load output plugins, so we detect a bad (removed) output plugin
    >>  	 * now.
    >>  	 */
    >> -	if (!fast_forward)
    >> -		LoadOutputPlugin(&ctx->callbacks, NameStr(slot->data.plugin));
    >> +	LoadOutputPlugin(&ctx->callbacks, NameStr(slot->data.plugin));
    > 
    > So this actually was broken by 9c7d06d60680c7f00d931233873dee81fdb311c6
    > and worked before? Petr, Simon?  Isn't the actual bug here that
    > CreateInitDecodingContext() passes true for fast_forward?  Dave, could
    > you confirm this is the case?  If so, this'll end up actually being an
    > open items entry...
    > 
    
    Indeed.
    
    -- 
      Petr Jelinek                  http://www.2ndQuadrant.com/
      PostgreSQL Development, 24x7 Support, Training & Services
    
    
    
  5. Re: patch to ensure logical decoding errors early

    Dave Cramer <davecramer@gmail.com> — 2018-08-01T15:07:24Z

    On 1 August 2018 at 10:13, Petr Jelinek <petr.jelinek@2ndquadrant.com>
    wrote:
    
    > Hi,
    >
    > On 31/07/18 20:58, Andres Freund wrote>
    > >> diff --git a/src/backend/replication/logical/logical.c
    > b/src/backend/replication/logical/logical.c
    > >> index 3cd4eef..9f883b9 100644
    > >> --- a/src/backend/replication/logical/logical.c
    > >> +++ b/src/backend/replication/logical/logical.c
    > >> @@ -143,8 +143,7 @@ StartupDecodingContext(List *output_plugin_options,
    > >>       * (re-)load output plugins, so we detect a bad (removed) output
    > plugin
    > >>       * now.
    > >>       */
    > >> -    if (!fast_forward)
    > >> -            LoadOutputPlugin(&ctx->callbacks,
    > NameStr(slot->data.plugin));
    > >> +    LoadOutputPlugin(&ctx->callbacks, NameStr(slot->data.plugin));
    > >
    > > So this actually was broken by 9c7d06d60680c7f00d931233873dee81fdb311c6
    > > and worked before? Petr, Simon?  Isn't the actual bug here that
    > > CreateInitDecodingContext() passes true for fast_forward?  Dave, could
    > > you confirm this is the case?  If so, this'll end up actually being an
    > > open items entry...
    > >
    >
    > Indeed.
    >
    
    See attached patch which fixes it, and adds a test for it.
    
  6. Re: patch to ensure logical decoding errors early

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-08-01T22:11:31Z

    On 2018-Aug-01, Dave Cramer wrote:
    
    > See attached patch which fixes it, and adds a test for it.
    
    Pushed, thanks.
    
    -- 
    Álvaro Herrera                https://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
    
    
    
  7. Re: patch to ensure logical decoding errors early

    Simon Riggs <simon@2ndquadrant.com> — 2018-08-02T08:05:06Z

    On 1 August 2018 at 23:11, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
    > On 2018-Aug-01, Dave Cramer wrote:
    >
    >> See attached patch which fixes it, and adds a test for it.
    >
    > Pushed, thanks.
    
    Thanks
    
    -- 
    Simon Riggs                http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services