From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | vignesh C <vignesh21(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Implement generalized sub routine find_in_log for tap test |
Date: | 2023-06-06 02:35:49 |
Message-ID: | CAA4eK1Kbyo-L3nfMnZbusgv_a7jejxcm1gsDBJ2VH5KnK1rkTw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Jun 4, 2023 at 3:51 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Mon, May 29, 2023 at 07:49:52AM +0530, vignesh C wrote:
> > Thanks for the comment, the attached v3 version patch has the changes
> > for the same.
>
> -if (find_in_log(
> - $node, $log_offset,
> - qr/peer authentication is not supported on this platform/))
> +if ($node->log_contains(
> + qr/peer authentication is not supported on this platform/),
> + $log_offset,)
>
> This looks like a typo to me, the log offset is eaten.
>
> Except of that, I am on board with log_contains().
>
> There are two things that bugged me with the refactoring related to
> connect_ok and connect_fails:
> - check_connect_log_contents() is a name too complicated. While
> looking at that I have settled to a simpler log_check(), as we could
> perfectly use that for something else than connections as long as we
> want to check multiple patterns at once.
> - The refactoring of the documentation for the routines of Cluster.pm
> became incorrect. For example, the patch does not list anymore
> log_like and log_unlike for connect_ok.
>
> With all that in mind I have hacked a few adjustments in a 0003,
> though I agree with the separation between 0001 and 0002.
>
> 033_replay_tsp_drops and 019_replslot_limit are not new to v16, but
> 003_peer.pl and 035_standby_logical_decoding.pl, making the number of
> places where find_in_log() exists twice as much. So I would be
> tempted to refactor these tests in v16. Perhaps anybody from the RMT
> could comment? We've usually been quite flexible with the tests even
> in beta.
>
Personally, I don't see any problem to do this refactoring for v16.
However, sometimes, we do decide to backpatch refactoring in tests to
avoid backpatch effort. I am not completely sure if that is the case
here.
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii.Yuki@df.MitsubishiElectric.co.jp | 2023-06-06 03:08:50 | RE: Partial aggregates pushdown |
Previous Message | jiye | 2023-06-06 02:34:35 | Re: confusion about this commit "Revert "Skip redundant anti-wraparound vacuums"" |