RE: [Proposal] Add foreign-server health checks infrastructure

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Fujii Masao' <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: 'Katsuragi Yuta' <katsuragiy(at)oss(dot)nttdata(dot)com>, 'vignesh C' <vignesh21(at)gmail(dot)com>, 'Ted Yu' <yuzhihong(at)gmail(dot)com>, 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "'pgsql-hackers(at)lists(dot)postgresql(dot)org'" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, 'Önder Kalacı' <onderkalaci(at)gmail(dot)com>, "'Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com'" <Shinya11(dot)Kato(at)oss(dot)nttdata(dot)com>, 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>, 'Shubham Khanna' <khannashubham1197(at)gmail(dot)com>
Subject: RE: [Proposal] Add foreign-server health checks infrastructure
Date: 2024-08-02 05:56:01
Message-ID: TYAPR01MB5692DA275D54AC6653604C6BF5B32@TYAPR01MB5692.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Fujii-san,

> Thanks for updating the patch!
>
> > - Changed the name of new API from `GetUserMappingFromOid` to
> `GetUserMappingByOid`
> > to keep the name consistent with others.
>
> If we expose this function as an FDW helper function, it should return
> a complete UserMapping object, including umoptions.
>
> However, if postgres_fdw_get_connections() is the only user of this function,
> I'm not inclined to expose it as an FDW helper.

One reason is that the function does not handle any specific data for postgres_fdw,
however, there are no users and requirests from other projects. Based on that, ok,
we can move it to connection.c. If needed, we can export it again.

> Instead, we can either get
> the user ID by user mapping OID directly in connection.c using SearchSysCache(),
> or add the user ID to ConnCacheEntry and use it in
> postgres_fdw_get_connections().
> Thought?

I moved the function to connection.c, which uses the SearchSysCache1().

I've tried both ways, and they worked well. One difference is that when we use
the extended ConnCacheEntry approach and the entry has been invalidated, we cannot
distinguish the reason. For example, in the below case, the entry is invalidated,
so the user_name of the output record will be NULL, whereas the user mapping is
actually still valid. We may be able to add the reason for invalidation, but
I'm not very motivated to modify the part.

```
BEGIN;
SELECT 1 FROM ft1 LIMIT 1; -- ft1 is at server "loopback"
...
ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off');
...
SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-> {"loopback", NULL, ....} will be returned
```

Another reason is that we can keep the code consistent with the server case.
The part does not read data from the entry, and we can follow.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
v2-0001-Extend-postgres_fdw_get_connections-to-return-use.patch application/octet-stream 10.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Antonin Houska 2024-08-02 06:09:26 Re: why there is not VACUUM FULL CONCURRENTLY?
Previous Message Tom Lane 2024-08-02 05:33:45 Re: pg_dump: optimize dumpFunc()