On Tue, Sep 29, 2026 at 12:24:46PM +0100, Zsolt Parragi wrote:
> a8458f508a7 on 2024-07-13 fixed the issue causing that instability, so
> now it should be safe to revert the revert, at least I can't reproduce
> the problem mentioned in the reverts with this fix in place, and I can
> without it.
> In theory a8458f508a7 was backported everywhere, but that doesn't mean
> this change would be completely risk-free.

Thanks for looking at some history and putting some pieces together.
I was not aware of this stuff. 

In this context, the revert of the revert can be translated as `git
revert 29992a6a509b`, right?  If we do that, being able to get rid of
the alternate outputs would be super nice, and we would not even need
to have alternate outputs like that:
--- temp-schema-cleanup.out     2026-07-27 08:05:11.192854428 +0900
+++ temp-schema-cleanup_1.out   2026-09-30 08:03:16.982528616 +0900
@@ -89,10 +89,10 @@ DETAIL:  It depends on temporary type ju
 step s1_exit: 
     SELECT pg_terminate_backend(pg_backend_pid());
 
-FATAL:  terminating connection due to administrator command
 server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
+invalid socket
 
 step s2_advisory: <... completed>
 pg_advisory_lock

This points to the fact that losing the messages is wrong, because the
tests do not check what we want them to do in some environments as the
data we want is not received in the backends.  And we would not even
need the isolationtester.c tweaks, assuming that things work out and
that we can rely on the fact that the backend has sent all its
messages, would we?

+    DO $$
+    BEGIN
+      WHILE EXISTS (SELECT FROM pg_stat_activity
+                    WHERE application_name = 'isolation/wait_cleanup/s1')
+      LOOP
+        PERFORM pg_sleep(0.01);
+        PERFORM pg_stat_clear_snapshot();
+      END LOOP;
+    END$$;

Even that feels like the wrong thing to do, spreading a tweak that
ought to be simpler for folks implement tests.

In short, it means that the proposal is papering over the actual
Windows problem: pqcomm.c should really close that socket, so my issue
here is that everybody has lost track of the inter-dependency between
all these issues; pieces that you are just putting together.  I would
not mind experimenting with a revert of 29992a6a509b on HEAD, at
least, and give it some time to brew before deciding what to do with
the stable branches.

Another option that would be on the table for me would be to make
these tests conditional, not running on WIN32 where we know they're
unstable.  We cannot do that within schedule files so
temp-schema-cleanup would be an issue if not moved out somewhere else
(just test_misc with a conditional ISOLATION list?), and I don't
remember a way to control that cleanly at SQL level..  We could do a
solution based on a test list filtering in test_decoding and
injection_points, at least, taking take of two instabilities out of
three.

The perfect scenario for me would be to prove that undoing
29992a6a509b is now really-absolutely-stable safe, as it's still a
server bug to me to not send back this information back to the client
on WIN32.

What do you think?
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to