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

From: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
To: David Zhang <david(dot)zhang(at)highgo(dot)ca>
Cc: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: postgres_fdw: commit remote (sub)transactions in parallel during pre-commit
Date: 2022-05-02 08:25:31
Message-ID: CAPmGK15DH+oqo=Lgn4S3ACnU3C07fSDYW_fKt09hVHskopQzxQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Wed, Apr 20, 2022 at 4:55 AM David Zhang <david(dot)zhang(at)highgo(dot)ca> wrote:
> I tried to apply the patch to master and plan to run some tests, but got
> below errors due to other commits.

I rebased the patch against HEAD. Attached is an updated patch.

> + * remote server in parallel at (sub)transaction end.
>
> Here, I think the comment above could potentially apply to multiple
> remote server(s).

I agree on that point, but I think it's correct to say "the remote
server" here, because we determine this for the given remote server.
Maybe I'm missing something, so could you elaborate on it?

> Not sure if there is a way to avoid repeated comments? For example, the
> same comment below appears in two places (line 231 and line 296).
>
> + /*
> + * If requested, consume whatever data is available from the socket.
> + * (Note that if all data is available, this allows
> + * pgfdw_get_cleanup_result to call PQgetResult without forcing the
> + * overhead of WaitLatchOrSocket, which would be large compared to the
> + * overhead of PQconsumeInput.)
> + */

IMO I think it's OK to have this in multiple places, because 1) IMO it
wouldn't be that long, and 2) we already duplicated comments like this
in the same file in v14 and earlier. Here is such an example in
pgfdw_xact_callback() and pgfdw_subxact_callback() in that file in
those versions:

/*
* If a command has been submitted to the remote server by
* using an asynchronous execution function, the command
* might not have yet completed. Check to see if a
* command is still being processed by the remote server,
* and if so, request cancellation of the command.
*/

Thanks for reviewing! Sorry for the delay.

Best regards,
Etsuro Fujita

Attachment Content-Type Size
v8-postgres-fdw-Add-support-for-parallel-abort.patch application/octet-stream 26.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2022-05-02 08:38:59 Refactor construct_array() and deconstruct_array() for built-in types
Previous Message Bharath Rupireddy 2022-05-02 07:57:16 Add last failed connection error message to pg_stat_wal_receiver