Re: row filtering for logical replication
От | Euler Taveira |
---|---|
Тема | Re: row filtering for logical replication |
Дата | |
Msg-id | d65b0801-34bb-4db2-b0d4-8e39852c9f9d@www.fastmail.com обсуждение исходный текст |
Ответ на | Re: row filtering for logical replication (Peter Smith <smithpb2250@gmail.com>) |
Ответы |
Re: row filtering for logical replication
Re: row filtering for logical replication |
Список | pgsql-hackers |
On Tue, Aug 24, 2021, at 4:46 AM, Peter Smith wrote:
I have used debug logging to confirm that what Amit wrote [1] iscorrect; the row-filter ExprState of *every* table's row_filter willbe invalidated (and so subsequently gets rebuilt) when the userchanges the PUBLICATION tables. This was a side-effect of therel_sync_cache_publication_cb which is freeing the cached ExprStateand setting the entry->replicate_valid = false; for *every* entry.So yes, the ExprCache is getting rebuilt for some situations where itis not strictly necessary to do so.
I'm afraid we are overenginnering this feature. We already have a cache
mechanism that was suggested (that shows a small improvement). As you said the
gain for this new improvement is zero or minimal (it depends on your logical
replication setup/maintenance).
1. Although the ExprState cache is effective, in practice theperformance improvement was not very much. My previous results [2]showed only about 2sec saving for 100K calls to thepgoutput_row_filter function. So I think eliminating just one or twounnecessary calls in the get_rel_sync_entry is going to make zeroobservable difference.2. IMO it is safe to expect that the ALTER PUBLICATION is a rareoperation relative to the number of times that pgoutput_row_filterwill be called (the pgoutput_row_filter is quite a "hot" functionsince it is called for every INSERT/UPDATE/DELETE). It will be ordersof magnitude difference 1:1000, 1:100000 etc.~~Anyway, I have implemented the suggested cache change because I agreeit is probably theoretically superior, even if in practice there isalmost no difference.
I didn't inspect your patch carefully but it seems you add another List to
control this new cache mechanism. I don't like it. IMO if we can use the data
structures that we have now, let's implement your idea; otherwise, -1 for this
new micro optimization.
[By the way, it took some time to extract what you changed. Since we're trading
patches, I personally appreciate if you can send a patch on the top of the
current one. I have some changes too and it is time consuming incorporating
changes in the main patch.]
В списке pgsql-hackers по дате отправления: