| From: | Önder Kalacı <onderkalaci(at)gmail(dot)com> |
|---|---|
| To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
| Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Ethan Mertz <ethan(dot)mertz(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Subject: | Re: [PATCH] Improving index selection for logical replication apply with replica identity full |
| Date: | 2026-06-22 10:57:48 |
| Message-ID: | CACawEhU9Lj7pNNLbxPYiwqhOP3ARJK=6KX1iPHKZxh648ik5bg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi all,
Thanks for picking this up, Ethan, and for the CC.
Hayato Kuroda (Fujitsu) <kuroda(dot)hayato(at)fujitsu(dot)com>, 22 Haz 2026 Pzt, 13:05
tarihinde şunu yazdı:
> Dear Bharath, Ethan,
>
> > Do we know the cost of invoking the planner for index selection on the
> > subscriber? AFAICS, the selection happens on the first apply or
> > whenever the relcache entry gets invalidated - so it's infrequent [1].
> > If that's the case, why not invoke the planner to make a
> > better-informed decision? It would account for bloat, size,
> > selectivity etc. I think it would be worth measuring the performance
> > impact before proceeding with the just unique vs. non-unique approach.
>
> Personally I'm still cautious to call planner code because of the
> maintenance cost.
> I'm not sure it's helpful to extend codes only for the REPLICA IDENTITY
> FULL case.
> But I see your point a bit, no one may not have compared these execution
> costs.
>
> Some history that may help, since the planner keeps coming up. The early
versions of the original patch (v1-v22) did use the planner: we built a
dummy PlannerInfo, made col = $1 AND ... restrictinfos, called
create_index_paths(), and picked the cheapest Path. So we already had
the cost and size awareness that is being discussed now.
We removed it in v23 [1]. The reason was not the cost of calling the
planner -- we agreed that index selection is rare (only on first apply
or after a relcache invalidation), so that cost was fine.
We removed it because of design and maintenance concerns: see Tom's
comment [2] about using planner internals inside execReplication.c (his
point
was that we would need to replace the existing logic there, not add on top
of it),
and Amit felt the extra maintenance was not worth it. A planner-based
approach would need to answer those points again.
One more note on the bloat idea: the chosen index is cached until the
next invalidation. So a planner-based choice also gets stale between
ANALYZE runs, just like a heuristic one. Calling the planner once at
relcache-open time does not track bloat over time.
I propose improving the index selection heuristic to prefer unique indexes,
On the heuristic itself, I am only mildly in favor, and I want to be
honest about how narrow the benefit is. It only helps when a usable
unique index already exists on the subscriber but is not picked first.
But in that case the correct answer is REPLICA IDENTITY USING INDEX (or
a primary key) on that index, which we already recommend. The case that
really pushes people to use REPLICA IDENTITY FULL: no unique key is
possible, only non-unique indexes is exactly the case this patch
leaves unchanged. Even Ethan's own benchmark uses a table that has a
unique index on "id", which would be better served by setting it as the
replica identity.
So I would describe this as a small, low-risk improvement to the default
choice, I am fine with it on that basis.
Regarding the code, it looked basically good, but one concern is that the
> selection could be slightly worse if several indexes are defined; it needs
> to
> iterate all the index entries in any cases. Can you prove the performance
> is
> acceptable?
On the concern about scanning all indexes: I do not think it is a
problem. We already walk the full index list today; this change only
removes the early return. It runs only on first apply or after an
invalidation, and the work is bounded by the number of indexes, which is
small. A quick microbenchmark should confirm it is tiny next to the
apply work itself.
Thanks,
Onder
[1]
https://postgr.es/m/CACawEhUN=+vjY0+4q416-rAYx6pw-nZMHQYsJZCftf9MjoPN3w@mail.gmail.com
[2] https://postgr.es/m/3466340.1673117404@sss.pgh.pa.us
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nisha Moond | 2026-06-22 11:02:16 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Pavel Borisov | 2026-06-22 10:44:33 | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |