| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Zhihong Yu <zyu(at)yugabyte(dot)com> |
| Cc: | Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "ashutosh(dot)bapat(dot)oss(at)gmail(dot)com" <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, "amit(dot)kapila16(at)gmail(dot)com" <amit(dot)kapila16(at)gmail(dot)com>, "m(dot)usama(at)gmail(dot)com" <m(dot)usama(at)gmail(dot)com>, "ikedamsh(at)oss(dot)nttdata(dot)com" <ikedamsh(at)oss(dot)nttdata(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "sulamul(at)gmail(dot)com" <sulamul(at)gmail(dot)com>, "alvherre(at)2ndquadrant(dot)com" <alvherre(at)2ndquadrant(dot)com>, "thomas(dot)munro(at)gmail(dot)com" <thomas(dot)munro(at)gmail(dot)com>, "ildar(at)adjust(dot)com" <ildar(at)adjust(dot)com>, "horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp" <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, "chris(dot)travers(at)adjust(dot)com" <chris(dot)travers(at)adjust(dot)com>, "robertmhaas(at)gmail(dot)com" <robertmhaas(at)gmail(dot)com>, "ishii(at)sraoss(dot)co(dot)jp" <ishii(at)sraoss(dot)co(dot)jp> |
| Subject: | Re: Transactions involving multiple postgres foreign servers, take 2 |
| Date: | 2021-05-01 04:09:13 |
| Message-ID: | CAD21AoBk_G70ketUHo-Vxu4djpcGvAEhyRVPoZSrB-k8Uw-R=A@mail.gmail.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Mar 17, 2021 at 6:03 PM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:
>
> Hi,
> For v35-0007-Prepare-foreign-transactions-at-commit-time.patch :
Thank you for reviewing the patch!
>
> With this commit, the foreign server modified within the transaction marked as 'modified'.
>
> transaction marked -> transaction is marked
Will fix.
>
> +#define IsForeignTwophaseCommitRequested() \
> + (foreign_twophase_commit > FOREIGN_TWOPHASE_COMMIT_DISABLED)
>
> Since the other enum is FOREIGN_TWOPHASE_COMMIT_REQUIRED, I think the macro should be named: IsForeignTwophaseCommitRequired.
But even if foreign_twophase_commit is
FOREIGN_TWOPHASE_COMMIT_REQUIRED, the two-phase commit is not used if
there is only one modified server, right? It seems the name
IsForeignTwophaseCommitRequested is fine.
>
> +static bool
> +checkForeignTwophaseCommitRequired(bool local_modified)
>
> + if (!ServerSupportTwophaseCommit(fdw_part))
> + have_no_twophase = true;
> ...
> + if (have_no_twophase)
> + ereport(ERROR,
>
> It seems the error case should be reported within the loop. This way, we don't need to iterate the other participant(s).
> Accordingly, nserverswritten should be incremented for local server prior to the loop. The condition in the loop would become if (!ServerSupportTwophaseCommit(fdw_part) && nserverswritten > 1).
> have_no_twophase is no longer needed.
Hmm, I think If we process one 2pc-non-capable server first and then
process another one 2pc-capable server, we should raise an error but
cannot detect that.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Thomas Munro | 2021-05-01 04:43:21 | New assertion failed in lazy_scan_heap() on mereswine |
| Previous Message | Isaac Morland | 2021-05-01 03:27:55 | Re: Granting control of SUSET gucs to non-superusers |