Re: inherit support for foreign tables

On Tue, Dec 2, 2014 at 8:29 AM, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
wrote:

> (2014/11/28 18:14), Ashutosh Bapat wrote:
>
>> On Thu, Nov 27, 2014 at 3:52 PM, Etsuro Fujita
>> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp <mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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".

>
>
> Thanks,
>
> Best regards,
> Etsuro Fujita
>

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company