Thank you for the new patch!

On 2021-12-15 15:40, kuroda.hay...@fujitsu.com wrote:
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.

It certainly incurs another overhead, but I think it's better than the previous one. So far, I haven't encountered any problems, but I'd like other people's opinions.

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!
Thank you, and it looks good to me.


--
Regards,

--
Shinya Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


Reply via email to