On Fri, May 11, 2018 at 9:56 AM, Masahiko Sawada <sawada.m...@gmail.com> wrote: > Thank you for the comment. > > On Fri, May 11, 2018 at 3:57 AM, Robert Haas <robertmh...@gmail.com> wrote: >> On Tue, Feb 27, 2018 at 2:21 AM, Masahiko Sawada <sawada.m...@gmail.com> >> wrote: >>> I might be missing your point. As for API breaking, this patch doesn't >>> break any existing FDWs. All new APIs I proposed are dedicated to 2PC. >>> In other words, FDWs that work today can continue working after this >>> patch gets committed, but if FDWs want to support atomic commit then >>> they should be updated to use new APIs. The reason why the calling of >>> FdwXactRegisterForeignServer is necessary is that the core code >>> controls the foreign servers that involved with the transaction but >>> the whether each foreign server uses 2PC option (two_phase_commit) is >>> known only on FDW code. We can eliminate the necessity of calling >>> FdwXactRegisterForeignServer by moving 2PC option from fdw-level to >>> server-level in order not to enforce calling the registering function >>> on FDWs. If we did that, the user can use the 2PC option as a >>> server-level option. >> >> Well, FdwXactRegisterForeignServer has a "bool two_phase_commit" >> argument. If it only needs to be called by FDWs that support 2PC, >> then that argument is unnecessary. If it needs to be called by all >> FDWs, then it breaks existing FDWs that don't call it. >> > > I understood now. For now since FdwXactRegisterForeignServer needs to > be called by only FDWs that support 2PC, I will remove the argument.
Regarding to API design, should we use 2PC for a distributed transaction if both two or more 2PC-capable foreign servers and 2PC-non-capable foreign server are involved with it? Or should we end up with an error? the 2PC-non-capable server might be either that has 2PC functionality but just disables it or that doesn't have it. If we use it, the transaction atomicity will be satisfied among only 2PC-capable servers that might be part of all participants. Or If we don't, we use 1PC instead in such case but when using 2PC transactions always ends up with satisfying the transaction atomicity among all participants of the transaction. The current patch takes the former (doesn't allow PREPARE case though), but I think we also could take the latter way because it doesn't make sense for user even if the transaction commit atomically among not all participants. Also, regardless whether we take either way I think it would be better to manage not only 2PC transaction but also non-2PC transaction in the core and add two_phase_commit argument. I think we can use it without breaking existing FDWs. Currently FDWs manage transactions using XactCallback but new APIs being added also manage transactions. I think it might be better if users use either way (using XactCallback or using new APIs) for transaction management rather than use both ways with combination. Otherwise two codes for transaction management will be required: the code that manages foreign transactions using XactCallback for non-2PC transactions and code that manages them using new APIs for 2PC transactions. That would not be easy for FDW developers. So what I imagined for new API is that if FDW developers use new APIs they can use both 2PC and non-2PC transaction, but if they use XactCallback they can use only non-2PC transaction. Any thoughts? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
