Re: row filtering for logical replication

Stephen Frost <sfrost@snowman.net>

From: Stephen Frost <sfrost@snowman.net>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Fabrízio de Royes Mello <fabriziomello@gmail.com>, petr.jelinek@2ndquadrant.com, Euler Taveira de Oliveira <euler@timbira.com.br>, Pgsql Hackers <pgsql-hackers@postgresql.org>, hironobu@interdb.jp
Date: 2018-12-14T15:56:45Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Release cache tuple when no longer needed

  2. Add some additional tests for row filters in logical replication.

  3. Fix one of the tests introduced in commit 52e4f0cd47.

  4. Allow specifying row filters for logical replication of tables.

  5. Move scanint8() to numutils.c

  6. Replace Test::More plans with done_testing

  7. Reduce relcache access in WAL sender streaming logical changes

  8. Small cleanups related to PUBLICATION framework code

  9. Add a view to show the stats of subscription workers.

  10. Allow publishing the tables of schema.

  11. Doc: improve documentation of CREATE/ALTER SUBSCRIPTION.

  12. Add PublicationTable and PublicationRelInfo structs

  13. Remove unused argument "txn" in maybe_send_schema().

  14. Add prepare API support for streaming transactions in logical replication.

  15. Unify PostgresNode's new() and get_new_node() methods

  16. Use l*_node() family of functions where appropriate

  17. Add support for prepared transactions to built-in logical replication.

  18. Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.

  19. Rename a parse node to be more general

  20. Remove unused column atttypmod from initial tablesync query

  21. SEARCH and CYCLE clauses

Greetings,

* Tomas Vondra (tomas.vondra@2ndquadrant.com) wrote:
> On 11/23/18 8:03 PM, Stephen Frost wrote:
> > * Fabrízio de Royes Mello (fabriziomello@gmail.com) wrote:
> >> On Fri, Nov 23, 2018 at 4:13 PM Petr Jelinek <petr.jelinek@2ndquadrant.com>
> >> wrote:
> >>>> If carefully documented I see no problem with it... we already have an
> >>>> analogous problem with functional indexes.
> >>>
> >>> The difference is that with functional indexes you can recreate the
> >>> missing object and everything is okay again. With logical replication
> >>> recreating the object will not help.
> >>>
> >>
> >> In this case with logical replication you should rsync the object. That is
> >> the price of misunderstanding / bad use of the new feature.
> >>
> >> As usual, there are no free beer ;-)
> > 
> > There's also certainly no shortage of other ways to break logical
> > replication, including ways that would also be hard to recover from
> > today other than doing a full resync.
> 
> Sure, but that seems more like an argument against creating additional
> ones (and for preventing those that already exist). I'm not sure this
> particular feature is where we should draw the line, though.

I was actually going in the other direction- we should allow it because
advanced users may know what they're doing better than we do and we
shouldn't prevent things just because they might be misused or
misunderstood by a user.

> > What that seems to indicate, to me at least, is that it'd be awful
> > nice to have a way to resync the data which doesn't necessairly
> > involve transferring all of it over again.
> > 
> > Of course, it'd be nice if we could track those dependencies too,
> > but that's yet another thing.
> 
> Yep, that seems like a good idea in general. Both here and for
> functional indexes (although I suppose sure is a technical reason why it
> wasn't implemented right away for them).

We don't track function dependencies in general and I could certainly
see cases where you really wouldn't want to do so, at least not in the
same way that we track FKs or similar.  I do wonder if maybe we didn't
track function dependencies because we didn't (yet) have create or
replace function and that now we should.  We don't track dependencies
inside a function either though.

> > In short, I'm not sure that I agree with the idea that we shouldn't
> > allow this and instead I'd rather we realize it and put the logical
> > replication into some kind of an error state that requires a resync.
> 
> That would still mean a need to resync the data to recover, so I'm not
> sure it's really an improvement. And I suppose it'd require tracking the
> dependencies, because how else would you mark the subscription as
> requiring a resync? At which point we could decline the DROP without a
> CASCADE, just like we do elsewhere, no?

I was actually thinking more along the lines of just simply marking the
publication/subscription as being in a 'failed' state when a failure
actually happens, and maybe even at that point basically throwing away
everything except the shell of the publication/subscription (so the user
can see that it failed and come in and properly drop it); I'm thinking
about this as perhaps similar to a transaction being aborted.

Thanks!

Stephen