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

On Tue, Jan 10, 2023 at 11:16 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> 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".
>

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.