SERIALIZABLE with parallel query

On Tue, Nov 8, 2016 at 1:51 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

Currently we don't generate parallel plans in SERIALIZABLE. What
problems need to be solved to be able to do that?

FWIW, parallel CREATE INDEX works at SERIALIZABLE isolation level by
specially asking the parallel infrastructure to not care. I think that
this works fine, given the limited scope of the problem, but it would
be nice to have that confirmed.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Nov 9, 2016 at 10:51 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

Need to audit predicate.c for cases where
MySerializableXact might be modified without suitable locking,

The only thing I see along those lines is that
CheckForSerializableConflictOut() and CheckForSerializableConflictIn()
access SxactIsDoomed(MySerializableXact) without any locking, but if
that's OK in the non-parallel case it should also be OK in a worker.
I guess this is an opportunistic early error path that doesn't mind
seeing data from the past without worrying about cache coherency, on
the basis that it will be checked again in
PreCommit_CheckForSerializationFailure().

I wonder what horrible things might happen
as a result of workers running with a SERIALIZABLEXACT that contains
the leader's vxid and other such things.

What is the consequence of that vxid? What other complications could
be involved here?

Here's a rebased patch that updates the documentation and adds a test
cast to show a serialization failure being detected when one of the
queries runs entirely in a parallel worker.

--
Thomas Munro
http://www.enterprisedb.com

On Wed, Nov 9, 2016 at 12:34 PM, Peter Geoghegan <pg@heroku.com> wrote:

On Tue, Nov 8, 2016 at 1:51 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

Currently we don't generate parallel plans in SERIALIZABLE. What
problems need to be solved to be able to do that?

FWIW, parallel CREATE INDEX works at SERIALIZABLE isolation level by
specially asking the parallel infrastructure to not care. I think that
this works fine, given the limited scope of the problem, but it would
be nice to have that confirmed.

I don't see any problem with it, but it'd be nicer to get rid of the
restriction so your change isn't needed.

--
Thomas Munro
http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Nov 8, 2016 at 4:51 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

Currently we don't generate parallel plans in SERIALIZABLE. What
problems need to be solved to be able to do that? I'm probably
steamrolling over a ton of subtleties and assumptions here, but it
occurred to me that a first step might be something like this:

1. Hand the leader's SERIALIZABLEXACT to workers.
2. Make sure that workers don't release predicate locks.
3. Make sure that the leader doesn't release predicate locks until
after the workers have finished accessing the leader's
SERIALIZABLEXACT. I think this is already the case.

What happens if the workers exit at the end of the query and the
leader then goes on and executes more queries? Will the
worker-acquired predicate locks be retained or will they go away when
the leader exits?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Feb 16, 2017 at 2:58 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Nov 8, 2016 at 4:51 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

Currently we don't generate parallel plans in SERIALIZABLE. What
problems need to be solved to be able to do that? I'm probably
steamrolling over a ton of subtleties and assumptions here, but it
occurred to me that a first step might be something like this:

1. Hand the leader's SERIALIZABLEXACT to workers.
2. Make sure that workers don't release predicate locks.
3. Make sure that the leader doesn't release predicate locks until
after the workers have finished accessing the leader's
SERIALIZABLEXACT. I think this is already the case.

What happens if the workers exit at the end of the query and the
leader then goes on and executes more queries? Will the
worker-acquired predicate locks be retained or will they go away when
the leader exits?

All predicate locks last at least until ReleasePredicateLocks() run
after ProcReleaseLocks(), and sometimes longer. Although
ReleasePredicateLocks() runs in workers too, this patch makes it
return without doing anything. I suppose someone could say that
ReleasePredicateLocks() should at least run
hash_destroy(LocalPredicateLockHash) and set LocalPredicateLockHash to
NULL in workers. This sort of thing could be important if we start
reusing worker processes. I'll do that in the next version.

The predicate locks themselves consist of state in shared memory, and
those created by workers are indistinguishable from those created by
the leader process. Having multiple workers and the leader all
creating predicate locks linked to the same SERIALIZABLEXACT is
*almost* OK, because most relevant shmem state is protected by locks
already in all paths (with the exception of the DOOMED flag already
mentioned, which seems to follow a "notice me as soon as possible"
philosophy, to avoid putting locking into the
CheckForSerializableConflict(In|Out) paths, with a definitive check at
commit time).

But... there is a problem with the management of the linked list of
predicate locks held by a transactions. The locks themselves are
covered by partition locks, but the links are not, and that previous
patch broke the assumption that they could never be changed by another
process.

Specifically, DeleteChildTargetLocks() assumes it can walk
MySerializableXact->predicateLocks and throw away locks that are
covered by a new lock (ie throw away tuple locks because a covering
page lock has been acquired) without let or hindrance until it needs
to modify the locks themselves. That assumption doesn't hold up with
that last patch and will require a new kind of mutual exclusion. I
wonder if the solution is to introduce an LWLock into each
SERIALIZABLEXACT object, so DeleteChildTargetLocks() can prevent
workers from stepping on each others' toes during lock cleanup. An
alternative would be to start taking SerializablePredicateLockListLock
in exclusive rather than shared mode, but that seems unnecessarily
course.

I have a patch that implements the above but I'm still figuring out
how to test it, and I'll need to do a bit more poking around for other
similar assumptions before I post a new version.

I tried to find any way that LocalPredicateLockHash could create
problems, but it's effectively a cache with
false-negatives-but-never-false-positives semantics. In cache-miss
scenarios it we look in shmem data structures and are prepared to find
that our SERIALIZABLEXACT already has the predicate lock even though
there was a cache miss in LocalPredicateLockHash. That works because
our SERIALIZABLEXACT's address is part of the tag, and it's stable
across backends.

Random observation: The global variable MyXactDidWrite would probably
need to move into shared memory when parallel workers eventually learn
to write.

--
Thomas Munro
http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Feb 16, 2017 at 6:19 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

Specifically, DeleteChildTargetLocks() assumes it can walk
MySerializableXact->predicateLocks and throw away locks that are
covered by a new lock (ie throw away tuple locks because a covering
page lock has been acquired) without let or hindrance until it needs
to modify the locks themselves. That assumption doesn't hold up with
that last patch and will require a new kind of mutual exclusion. I
wonder if the solution is to introduce an LWLock into each
SERIALIZABLEXACT object, so DeleteChildTargetLocks() can prevent
workers from stepping on each others' toes during lock cleanup. An
alternative would be to start taking SerializablePredicateLockListLock
in exclusive rather than shared mode, but that seems unnecessarily
coarse.

Here is a patch to do that, for discussion. It adds an LWLock to each
SERIALIZABLEXACT, and acquires it after SerializablePredicateListLock
and before any predicate lock partition lock. It doesn't bother with
that if not in parallel mode, or in the cases where
SerializablePredicateListLock is held exclusively. This prevents
parallel query workers and leader from stepping on each others' toes
when manipulating the predicate list.

The case in CheckTargetForConflictsIn is theoretical for now since we
don't support writing in parallel query yet. The case in
CreatePredicateLock is reachable by running a simple parallel
sequential scan. The case in DeleteChildTargetLocks is for when we've
acquired a new predicate lock that covers finer grained locks which
can be dropped; that is reachable the same way again. I don't think
it's required in ReleaseOneSerializableXact since it was already
called in several places with an sxact other than the caller's, and
deals with finished transactions.

--
Thomas Munro
http://www.enterprisedb.com

On Tue, Feb 21, 2017 at 12:55 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

On Thu, Feb 16, 2017 at 6:19 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

Specifically, DeleteChildTargetLocks() assumes it can walk
MySerializableXact->predicateLocks and throw away locks that are
covered by a new lock (ie throw away tuple locks because a covering
page lock has been acquired) without let or hindrance until it needs
to modify the locks themselves. That assumption doesn't hold up with
that last patch and will require a new kind of mutual exclusion. I
wonder if the solution is to introduce an LWLock into each
SERIALIZABLEXACT object, so DeleteChildTargetLocks() can prevent
workers from stepping on each others' toes during lock cleanup. An
alternative would be to start taking SerializablePredicateLockListLock
in exclusive rather than shared mode, but that seems unnecessarily
coarse.

Here is a patch to do that, for discussion. It adds an LWLock to each
SERIALIZABLEXACT, and acquires it after SerializablePredicateListLock
and before any predicate lock partition lock. It doesn't bother with
that if not in parallel mode, or in the cases where
SerializablePredicateListLock is held exclusively. This prevents
parallel query workers and leader from stepping on each others' toes
when manipulating the predicate list.

The case in CheckTargetForConflictsIn is theoretical for now since we
don't support writing in parallel query yet. The case in
CreatePredicateLock is reachable by running a simple parallel
sequential scan. The case in DeleteChildTargetLocks is for when we've
acquired a new predicate lock that covers finer grained locks which
can be dropped; that is reachable the same way again. I don't think
it's required in ReleaseOneSerializableXact since it was already
called in several places with an sxact other than the caller's, and
deals with finished transactions.

I don't think I know enough about the serializable code to review
this, or at least not quickly, but it seems very cool if it works.
Have you checked what effect it has on shared memory consumption?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Feb 22, 2017 at 2:01 PM, Robert Haas <robertmhaas@gmail.com> wrote:

I don't think I know enough about the serializable code to review
this, or at least not quickly, but it seems very cool if it works.
Have you checked what effect it has on shared memory consumption?

I'm not sure how to test that. Kevin, can you provide any pointers to
the test workloads you guys used when developing SSI? In theory shmem
usage shouldn't change, since the predicate locks are shared by the
cooperating backends. It might be able to use a bit more by creating
finer grain locks in worker A that are already covered by coarse
grained locks acquired by worker B or something like that, but it
seems unlikely if they tend to scan disjoint sets of pages.

Here is a rebased patch.

I should explain the included isolation test. It's almost the same as
the SERIALIZABLE variant that I submitted for
https://commitfest.postgresql.org/13/949/. That is a useful test here
because it's a serialisation anomaly that involves a read-only query.
In this version we run that query (s3r) in a parallel worker, and the
query plan comes out like this:

Gather (cost=1013.67..1013.87 rows=2 width=64)
Workers Planned: 1
Single Copy: true
-> Sort (cost=13.67..13.67 rows=2 width=64)
Sort Key: id
-> Bitmap Heap Scan on bank_account (cost=8.32..13.66 rows=2 width=64)
Recheck Cond: (id = ANY ('{X,Y}'::text[]))
-> Bitmap Index Scan on bank_account_pkey
(cost=0.00..8.32 rows=2 width=0)
Index Cond: (id = ANY ('{X,Y}'::text[]))

A dangerous cycle is detected, confirming that reads done by the
worker participate correctly in predicate locking and conflict
detection:

step s2wx: UPDATE bank_account SET balance = -11 WHERE id = 'X';
ERROR: could not serialize access due to read/write dependencies
among transactions

It's probably too late for this WIP patch to get the kind of review
and testing it needs for PostgreSQL 10. That's OK, but think it might
be a strategically good idea to get parallel SSI support (whether with
this or some other approach) into the tree before people start showing
up with parallel write patches.

--
Thomas Munro
http://www.enterprisedb.com

Hi,

On 2017-03-11 15:19:23 +1300, Thomas Munro wrote:

Here is a rebased patch.

It seems that this patch is still undergoing development, review and
performance evaluation. Therefore it seems like it'd be a bad idea to
try to get this into v10. Any arguments against moving this to the next
CF?

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Apr 4, 2017 at 6:41 AM, Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2017-03-11 15:19:23 +1300, Thomas Munro wrote:

Here is a rebased patch.

It seems that this patch is still undergoing development, review and
performance evaluation. Therefore it seems like it'd be a bad idea to
try to get this into v10. Any arguments against moving this to the next
CF?

None, and done.

It would be good to get some feedback from Kevin on whether this is a
reasonable approach, but considering that these data structures may
finish up being redesigned as part of the GSoC project[1]https://wiki.postgresql.org/wiki/GSoC_2017#Eliminate_O.28N.5E2.29_scaling_from_rw-conflict_tracking_in_serializable_transactions, it may be
best to wait and see where that goes before doing anything. I'll
follow developments there, and if this patch remains relevant I'll
plan to do some more work on it including testing (possibly with the
RUBiS benchmark from Kevin and Dan's paper since it seems the most
likely to be able to really use parallelism) for PG11 CF1.

[1]: https://wiki.postgresql.org/wiki/GSoC_2017#Eliminate_O.28N.5E2.29_scaling_from_rw-conflict_tracking_in_serializable_transactions

--
Thomas Munro
http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Mar 10, 2017 at 8:19 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

On Wed, Feb 22, 2017 at 2:01 PM, Robert Haas <robertmhaas@gmail.com> wrote:

I don't think I know enough about the serializable code to review
this, or at least not quickly, but it seems very cool if it works.
Have you checked what effect it has on shared memory consumption?

I'm not sure how to test that. Kevin, can you provide any pointers to
the test workloads you guys used when developing SSI?

During development there was first and foremost the set of tests
which wound up implemented in the isolation testing environment
developed by Heikki, although for most of the development cycle
these tests were run by a python tool written by Markus Wanner
(which was not as fast as Heikki's C-based tool, but provided a lot
more detail in case of failure -- so it was very nice to have until
we got down to polishing things).

The final stress test to chase out race conditions and the
performance benchmarks were all run by Dan Ports on big test
machines at MIT. I'm not sure how much I can say to elaborate on
what is in section 8 of this paper:

http://vldb.org/pvldb/vol5/p1850_danrkports_vldb2012.pdf

I seem to remember that he had some saturation run versions of the
"DBT-2++" tests, modified to monitor for serialization anomalies
missed by the implementation, which he sometimes ran for days at a
time on MIT testing machines. There were some very narrow race
conditions uncovered by this testing, which at high concurrency on a
16 core machine might hit a particular problem less often than once
per day.

I also remember that both Anssi Kääriäinen and Yamamoto Takahashi
did a lot of valuable testing of the patch and found problems that
we had missed. Perhaps they can describe their testing or make
other suggestions.

--
Kevin Grittner

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Apr 4, 2017 at 8:25 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

... but considering that these data structures may
finish up being redesigned as part of the GSoC project[1], it may be
best to wait and see where that goes before doing anything. I'll
follow developments there, and if this patch remains relevant I'll
plan to do some more work on it including testing (possibly with the
RUBiS benchmark from Kevin and Dan's paper since it seems the most
likely to be able to really use parallelism) for PG11 CF1.

I've been keeping one eye on the GSoC project. That patch changes the
inConflicts and outConflicts data structures, but not the locking
protocol. This patch works by introducing per-SERIALIZABLEXACT
locking in the places where the code currently assumes that the
current backend is the only one that could modify a shared data
structure (namely MySerializableXact->predicateLocks), so that
MySerializableXact can be shared with workers. There doesn't seem to
be any incompatibility or dependency so far, so here's a rebased
patch. Testing needed.

--
Thomas Munro
http://www.enterprisedb.com

On Wed, Jun 28, 2017 at 11:21 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

[ssi-parallel-v5.patch]

Rebased.

--
Thomas Munro
http://www.enterprisedb.com

On Fri, Sep 1, 2017 at 5:11 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

On Wed, Jun 28, 2017 at 11:21 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

[ssi-parallel-v5.patch]

Rebased.

Rebased again.

--
Thomas Munro
http://www.enterprisedb.com

On Tue, Sep 19, 2017 at 11:42 AM, Thomas Munro <
thomas.munro@enterprisedb.com> wrote:

On Fri, Sep 1, 2017 at 5:11 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

On Wed, Jun 28, 2017 at 11:21 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

[ssi-parallel-v5.patch]

Rebased.

Rebased again.

During testing of this patch, I found some behavior difference
with the support of parallel query, while experimenting with the provided
test case in the patch.

But I tested the V6 patch, and I don't think that this version contains
any fixes other than rebase.

Test steps:

CREATE TABLE bank_account (id TEXT PRIMARY KEY, balance DECIMAL NOT NULL);
INSERT INTO bank_account (id, balance) VALUES ('X', 0), ('Y', 0);

Session -1:

BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SELECT balance FROM bank_account WHERE id = 'Y';

Session -2:

BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SET max_parallel_workers_per_gather = 2;
SET force_parallel_mode = on;
set parallel_setup_cost = 0;
set parallel_tuple_cost = 0;
set min_parallel_table_scan_size = 0;
set enable_indexscan = off;
set enable_bitmapscan = off;

SELECT balance FROM bank_account WHERE id = 'X';

Session -1:

update bank_account set balance = 10 where id = 'X';

Session -2:

update bank_account set balance = 10 where id = 'Y';
ERROR: could not serialize access due to read/write dependencies among
transactions
DETAIL: Reason code: Canceled on identification as a pivot, during write.
HINT: The transaction might succeed if retried.

Without the parallel query of select statement in session-2,
the update statement in session-2 is passed.

Regards,
Hari Babu
Fujitsu Australia

On Tue, Sep 19, 2017 at 1:47 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

During testing of this patch, I found some behavior difference
with the support of parallel query, while experimenting with the provided
test case in the patch.

But I tested the V6 patch, and I don't think that this version contains
any fixes other than rebase.

Test steps:

CREATE TABLE bank_account (id TEXT PRIMARY KEY, balance DECIMAL NOT NULL);
INSERT INTO bank_account (id, balance) VALUES ('X', 0), ('Y', 0);

Session -1:

BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SELECT balance FROM bank_account WHERE id = 'Y';

Session -2:

BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SET max_parallel_workers_per_gather = 2;
SET force_parallel_mode = on;
set parallel_setup_cost = 0;
set parallel_tuple_cost = 0;
set min_parallel_table_scan_size = 0;
set enable_indexscan = off;
set enable_bitmapscan = off;

SELECT balance FROM bank_account WHERE id = 'X';

Session -1:

update bank_account set balance = 10 where id = 'X';

Session -2:

update bank_account set balance = 10 where id = 'Y';
ERROR: could not serialize access due to read/write dependencies among
transactions
DETAIL: Reason code: Canceled on identification as a pivot, during write.
HINT: The transaction might succeed if retried.

Without the parallel query of select statement in session-2,
the update statement in session-2 is passed.

Hi Haribabu,

Thanks for looking at this!

Yeah. The difference seems to be that session 2 chooses a Parallel
Seq Scan instead of an Index Scan when you flip all those GUCs into
parallelism-is-free mode. Seq Scan takes a table-level predicate lock
(see heap_beginscan_internal()). But if you continue your example in
non-parallel mode (patched or unpatched), you'll find that only one of
those transactions can commit successfully.

Using the fancy notation in the papers about this stuff where w1[x=42]
means "write by transaction 1 on object x with value 42", let's see if
there is an apparent sequential order of these transactions that makes
sense:

Actual order: r1[Y=0] r2[X=0] w1[X=10] w2[Y=10] ... some commit order ...
Apparent order A: r2[X=0] w2[Y=10] c2 r1[Y=0*] w1[X=10] c1 (*nonsense)
Apparent order B: r1[Y=0] w1[X=10] c1 r2[X=0*] w2[Y=10] c2 (*nonsense)

Both potential commit orders are nonsensical. I think what happened
in your example was that a Seq Scan allowed the SSI algorithm to
reject a transaction sooner. Instead of r2[X=0], the executor sees
r2[X=0,Y=0] (we scanned the whole table, as if we read all objects, in
this case X and Y, even though we only asked to read X). Then the SSI
algorithm is able to detect a "dangerous structure" at w2[Y=10],
instead of later at commit time.

So I don't think this indicates a problem with the patch.

--
Thomas Munro
http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Sep 21, 2017 at 4:13 PM, Thomas Munro <thomas.munro@enterprisedb.com

wrote:

On Tue, Sep 19, 2017 at 1:47 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

During testing of this patch, I found some behavior difference
with the support of parallel query, while experimenting with the provided
test case in the patch.

But I tested the V6 patch, and I don't think that this version contains
any fixes other than rebase.

Test steps:

CREATE TABLE bank_account (id TEXT PRIMARY KEY, balance DECIMAL NOT

NULL);

INSERT INTO bank_account (id, balance) VALUES ('X', 0), ('Y', 0);

Session -1:

BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SELECT balance FROM bank_account WHERE id = 'Y';

Session -2:

BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SET max_parallel_workers_per_gather = 2;
SET force_parallel_mode = on;
set parallel_setup_cost = 0;
set parallel_tuple_cost = 0;
set min_parallel_table_scan_size = 0;
set enable_indexscan = off;
set enable_bitmapscan = off;

SELECT balance FROM bank_account WHERE id = 'X';

Session -1:

update bank_account set balance = 10 where id = 'X';

Session -2:

update bank_account set balance = 10 where id = 'Y';
ERROR: could not serialize access due to read/write dependencies among
transactions
DETAIL: Reason code: Canceled on identification as a pivot, during

write.

HINT: The transaction might succeed if retried.

Without the parallel query of select statement in session-2,
the update statement in session-2 is passed.

Hi Thomas,

Yeah. The difference seems to be that session 2 chooses a Parallel
Seq Scan instead of an Index Scan when you flip all those GUCs into
parallelism-is-free mode. Seq Scan takes a table-level predicate lock
(see heap_beginscan_internal()). But if you continue your example in
non-parallel mode (patched or unpatched), you'll find that only one of
those transactions can commit successfully.

Yes, That's correct. Only one commit can be successful.

Using the fancy notation in the papers about this stuff where w1[x=42]
means "write by transaction 1 on object x with value 42", let's see if
there is an apparent sequential order of these transactions that makes
sense:

Actual order: r1[Y=0] r2[X=0] w1[X=10] w2[Y=10] ... some commit order ...
Apparent order A: r2[X=0] w2[Y=10] c2 r1[Y=0*] w1[X=10] c1 (*nonsense)
Apparent order B: r1[Y=0] w1[X=10] c1 r2[X=0*] w2[Y=10] c2 (*nonsense)

Both potential commit orders are nonsensical. I think what happened
in your example was that a Seq Scan allowed the SSI algorithm to
reject a transaction sooner. Instead of r2[X=0], the executor sees
r2[X=0,Y=0] (we scanned the whole table, as if we read all objects, in
this case X and Y, even though we only asked to read X). Then the SSI
algorithm is able to detect a "dangerous structure" at w2[Y=10],
instead of later at commit time.

Thanks for explaining with more details, now I can understand some more
about serialization.

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.

I am just thinking that with parallel sequential scan with serialize
isolation,
the user has lost the control of committing the desired session. I may be
thinking
a rare and never happen scenario.

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

Regards,
Hari Babu
Fujitsu Australia

On Mon, Sep 25, 2017 at 8:37 PM, Haribabu Kommi
<kommi.haribabu@gmail.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.

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

Thanks!

--
Thomas Munro
http://www.enterprisedb.com

On Mon, Sep 25, 2017 at 6:57 PM, Thomas Munro <thomas.munro@enterprisedb.com

wrote:

On Mon, Sep 25, 2017 at 8:37 PM, Haribabu Kommi
<kommi.haribabu@gmail.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