On 15 June 2018 at 09:46, Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > On 6/5/18 07:02, Amit Khandekar wrote: >> I haven't written a patch for it, but I think we should have a >> separate on_commit_stop_workers for eachyou get subtransaction. At >> subtransaction commit, we replace the on_commit_stop_workers list of >> the parent subtransaction with the one from the committed >> subtransaction; and on abort, discard the list of the current >> subtransaction. So have a stack of the lists. > > Is there maybe a more general mechanism we could attach this to? Maybe > resource owners?
You mean using something like ResourceOwnerRelease() ? We need to merge the on_commit_stop_workers list into the parent transaction's list. So we can't release the whole list on commit. The way I am implementing this can be seen in attached apply_launcher_subtrans_WIP.patch. (check launcher.c changes). I haven't started testing it yet though. on_commit_stop_workers denotes a list of subscriptions, and each element also contains a list of relations for that subscription. This list is built by ALTER-SUBSCRIPTION commands that have run in the current (sub)transaction. At sub-transaction start, the on_commit_stop_workers is pushed into a stack. Then on_commit_stop_workers starts with empty list. This list is then populated by ALTER-SUBCSRIPTION commands of the current sub-transaction. At sub-transaction commit, this list is merged into the list of parent subtransaction, the parent transaction stack element is popped out, and the merged list becomes the new on_commit_stop_workers. So say, parent has sub1(tab1, tab2), sub2(tab2, tab3), where sub means subscription. and current on_commit_workers has sub2(tab4) and sub3(tab1) At commit, for subscription sub2, we should replace the outer sub2(tab2, tab3) with the newer sub2(tab4). So, the merged list will have : sub1(tab1, tab2), sub2(tab4), sub3(tab1) At sub-transaction abort, the on_commit_stop_workers is discarded. The parent transaction worker list is assigned back to on_commit_stop_workers. > >> Subscription mysub is set to synchronise tables tx1 and tx2 : >> CREATE SUBSCRIPTION mysub ... PUBLICATION pubx; >> >> Now suppose the subscription is altered to synchronise ty1 and ty2, >> and then again altered back to synchronise tx1 and tx2 in the same >> transaction. >> begin; >> ALTER SUBSCRIPTION mysub set publication puby; >> ALTER SUBSCRIPTION mysub set publication pubx; >> commit; >> >> Here, workers for tx1 and tx2 are added to on_commit_stop_workers >> after the publication is set to puby. And then workers for ty1 and ty2 >> are further added to that list after the 2nd ALTER command where >> publication is set to pubx, because the earlier ALTER command has >> already changed the catalogs to denote that ty1 and ty2 are being >> synchronised. Effectively, at commit, all the workers are targetted to >> be stopped, when actually at commit time there is no final change in >> the tables to be synchronised. > > I'm not so bothered about this scenario. When you drop and then > recreate a table in the same transaction, that doesn't mean you keep the > data that was previously in the table. If you want to *undo* a change, > you need to do rollback, not commit further changes that try to recreate > the previous state. May be the example I gave is not practical. But a user can as well do something like : CREATE SUBSCRIPTION mysub ... PUBLICATION pub1; ... begin; ALTER SUBSCRIPTION mysub set publication pub2; .... ALTER SUBSCRIPTION mysub set publication pub3; commit; So pub3 is yet another publication. So here the old one is not set back again. Or may there can be ALTER SUBSCRIPTION REFRESH. I believe on_commit_stop_workers was designed keeping in mind that all actions are to be done only at commit; we should not immediately stop workers. So it implies that the workers it determines in the end of commit, also should be accurate. I have anyways included the changes for this in the same attached patch (changes are in subscriptioncmds.c) -- Thanks, -Amit Khandekar EnterpriseDB Corporation The Postgres Database Company
apply_launcher_subtrans_WIP.patch
Description: Binary data
