Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

On Wed, Feb 15, 2023 at 9:23 AM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:
>
> On Sat, Feb 4, 2023 7:24 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Feb 2, 2023 at 2:03 PM Önder Kalacı <onderkalaci@gmail.com> wrote:
> > >
> > > On v23, I dropped the planner support for picking the index. Instead, it simply
> > > iterates over the indexes and picks the first one that is suitable.
> > >
> > > I'm currently thinking on how to enable users to override this decision.
> > > One option I'm leaning towards is to add a syntax like the following:
> > >
> > > ALTER SUBSCRIPTION .. ALTER TABLE ... SET INDEX ...
> > >
> > > Though, that should probably be a seperate patch. I'm going to work
> > > on that, but still wanted to share v23 given picking the index sounds
> > > complementary, not strictly required at this point.
> > >
> >
> > I agree that it could be a separate patch. However, do you think we
> > need some way to disable picking the index scan? This is to avoid
> > cases where sequence scan could be better or do we think there won't
> > exist such a case?
> >
>
> I think such a case exists. I tried the following cases based on v23 patch.
>
...
> # Result
> The time executing update (the average of 3 runs is taken, the unit is
> milliseconds):
>
> +--------+---------+---------+
> |        | patched |  master |
> +--------+---------+---------+
> | case 1 | 3933.68 | 1298.32 |
> | case 2 | 1803.46 | 1294.42 |
> | case 3 | 1380.82 | 1299.90 |
> | case 4 | 1042.60 | 1300.20 |
> | case 5 |  691.69 | 1297.51 |
> | case 6 |  578.50 | 1300.69 |
> | case 7 |  566.45 | 1302.17 |
> +--------+---------+---------+
>
> In case 1~3, there's an overhead after applying the patch. In other cases, the
> patch improved the performance. As more duplicate values, the greater the
> overhead after applying the patch.
>

I think this overhead seems to be mostly due to the need to perform
tuples_equal multiple times for duplicate values. I don't know if
there is any simple way to avoid this without using the planner stuff
as was used in the previous approach. So, this brings us to the
question of whether just providing a way to disable/enable the use of
index scan for such cases is sufficient or if we need any other way.

Tom, Andres, or others, do you have any suggestions on how to move
forward with this patch?