On 21/09/16 15:04, Peter Eisentraut wrote:
> Some partial notes on 0005-Add-logical-replication-workers.patch:
>
> Document to what extent other relation types are supported (e.g.,
> materialized views as source, view or foreign table or temp table as
> target). Suggest an updatable view as target if user wants to have
> different table names or write into a different table structure.
>
I don't think that's good suggestion, for one it won't work for UPDATEs
as we have completely different path for finding the tuple to update
which only works on real data, not on view. I am thinking of even just
allowing table to table replication in v1 tbh, but yes it should be
documented what target relation types can be.
>
> subscriptioncmds.c: Perhaps combine logicalrep_worker_find() and
> logicalrep_worker_stop() into one call that also encapsulates the
> required locking.
I was actually thinking of moving the wait loop that waits for worker to
finish there as well.
>
> In get_subscription_list(), the memory context pointers don't appear to
> do anything useful, because everything ends up being CurrentMemoryContext.
>
That's kind of the point of the memory context pointers there though as
we start transaction inside that function.
> pg_stat_get_subscription(NULL) for "all" seems a bit of a weird interface.
>
I modeled that after pg_stat_get_activity() which seems to be similar
type of interface.
> pglogical_apply_main not used, should be removed.
>
Hah.
> In logicalreprel_open(), the error message "cache lookup failed for
> remote relation %u" could be clarified. This message could probably
> happen if the protocol did not send a Relation message first. (The term
> "cache" is perhaps inappropriate for LogicalRepRelMap, because it
> implies that the value can be gotten from elsewhere if it's not in the
> cache. In this case it's really session state that cannot be recovered
> easily.)
>
Yeah I have different code and error for that now.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
