Re: Perform streaming logical transactions by background workers and parallel apply

On Tue, Jan 10, 2023 at 11:16 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Mon, 9 Jan 2023 14:21:03 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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".
>

Right, it is mentioned in docs [1] (see section "Tricky Words to Avoid").

> (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,
> =====
>

Can you please start a new thread and post these changes as we are
proposing to change existing message as well?

[1] - https://www.postgresql.org/docs/devel/error-style-guide.html

--
With Regards,
Amit Kapila.