| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Fabrice Chapuis <fabrice636861(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, Alexander Kukushkin <cyberdemn(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: issue with synchronized_standby_slots |
| Date: | 2025-10-24 07:22:18 |
| Message-ID: | CANhcyEVVL3yLpKzXo3Yq8ttXr9oo8=OG+Y4k7i639E2EagCa-w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Fujii-san, Amit,
Thanks for reviewing the patch.
On Fri, 24 Oct 2025 at 10:31, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Oct 23, 2025 at 9:08 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> >
> > On Thu, Oct 23, 2025 at 8:28 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > +-- Parallel worker does not throw error during startup.
> > > +SET min_parallel_table_scan_size TO 0;
> > > +SET max_parallel_workers_per_gather TO 2;
> > > +SET parallel_setup_cost TO 0;
> > > +SET parallel_tuple_cost TO 0;
> > > +CREATE TABLE t1(a int);
> > > +INSERT INTO t1 VALUES(1), (2), (3), (4);
> > > +SELECT count(*) FROM t1;
> > >
> > > Isn't it better to reset these parameters after the test?
> >
> > I think the intention of this test case is to verify that the issue seen
> > in HEAD no longer occurs with the patch applied. So in HEAD this test
> > procedure needs to be able to reproduce the problem with
> > synchronized_standby_slots and parallel workers, but does it actually do?
> > I'm afraid additional steps are needed.
> >
> > Also, I wonder if it's really worth doing this test after the fix,
> > since it seems a special case.
> >
>
> Agreed. I also don't see the need.
>
I also agree. I have removed the test in the latest version of patch.
> >
> > +-- Cannot set synchronized_standby_slots to a invalid slot name.
> > +ALTER SYSTEM SET synchronized_standby_slots='invalid*';
> >
> > Typo: "a invalid" should be "an invalid"
> >
>
> We can keep this test as the results with and without patch will be
> different for this case.
>
> HEAD:
> postgres=# ALTER SYSTEM SET synchronized_standby_slots='invalid*';
> ERROR: invalid value for parameter "synchronized_standby_slots": "invalid*"
> DETAIL: Replication slot "invalid*" does not exist.
>
> Patch:
> postgres=# ALTER SYSTEM SET synchronized_standby_slots='invalid*';
> ERROR: invalid value for parameter "synchronized_standby_slots": "invalid*"
> DETAIL: replication slot name "invalid*" contains invalid character
> HINT: Replication slot names may only contain lower case letters,
> numbers, and the underscore character.
>
I have corrected the typo in the comment and also kept this test.
I have attached the patches in [1].
[1]: https://www.postgresql.org/message-id/CANhcyEXBnQs31N2zAH7m9uSNWfOBianZEMEAzi-rj_kQ3B9ihQ%40mail.gmail.com
Thanks,
Shlok Kyal
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Arseniy Mukhin | 2025-10-24 07:36:44 | Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue |
| Previous Message | Shlok Kyal | 2025-10-24 07:20:56 | Re: issue with synchronized_standby_slots |