On 2020-11-19 07:11, Bharath Rupireddy wrote:
On Wed, Nov 18, 2020 at 10:32 PM Alexey Kondratov
<a.kondra...@postgrespro.ru> wrote:
Thanks! I will make separate patches and post them soon.
>> Attached is a small POC patch, which implements this contrib-level
>> postgres_fdw.keep_connections GUC. What do you think?
>
> I see two problems with your patch: 1) It just disconnects the remote
> connection at the end of txn if the GUC is set to false, but it
> doesn't remove the connection cache entry from ConnectionHash.
Yes, and this looks like a valid state for postgres_fdw and it can get
into the same state even without my patch. Next time GetConnection()
will find this cache entry, figure out that entry->conn is NULL and
establish a fresh connection. It is not clear for me right now, what
benefits we will get from clearing also this cache entry, except just
doing this for sanity.
By clearing the cache entry we will have 2 advantages: 1) we could
save a(small) bit of memory 2) we could allow new connections to be
cached, currently ConnectionHash can have only 8 entries. IMHO, along
with disconnecting, we can also clear off the cache entry. Thoughts?
IIUC, 8 is not a hard limit, it is just a starting size. ConnectionHash
is not a shared-memory hash table, so dynahash can expand it on-the-fly
as follow, for example, from the comment before hash_create():
* Note: for a shared-memory hashtable, nelem needs to be a pretty good
* estimate, since we can't expand the table on the fly. But an
unshared
* hashtable can be expanded on-the-fly, so it's better for nelem to be
* on the small side and let the table grow if it's exceeded. An overly
* large nelem will penalize hash_seq_search speed without buying much.
Also I am not sure that by doing just a HASH_REMOVE you will free any
memory, since hash table is already allocated (or expanded) to some
size. So HASH_REMOVE will only add removed entry to the freeList, I
guess.
Anyway, I can hardly imagine bloating of ConnectionHash to be a problem
even in the case, when one has thousands of foreign servers all being
accessed during a single backend life span.
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
