[Slony1-patches] Removal of session replication role for Logical Replication compatibility

[Steve Singer]

On Thu, 20 Feb 2020, Richard Yen wrote:

I've CC's slony1-hackers since I don't think many people are subscribed to 
-patches.

I'm inclined to apply this patch. I don't think the value of the extra check 
is worth making like difficult for users that want to run the slony triggers 
on a logical replica.

If someone things that the extra guard in the log trigger should be 
configurable instead of just removed they should speak up.



> 
> 
> On Thu, Feb 20, 2020 at 4:16 AM Steve Singer <steve at ssinger.info> wrote:
>       On Thu, 20 Feb 2020, Richard Yen wrote:
>
>       So with your patch, what happens on node C?
>       Do the replication triggers fire there as well?
> 
> 
> No, on node C, the replication triggers do not fire (i.e., sl_log_{1,2} are empty).  This is because the
> triggers themselves are disabled on the tables (Slony's doing, not mine):
> 
> pgbench=# select tgname, tgenabled  from pg_trigger where tgname like '_slony_example%' order by 2;
>              tgname             | tgenabled
> --------------------------------+-----------
>  _slony_example_truncatetrigger | D
>  _slony_example_logtrigger      | D
>  _slony_example_truncatetrigger | D
>  _slony_example_logtrigger      | D
>  _slony_example_truncatetrigger | D
>  _slony_example_logtrigger      | D
>  _slony_example_truncatedeny    | O
>  _slony_example_truncatedeny    | O
>  _slony_example_denyaccess      | O
>  _slony_example_denyaccess      | O
>  _slony_example_denyaccess      | O
>  _slony_example_truncatedeny    | O
> (12 rows)
> 
> Granted, my patch is like taking the guardrails off, but I'd argue that the ENABLE/DISABLE syntax on PG's
> ALTER TABLE command is quite comprehensive, in terms of letting Slony do what it needs to do to make sure
> the triggers are properly turned on/off, based on the publisher/subscriber status.
> 
> There are other locations where there's this redundancy of 1) setting triggers to fire on replica-only
> and 2) enforcing replica role identity in the C-side of the trigger code (i.e., logApply):
> 
> pgbench=# \d _slony_example.sl_log_1;
>                Table "_slony_example.sl_log_1"
>       Column      |  Type   | Collation | Nullable | Default
> ------------------+---------+-----------+----------+---------
>  log_origin       | integer |           |          |
> ...<snip>...
> Triggers firing on replica only:
>     apply_trigger BEFORE INSERT ON _slony_example.sl_log_1 FOR EACH ROW EXECUTE PROCEDURE
> _slony_example.logapply('_slony_example')
> 
> To avoid inserting more risk, I didn't take those checks out--just enough to get Slony working nicely
> with Logical Replication.
> 
> With that said, getting Slony to work with Logical Replication (with my patch installed) isn't a trivial
> task.  Between the "set add table" slonik command and the "subscribe set" slonik command, there needs to
> be a manual execution of "ALTER TABLE ENABLE TRIGGER [ALWAYS|REPLICA]" on each *_logtrigger and
> *_truncatetrigger for each table, along with an "ALTER TABLE DISABLE TRIGGER apply_trigger" for
> sl_log_{1,2} on the Slony-provider/LR-subscriber side (node B in my example).  But at least with my
> patch, it becomes *possible* to set up such an architecture.
> 
> Thanks for considering,
> --Richard
> 
> 
> 
> 
>
>       > I recently came across a customer who sought to set up Slony on the tail end of a
>       cascaded-replication
>       > setup:
>       > Node A -> Logical Replication -> Node B -> Slony -> Node C
>       >
>       > We discovered that this does not work because the client (WAL replay process) logs in as a
>       Replica user,
>       > but logTrigger won't fire unless replication role is Origin.
>       >
>       > Seeing that the "replication role == Origin" check is a bit dated, and Slony will
>       enable/disable triggers
>       > based on subscriber/provider status, I'm thinking that the attached patch might be worth a
>       discussion. 
>       > After all, the "replication role == Origin" check basically negates all the replication
>       role features
>       > that are now baked into Postgres core.
>       >
>       > Please let me know if there are other factors to consider.
>       >
>       > Cheers,
>       > --Richard
>       >
>       >
>       >
>       >
> 
> 
> 
> --
> Richard Yen
> Principal Support Engineer
> 
> ____________________________________________________________
> PRIVACY & CONFIDENTIALITY NOTICE
> 
> Please ensure that any data relative to individual or entity privacy is not in violation of the General
> Data Protection Regulation Act, as outlined at https://gdpr-info.eu/.  EnterpriseDB process requires that
> information which may be relevant to your technical discourse are submitted via either the Customer
> Portal through secure credentials, or provided via DropBox as provided by EnterpriseDB.
> 
> This e-mail transmission and any documents, files, or previous e-mail messages appended or attached to
> it, may contain information that is confidential or legally privileged. If you are not the intended
> recipient, or a person responsible for delivering it to the intended recipient, you are hereby notified
> that you must not read this transmission and that any disclosure, copying, printing, distribution, or use
> of the information contained or attached to this transmission is STRICTLY PROHIBITED. If you have
> received this transmission in error, please immediately notify the sender by telephone or return e-mail
> message and delete the original transmission, its attachments, and any copies without reading or saving
> in any manner. Thank you.
> 
>