On Wed, Dec 3, 2014 at 4:05 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote:
> > > On Tue, Dec 2, 2014 at 8:29 AM, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp > > wrote: > >> (2014/11/28 18:14), Ashutosh Bapat wrote: >> >>> On Thu, Nov 27, 2014 at 3:52 PM, Etsuro Fujita >>> <fujita.ets...@lab.ntt.co.jp <mailto:fujita.ets...@lab.ntt.co.jp>> >>> wrote: >>> Apart from the above, I noticed that the patch doesn't consider to >>> call ExplainForeignModify during EXPLAIN for an inherited >>> UPDATE/DELETE, as shown below (note that there are no UPDATE remote >>> queries displayed): >>> >> >> So, I'd like to modify explain.c to show those queries like this: >>> >> >> postgres=# explain verbose update parent set a = a * 2 where a = 5; >>> QUERY PLAN >>> ------------------------------__---------------------------- >>> --__------------------------- >>> Update on public.parent (cost=0.00..280.77 rows=25 width=10) >>> -> Seq Scan on public.parent (cost=0.00..0.00 rows=1 width=10) >>> Output: (parent.a * 2), parent.ctid >>> Filter: (parent.a = 5) >>> Remote SQL: UPDATE public.mytable_1 SET a = $2 WHERE ctid = $1 >>> -> Foreign Scan on public.ft1 (cost=100.00..140.38 rows=12 >>> width=10) >>> Output: (ft1.a * 2), ft1.ctid >>> Remote SQL: SELECT a, ctid FROM public.mytable_1 WHERE ((a >>> = 5)) FOR UPDATE >>> Remote SQL: UPDATE public.mytable_2 SET a = $2 WHERE ctid = $1 >>> -> Foreign Scan on public.ft2 (cost=100.00..140.38 rows=12 >>> width=10) >>> Output: (ft2.a * 2), ft2.ctid >>> Remote SQL: SELECT a, ctid FROM public.mytable_2 WHERE ((a >>> = 5)) FOR UPDATE >>> (12 rows) >>> >> >> Two "remote SQL" under a single node would be confusing. Also the node >>> is labelled as "Foreign Scan". It would be confusing to show an "UPDATE" >>> command under this "scan" node. >>> >> >> I thought this as an extention of the existing (ie, non-inherited) case >> (see the below example) to the inherited case. >> >> postgres=# explain verbose update ft1 set a = a * 2 where a = 5; >> QUERY PLAN >> ------------------------------------------------------------ >> ------------------------- >> Update on public.ft1 (cost=100.00..140.38 rows=12 width=10) >> Remote SQL: UPDATE public.mytable_1 SET a = $2 WHERE ctid = $1 >> -> Foreign Scan on public.ft1 (cost=100.00..140.38 rows=12 width=10) >> Output: (a * 2), ctid >> Remote SQL: SELECT a, ctid FROM public.mytable_1 WHERE ((a = 5)) >> FOR UPDATE >> (5 rows) >> >> I think we should show update commands somewhere for the inherited case >> as for the non-inherited case. Alternatives to this are welcome. >> > This is not exactly extension of non-inheritance case. non-inheritance > case doesn't show two remote SQLs under the same plan node. May be you can > rename the label Remote SQL as Remote UPDATE/INSERT/DELETE (or something to > that effect) for the DML command and the Foreign plan node should be > renamed to Foreign access node or something to indicate that it does both > the scan as well as DML. I am not keen about the actual terminology, but I > think a reader of plan shouldn't get confused. > > We can leave this for committer's judgement. > >> >> BTW, I was experimenting with DMLs being executed on multiple FDW server >>> under same transaction and found that the transactions may not be atomic >>> (and may be inconsistent), if one or more of the server fails to commit >>> while rest of them commit the transaction. The reason for this is, we do >>> not "rollback" the already "committed" changes to the foreign server, if >>> one or more of them fail to "commit" a transaction. With foreign tables >>> under inheritance hierarchy a single DML can span across multiple >>> servers and the result may not be atomic (and may be inconsistent). So, >>> >> >> IIUC, even the transactions over the local and the *single* remote server >> are not guaranteed to be executed atomically in the current form. It is >> possible that the remote transaction succeeds and the local one fails, for >> example, resulting in data inconsistency between the local and the remote. >> > > IIUC, while committing transactions involving a single remote server, the > steps taken are as follows > 1. the local changes are brought to PRE-COMMIT stage, which means that the > transaction *will* succeed locally after successful completion of this > phase, > 2. COMMIT message is sent to the foreign server > 3. If step two succeeds, local changes are committed and successful commit > is conveyed to the client > 4. if step two fails, local changes are rolled back and abort status is > conveyed to the client > 5. If step 1 itself fails, the remote changes are rolled back. > This is as per one phase commit protocol which guarantees ACID for single > foreign data source. So, the changes involving local and a single foreign > server seem to be atomic and consistent. > >> >> either we have to disable DMLs on an inheritance hierarchy which spans >>> multiple servers. OR make sure that such transactions follow 2PC norms. >>> >> >> -1 for disabling update queries on such an inheritance hierarchy because >> I think we should leave that to the user's judgment. And I think 2PC is >> definitely a separate patch. > > > I agree that 2pc is much larger work and is subject for separate patch/es. > But it may not be acceptable that changes made within a single command > violate atomicity and consistency, which can not be controlled or altered > by user intervention. Again, we can leave it for committer's judgement. > > Marking this as "ready for committer". > Sorry, I noticed in the commitfest app, that there are other reviewers as well. Let's wait for them to comment. > >> >> Thanks, >> >> Best regards, >> Etsuro Fujita >> > > > > -- > Best Wishes, > Ashutosh Bapat > EnterpriseDB Corporation > The Postgres Database Company > -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
