Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit

On 2022/02/11 21:59, Etsuro Fujita wrote:
> I tweaked comments/docs a little bit as well.

Thanks for updating the patches!

I reviewed 0001 patch. It looks good to me except the following minor things. If these are addressed, I think that the
001patch can be marked as ready for committer.
 

+     * Also determine to commit (sub)transactions opened on the remote server
+     * in parallel at (sub)transaction end.

Like the comment "Determine whether to keep the connection ...", "determine to commit" should be "determine whether to
commit"?

"remote server" should be "remote servers"?


+    curlevel = GetCurrentTransactionNestLevel();
+    snprintf(sql, sizeof(sql), "RELEASE SAVEPOINT s%d", curlevel);

Why does pgfdw_finish_pre_subcommit_cleanup() need to call GetCurrentTransactionNestLevel() and construct the "RELEASE
SAVEPOINT"query string again? pgfdw_subxact_callback() already does them and probably we can make it pass either of
themto pgfdw_finish_pre_subcommit_cleanup() as its argument.
 


+       This option controls whether <filename>postgres_fdw</filename> commits
+       remote (sub)transactions opened on a foreign server in a local
+       (sub)transaction in parallel when the local (sub)transaction commits.

"a foreign server" should be "foreign servers"?

"in a local (sub)transaction" part seems not to be necessary.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION