On Tue, 14 Jul 2020 at 11:19, Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > > > On 2020/07/14 9:08, Masahiro Ikeda wrote: > >> I've attached the latest version patches. I've incorporated the review > >> comments I got so far and improved locking strategy. > > > > Thanks for updating the patch! > > +1 > I'm interested in these patches and now studying them. While checking > the behaviors of the patched PostgreSQL, I got three comments.
Thank you for testing this patch! > > 1. We can access to the foreign table even during recovery in the HEAD. > But in the patched version, when I did that, I got the following error. > Is this intentional? > > ERROR: cannot assign TransactionIds during recovery No, it should be fixed. I'm going to fix this by not collecting participants for atomic commit during recovery. > > 2. With the patch, when INSERT/UPDATE/DELETE are executed both in > local and remote servers, 2PC is executed at the commit phase. But > when write SQL (e.g., TRUNCATE) except INSERT/UPDATE/DELETE are > executed in local and INSERT/UPDATE/DELETE are executed in remote, > 2PC is NOT executed. Is this safe? Hmm, you're right. I think atomic commit must be used also when the user executes other write SQLs such as TRUNCATE, COPY, CLUSTER, and CREATE TABLE on the local node. > > 3. XACT_FLAGS_WROTENONTEMPREL is set when INSERT/UPDATE/DELETE > are executed. But it's not reset even when those queries are canceled by > ROLLBACK TO SAVEPOINT. This may cause unnecessary 2PC at the commit phase. Will fix. Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services