Date: Tue, 10 Jan 2023 14:46:38 +0900 (JST)
Message-Id: <20230110.144638.1087113443723851718.horikyota.ntt@gmail.com>
To: amit.kapila16@gmail.com
Cc: houzj.fnst@fujitsu.com, sawada.mshk@gmail.com, wangw.fnst@fujitsu.com,
 smithpb2250@gmail.com, shiy.fnst@fujitsu.com,
 pgsql-hackers@lists.postgresql.org, dilipbalaut@gmail.com
Subject: Re: Perform streaming logical transactions by background workers
 and parallel apply
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <CAA4eK1KK7Xty7SaCReY_b5moPURowMGOsfvhOb6xQ1EAaJAh6A@mail.gmail.com>
References: <OS0PR01MB571685A645E509267AC5F30894F99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<OS0PR01MB57168A5A1C31BC316CB63D3C94F99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAA4eK1KK7Xty7SaCReY_b5moPURowMGOsfvhOb6xQ1EAaJAh6A@mail.gmail.com>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20230110.144638.1087113443723851718.horikyota.ntt%40gmail.com>
Precedence: bulk

Hello.

At Mon, 9 Jan 2023 14:21:03 +0530, Amit Kapila <amit.kapila16@gmail.com> wrote in 
> Pushed the first (0001) patch.

It added the following error message.

+	seg = dsm_attach(handle);
+	if (!seg)
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("unable to map dynamic shared memory segment")));

On the other hand we already have the following one in parallel.c
(another in pg_prewarm)

	seg = dsm_attach(DatumGetUInt32(main_arg));
	if (seg == NULL)
		ereport(ERROR,
				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
				 errmsg("could not map dynamic shared memory segment")));

Although I don't see a technical difference between the two, all the
other occurances including the just above (except test_shm_mq) use
"could not". A faint memory in my non-durable memory tells me that we
have a policy that we use "can/could not" than "unable".

(Mmm. I find ones in StartBackgroundWorker and sepgsql_client_auth.)

Shouldn't we use the latter than the former?  If that's true, it seems
to me that test_shm_mq also needs the same amendment to avoid the same
mistake in future.

=====
index 2e5914d5d9..a2d7474ed4 100644
--- a/src/backend/replication/logical/applyparallelworker.c
+++ b/src/backend/replication/logical/applyparallelworker.c
@@ -891,7 +891,7 @@ ParallelApplyWorkerMain(Datum main_arg)
        if (!seg)
                ereport(ERROR,
                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                errmsg("unable to map dynamic shared memory segment")));
+                                errmsg("could not map dynamic shared memory segment")));
 
        toc = shm_toc_attach(PG_LOGICAL_APPLY_SHM_MAGIC, dsm_segment_address(seg));
        if (!toc)
diff --git a/src/test/modules/test_shm_mq/worker.c b/src/test/modules/test_shm_mq/worker.c
index 8807727337..005b56023b 100644
--- a/src/test/modules/test_shm_mq/worker.c
+++ b/src/test/modules/test_shm_mq/worker.c
@@ -81,7 +81,7 @@ test_shm_mq_main(Datum main_arg)
        if (seg == NULL)
                ereport(ERROR,
                                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                errmsg("unable to map dynamic shared memory segment")));
+                                errmsg("could not map dynamic shared memory segment")));
        toc = shm_toc_attach(PG_TEST_SHM_MQ_MAGIC, dsm_segment_address(seg));
        if (toc == NULL)
                ereport(ERROR,
=====

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center