public inbox for [email protected]  
help / color / mirror / Atom feed
From: Euler Taveira <[email protected]>
To: Peter Smith <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Rahila Syed <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Önder Kalacı <[email protected]>
Cc: japin <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: David Steele <[email protected]>
Cc: Craig Ringer <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Amit Langote <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: row filtering for logical replication
Date: Tue, 24 Aug 2021 21:21:57 -0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHut+Ps5j7mkO0xLmNW=kXh0eezGoKyzBCiQc9bfkCiM_MVDrg@mail.gmail.com>
References: <CA+HiwqH+9NK5m-=BOYpzjH5UEpeWjT_3HEm4XBYNrda88naGMQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<ME3P282MB16678160F5EA09DCB962474EB6B69@ME3P282MB1667.AUSP282.PROD.OUTLOOK.COM>
	<[email protected]>
	<YBjc/[email protected]>
	<MEYP282MB16699D041638CE2F8284701FB6B59@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
	<MEYP282MB1669F5063EC084C0C5854EB1B6B59@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
	<[email protected]>
	<CACawEhW0NUMHHAQjVP_xM8gWXKvy9haK9nKhC-H=Qn2yTCv-=g@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAH2L28tjTYctr+eu6Vg5U6iihVhe6P+qKnB_diMGy4=CfwXzdg@mail.gmail.com>
	<CAH2L28sS8=jY6Lj2-xVu6exxQ9gK8uGZr9jf64zDw9hacMrkyA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAH2L28tvKGLO9ZRFsaA5msXqZzF6wHgxgXNvnCQ2dRQfs1T8XQ@mail.gmail.com>
	<[email protected]>
	<CAA4eK1Lu7oPHm2j=nLeqZLVoro76E0EWvH+5wmGG39iJNBzUog@mail.gmail.com>
	<[email protected]>
	<CAHut+PvrJHz_9HG5jvqHWe=FZLz9A+CxyJDyZKsHmY8THU+pjg@mail.gmail.com>
	<[email protected]>
	<CAHut+PvCWX8WauZJkXfgL08nn8+rDsTdxQinhyL1z2+E7r06fw@mail.gmail.com>
	<CAA4eK1LPh+KCALU1Eo4itWq+bYU6NzUGr4dwypMynM3Skjvv5A@mail.gmail.com>
	<[email protected]>
	<CAHut+Ps3GgPKUJ2npfY4bQdxAmYW+yQin+hQuBsMYvX=kBqEpA@mail.gmail.com>
	<[email protected]>
	<CAA4eK1+xQb06NGs6Y7OzwMtKYYixEqR8tdWV5THAVE4SAqNrDg@mail.gmail.com>
	<CAHut+Ps5j7mkO0xLmNW=kXh0eezGoKyzBCiQc9bfkCiM_MVDrg@mail.gmail.com>

On Tue, Aug 24, 2021, at 4:46 AM, Peter Smith wrote:
> I have used debug logging to confirm that what Amit wrote [1] is
> correct; the row-filter ExprState of *every* table's row_filter will
> be invalidated (and so subsequently gets rebuilt) when the user
> changes the PUBLICATION tables. This was a side-effect of the
> rel_sync_cache_publication_cb which is freeing the cached ExprState
> and setting the entry->replicate_valid = false; for *every* entry.
> 
> So yes, the ExprCache is getting rebuilt for some situations where it
> is 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 the
> performance improvement was not very much. My previous results [2]
> showed only about 2sec saving for 100K calls to the
> pgoutput_row_filter function. So I think eliminating just one or two
> unnecessary calls in the get_rel_sync_entry is going to make zero
> observable difference.
> 
> 2. IMO it is safe to expect that the ALTER PUBLICATION is a rare
> operation relative to the number of times that pgoutput_row_filter
> will be called (the pgoutput_row_filter is quite a "hot" function
> since it is called for every INSERT/UPDATE/DELETE). It will be orders
> of magnitude difference 1:1000, 1:100000 etc.
> 
> ~~
> 
> Anyway, I have implemented the suggested cache change because I agree
> it is probably theoretically superior, even if in practice there is
> almost 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.]


--
Euler Taveira
EDB   https://www.enterprisedb.com/


view thread (489+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: row filtering for logical replication
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox