From: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Retry Cached Remote Connections for postgres_fdw in case remote backend gets killed/goes away |
Date: | 2020-10-06 01:54:27 |
Message-ID: | 182fb357-a264-88fe-cbb7-bf0f1563edbe@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2020/10/05 20:32, Bharath Rupireddy wrote:
> On Mon, Oct 5, 2020 at 9:45 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>
>>> I see the errmsg() with plain texts in other places in the code base
>>> as well. Is it that we look at the error message and if it is a plain
>>> text(without database objects or table data), we decide to have no
>>> translation? Or is there any other policy?
>>
>> I was thinking that elog() basically should be used to report this
>> debug message, instead, but you used ereport() because maybe
>> you'd like to add detail message about connection error. Is this
>> understanding right? elog() uses errmsg_internal().
>>
>
> Yes that's correct.
>
>>
>> So if ereport() is used as an aternative of elog() for some reasons,
>> IMO errmsg_internal() should be used. Thought?
>>
>
> Yes, this is apt for our case.
>
>>
>> OTOH, the messages you mentioned are not debug ones,
>> so basically ereport() and errmsg() should be used, I think.
>>
>
> In connection.c file, yes they are of ERROR type. Looks like it's not
> a standard to use errmsg_internal for DEBUG messages that require no
> translation with ereport
>
> (errmsg("wrote block details for %d blocks", num_blocks)));
> (errmsg("MultiXact member stop limit is now %u based on MultiXact %u
> (errmsg("oldest MultiXactId member is at offset %u",
>
> However, there are few other places, where errmsg_internal is used for
> DEBUG purposes.
>
> (errmsg_internal("finished verifying presence of "
> (errmsg_internal("%s(%d) name: %s; blockState:
>
> Having said that, IMHO it's better to keep the way it is currently in
> the code base.
Agreed.
>
>>
>>> Thanks. Attaching v10 patch. Please have a look.
>>
>> Thanks for updating the patch! I will mark the patch as ready for committer in CF.
>> Barring any objections, I will commit that.
>>
>
> Thanks a lot for the review comments.
I pushed the patch. Thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2020-10-06 02:18:19 | Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view? |
Previous Message | Kyotaro Horiguchi | 2020-10-06 01:06:44 | Re: shared-memory based stats collector |