On 5 December 2017 at 16:00, Nikhil Sontakke <nikh...@2ndquadrant.com> wrote:
> > We disallow rewrites on user_catalog_tables, so they cannot change > underneath. Yes, DML can be carried out on them inside a 2PC > transaction which then gets ROLLBACK'ed. But if it's getting aborted, > then we are not interested in that data anyways. Also, now that we > have the "filter_decode_txn_cb_wrapper()" function, we will stop > decoding by the next change record cycle because of the abort. > > So, I am not sure if we need to track user_catalog_tables in > HeapTupleSatisfiesVacuum explicitly. > I guess it's down to whether, when we're decoding a txn that just got concurrently aborted, the output plugin might do anything with its user catalogs that could cause a crash. Output plugins are most likely to be using the genam (or even SPI, I guess?) to read user-catalogs during logical decoding. Logical decoding its self does not rely on the correctness of user catalogs in any way, it's only a concern for output plugin callbacks. It may make sense to kick this one down the road at this point, I can't conclusively see where it'd cause an actual problem. > > > Otherwise I'm really, really happy with how this is progressing and want > to > > find time to play with it. > > Yeah, I will do some more testing and add a few more test cases in the > test_decoding plugin. It might be handy to have a DELAY of a few > seconds after every change record processing, for example. That ways, > we can have a TAP test which can do a few WAL activities and then we > introduce a concurrent rollback midways from another session in the > middle of that delayed processing. I have done debugger based testing > of this concurrent rollback functionality as of now. > > Sounds good. > Another test (actually, functionality) that might come in handy, is to > have a way for DDL to be actually carried out on the subscriber. We > will need something like pglogical.replicate_ddl_command to be added > to the core for this to work. We can add this functionality as a > follow-on separate patch after discussing how we want to implement > that in core. Yeah, definitely a different patch, but assuredly valuable. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
