Hi, For pg-foreign/v31-0004-Add-PrepareForeignTransaction-API.patch : However these functions are not neither committed nor aborted at
I think the double negation was not intentional. Should be 'are neither ...' For FdwXactShmemSize(), is another MAXALIGN(size) needed prior to the return statement ? + fdwxact = FdwXactInsertFdwXactEntry(xid, fdw_part); For the function name, Fdw and Xact appear twice, each. Maybe one of them can be dropped ? + * we don't need to anything for this participant because all foreign 'need to' -> 'need to do' + else if (TransactionIdDidAbort(xid)) + return FDWXACT_STATUS_ABORTING; + the 'else' can be omitted since the preceding if would return. + if (max_prepared_foreign_xacts <= 0) I wonder when the value for max_prepared_foreign_xacts would be negative (and whether that should be considered an error). Cheers On Wed, Jan 6, 2021 at 5:45 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > On Mon, Dec 28, 2020 at 11:24 PM Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > > > On Wed, Nov 25, 2020 at 9:50 PM Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > > > > > Since the previous version conflicts with the current HEAD I've > > > attached the rebased version patch set. > > > > Rebased the patch set again to the current HEAD. > > > > The discussion of this patch is very long so here is a short summary > > of the current state: > > > > It’s still under discussion which approaches are the best for the > > distributed transaction commit as a building block of built-in sharing > > using foreign data wrappers. > > > > Since we’re considering that we use this feature for built-in > > sharding, the design depends on the architecture of built-in sharding. > > For example, with the current patch, the PostgreSQL node that received > > a COMMIT from the client works as a coordinator and it commits the > > transactions using 2PC on all foreign servers involved with the > > transaction. This approach would be good with the de-centralized > > sharding architecture but not with centralized architecture like the > > GTM node of Postgres-XC and Postgres-XL that is a dedicated component > > that is responsible for transaction management. Since we don't get a > > consensus on the built-in sharding architecture yet, it's still an > > open question that this patch's approach is really good as a building > > block of the built-in sharding. > > > > On the other hand, this feature is not necessarily dedicated to the > > built-in sharding. For example, the distributed transaction commit > > through FDW is important also when atomically moving data between two > > servers via FDWs. Using a dedicated process or server like GTM could > > be an over solution. Having the node that received a COMMIT work as a > > coordinator would be better and straight forward. > > > > There is no noticeable TODO in the functionality so far covered by > > this patch set. This patchset adds new FDW APIs to support 2PC, > > introduces the global transaction manager, and implement those FDW > > APIs to postgres_fdw. Also, it has regression tests and documentation. > > Transactions on foreign servers involved with the distributed > > transaction are committed using 2PC. Committing using 2PC is performed > > asynchronously and transparently to the user. Therefore, it doesn’t > > guarantee that transactions on the foreign server are also committed > > when the client gets an acknowledgment of COMMIT. The patch doesn't > > cover synchronous foreign transaction commit via 2PC is not covered by > > this patch as we still need a discussion on the design. > > > > I've attached the rebased patches to make cfbot happy. > > > Regards, > > -- > Masahiko Sawada > EnterpriseDB: https://www.enterprisedb.com/ >
