Re: [HACKERS] SERIALIZABLE with parallel query
On Mon, Sep 25, 2017 at 6:57 PM, Thomas Munrowrote: > On Mon, Sep 25, 2017 at 8:37 PM, Haribabu Kommi > 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Mon, Sep 25, 2017 at 8:37 PM, Haribabu Kommiwrote: > 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 ssi-parallel-v8.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SERIALIZABLE with parallel query
On Thu, Sep 21, 2017 at 4:13 PM, Thomas Munrowrote: > On Tue, Sep 19, 2017 at 1:47 PM, Haribabu Kommi > 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Tue, Sep 19, 2017 at 1:47 PM, Haribabu Kommiwrote: > 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Tue, Sep 19, 2017 at 11:42 AM, Thomas Munro < thomas.mu...@enterprisedb.com> wrote: > On Fri, Sep 1, 2017 at 5:11 PM, Thomas Munro >wrote: > > On Wed, Jun 28, 2017 at 11:21 AM, Thomas Munro > > 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Fri, Sep 1, 2017 at 5:11 PM, Thomas Munrowrote: > On Wed, Jun 28, 2017 at 11:21 AM, Thomas Munro > wrote: >> [ssi-parallel-v5.patch] > > Rebased. Rebased again. -- Thomas Munro http://www.enterprisedb.com ssi-parallel-v7.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SERIALIZABLE with parallel query
On Wed, Jun 28, 2017 at 11:21 AM, Thomas Munrowrote: > [ssi-parallel-v5.patch] Rebased. -- Thomas Munro http://www.enterprisedb.com ssi-parallel-v6.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SERIALIZABLE with parallel query
On Tue, Apr 4, 2017 at 8:25 AM, Thomas Munrowrote: > ... 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 ssi-parallel-v5.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SERIALIZABLE with parallel query
On Fri, Mar 10, 2017 at 8:19 PM, Thomas Munrowrote: > On Wed, Feb 22, 2017 at 2:01 PM, Robert Haas 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Tue, Apr 4, 2017 at 6:41 AM, Andres Freundwrote: > 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], 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
Re: [HACKERS] SERIALIZABLE with parallel query
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
Re: [HACKERS] SERIALIZABLE with parallel query
On Wed, Feb 22, 2017 at 2:01 PM, Robert Haaswrote: > 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 ssi-parallel-v4.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SERIALIZABLE with parallel query
On Tue, Feb 21, 2017 at 12:55 AM, Thomas Munrowrote: > On Thu, Feb 16, 2017 at 6:19 PM, Thomas Munro > 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Thu, Feb 16, 2017 at 6:19 PM, Thomas Munrowrote: > 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 ssi-parallel-v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SERIALIZABLE with parallel query
On Thu, Feb 16, 2017 at 2:58 AM, Robert Haaswrote: > On Tue, Nov 8, 2016 at 4:51 PM, Thomas Munro > 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Tue, Nov 8, 2016 at 4:51 PM, Thomas Munrowrote: > 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Wed, Nov 9, 2016 at 12:34 PM, Peter Geogheganwrote: > On Tue, Nov 8, 2016 at 1:51 PM, Thomas Munro > 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
Re: [HACKERS] SERIALIZABLE with parallel query
On Wed, Nov 9, 2016 at 10:51 AM, Thomas Munrowrote: > 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 ssi-parallel-v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SERIALIZABLE with parallel query
On Tue, Nov 8, 2016 at 1:51 PM, Thomas Munrowrote: > 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