Re: SERIALIZABLE with parallel query

From: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
To: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: SERIALIZABLE with parallel query
Date: 2017-09-26 06:41:19
Message-ID: CAJrrPGfK-=oRdBf_TWL91nCYJ3j6g_aM_PRsGt=JOpYFCHiUgg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 25, 2017 at 6:57 PM, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com
> wrote:

> On Mon, Sep 25, 2017 at 8:37 PM, Haribabu Kommi
> <kommi(dot)haribabu(at)gmail(dot)com> wrote:
> > After I tune the GUC to go with sequence scan, still I am not getting the
> > error
> > in the session-2 for update operation like it used to generate an error
> for
> > parallel
> > sequential scan, and also it even takes some many commands until unless
> the
> > S1
> > commits.
>
> Hmm. Then this requires more explanation because I don't expect a
> difference. I did some digging and realised that the error detail
> message "Reason code: Canceled on identification as a pivot, during
> write." was reached in a code path that requires
> SxactIsPrepared(writer) and also MySerializableXact == writer, which
> means that the process believes it is committing. Clearly something
> is wrong. After some more digging I realised that
> ParallelWorkerMain() calls EndParallelWorkerTransaction() which calls
> CommitTransaction() which calls
> PreCommit_CheckForSerializationFailure(). Since the worker is
> connected to the leader's SERIALIZABLEXACT, that finishes up being
> marked as preparing to commit (not true!), and then the leader get
> confused during that write, causing a serialization failure to be
> raised sooner (though I can't explain why it should be raised then
> anyway, but that's another topic). Oops. I think the fix here is
> just not to do that in a worker (the worker's CommitTransaction()
> doesn't really mean what it says).
>
> Here's a version with a change that makes that conditional. This way
> your test case behaves the same as non-parallel mode.
>

With the latest patch, I didn't find any problems.

> > I will continue my review on the latest patch and share any updates.
>
> Thanks!

The patch looks good, and I don't have any comments for the code.
The test that is going to add by the patch is not generating a true
parallelism scenario, I feel it is better to change the test that can
generate a parallel sequence/index/bitmap scan.

Regards,
Hari Babu
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haribabu Kommi 2017-09-26 06:42:27 Re: visual studio 2017 build support
Previous Message Alvaro Hernandez 2017-09-26 06:08:07 Re: Built-in plugin for logical decoding output