Re: Logical Replication WIP

On 03/01/17 20:39, Peter Eisentraut wrote:
> In 0001-Add-PUBLICATION-catalogs-and-DDL-v16.patch.gz,
>
> +static bool
> +is_publishable_class(Oid relid, Form_pg_class reltuple)
> +{
> + return reltuple->relkind == RELKIND_RELATION &&
> + !IsCatalogClass(relid, reltuple) &&
> + reltuple->relpersistence == RELPERSISTENCE_PERMANENT &&
> + /* XXX needed to exclude information_schema tables */
> + relid >= FirstNormalObjectId;
> +}
>
> I don't think the XXX part is necessary, because IsCatalogClass()
> already checks for the same thing. (The whole thing is a bit bogus
> anyway, because you can drop and recreate the information schema at run
> time without restriction.)
>

I got this remark about IsCatalogClass() from Andres offline as well,
but it's not true, it only checks for FirstNormalObjectId for objects in
pg_catalog and toast schemas, not anywhere else.

> +#define MAX_RELCACHE_INVAL_MSGS 100
> + List *relids = GetPublicationRelations(HeapTupleGetOid(tup));
> +
> + /*
> + * We don't want to send too many individual messages, at some point
> + * it's cheaper to just reset whole relcache.
> + *
> + * XXX: the MAX_RELCACHE_INVAL_MSGS was picked arbitrarily, maybe
> + * there is better limit.
> + */
> + if (list_length(relids) < MAX_RELCACHE_INVAL_MSGS)
>
> Do we have more data on this? There are people running with 100000
> tables, and changing a publication with a 1000 tables would blow all
> that away?
>
> Maybe at least it should be set relative to INITRELCACHESIZE (400) to
> tie things together a bit?
>

I am actually thinking this should correspond to MAXNUMMESSAGES (4096)
as that's the limit on buffer size. I didn't find it the first time
around when I was looking for good number.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services