RE: [Proposal] Add foreign-server health checks infrastructure
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
From: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>
To: 'Shinya Kato' <Shinya11.Kato@oss.nttdata.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "zyu@yugabyte.com" <zyu@yugabyte.com>
Date: 2021-12-15T06:40:02Z
Lists: pgsql-hackers
Attachments
- patchset.zip (application/x-zip-compressed)
- v05_0001_add_checking_infrastracture.patch (application/octet-stream) patch v5
- v05_0002_add_doc.patch (application/octet-stream) patch v5
Dear Kato-san, Thank you for giving comments! And sorry for late reply. I rebased my patches. > Even for local-only transaction, I thought it useless to execute > CallCheckingRemoteServersCallbacks() and enable_timeout_after(). Can I > make it so that it determines outside whether it contains SQL to the > remote or not? Yeah, remote-checking timeout will be enable even ifa local transaction is opened. In my understanding, postgres cannot distinguish whether opening transactions are using only local object or not. My first idea was that turning on the timeout when GetFdwRoutineXXX functions were called, but in some cases FDWs may not use remote connection even if they call such a handler function. e.g. postgresExplainForeignScan(). Hence I tried another approach that unregister all checking callback the end of each transactions. Only FDWs can notice that transactions are remote or local, so they must register callback when they really connect to other database. This implementation will avoid calling remote checking in the case of local transaction, but multiple registering and unregistering may lead another overhead. I attached which implements that. > The following points are minor. > 1. In foreign.c, some of the lines are too long and should be broken. > 2. In pqcomm.c, the newline have been removed, what is the intention of > this? > 3. In postgres.c, > 3-1. how about inserting a comment between lines 2713 and 2714, similar > to line 2707? > 3-2. the line breaks in line 3242 and line 3243 should be aligned. > 3-3. you should change > /* > * Skip checking foreign servers while reading > messages. > */ > to > /* > * Skip checking foreign servers while reading > messages. > */ > 4. In connection.c, There is a typo in line 1684, so "fucntion" should > be changed to "function". Maybe all of them were fixed. Thanks! How do you think? Best Regards, Hayato Kuroda FUJITSU LIMITED
Commits
-
postgres_fdw: Extend postgres_fdw_get_connections to return user name.
- 4f08ab554577 18.0 landed
-
postgres_fdw: Fix bug in connection status check.
- 454aab4b738e 18.0 landed
-
postgres_fdw: Add connection status check to postgres_fdw_get_connections().
- 857df3cef7be 18.0 landed
-
postgres_fdw: Add "used_in_xact" column to postgres_fdw_get_connections().
- c297a47c5f8d 18.0 landed
-
doc: Enhance documentation for postgres_fdw_get_connections() output columns.
- 284c030a10b8 18.0 landed
-
Avoid reference to nonexistent array element in ExecInitAgg().
- 92957ed98c5c 16.0 cited
-
Add WL_SOCKET_CLOSED for socket shutdown events.
- 50e570a59e7f 15.0 cited
-
postgres_fdw: reestablish new connection if cached one is detected as broken.
- 32a9c0bdf493 14.0 cited