On 5 December 2017 at 16:00, Nikhil Sontakke
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