Hi,
On Thursday, January 19, 2023 10:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Thu, Jan 19, 2023 at 12:06 PM Takamichi Osumi (Fujitsu)
> <osumi.takamichi@fujitsu.com> wrote:
> >
> > Kindly have a look at the updated patch v17.
> >
>
> Can we try to optimize the test time for this test? On my machine, it is the
> second highest time-consuming test in src/test/subscription. It seems you are
> waiting twice for apply_delay and both are for streaming cases by varying the
> number of changes. I think it should be just once and that too for the
> non-streaming case. I think it would be good to test streaming code path
> interaction but not sure if it is important enough to have two test cases for
> apply_delay.
The first insert test is for non-streaming case and we need both cases
for coverage. Regarding the time of test, conducted some optimization
such as turning off the initial table sync, shortening the time of wait, and so on.
>
> One minor comment that I observed while going through the patch.
> + /*
> + * The combination of parallel streaming mode and min_apply_delay is
> + not
> + * allowed.
> + */
> + if (IsSet(supported_opts, SUBOPT_MIN_APPLY_DELAY) &&
> + opts->min_apply_delay > 0)
>
> I think it would be good if you can specify the reason for not allowing this
> combination in the comments.
Added.
Please have a look at the latest v18 patch in [1].
[1] -
https://www.postgresql.org/message-id/TYCPR01MB8373BED9E390C4839AF56685EDC59%40TYCPR01MB8373.jpnprd01.prod.outlook.com
Best Regards,
Takamichi Osumi