On 2017-08-16 16:20:28 +0300, Heikki Linnakangas wrote:
> On 05/06/2017 04:57 PM, David Rowley wrote:
> > Andres mentioned in [2] that it might be worth exploring using atomics
> > to do the same job. So I went ahead and did that, and came up with the
> > attached, which is a slight variation on what he mentioned in the
> > thread.
> > 
> > To keep things a bit more simple, and streamline, I ended up pulling
> > out the logic for setting the startblock into another function, which
> > we only call once before the first call to
> > heap_parallelscan_nextpage().  I also ended up changing phs_cblock and
> > replacing it with a counter that always starts at zero. The actual
> > block is calculated based on that + the startblock modulo nblocks.
> > This makes things a good bit more simple for detecting when we've
> > allocated all the blocks to the workers, and also works nicely when
> > wrapping back to the start of a relation when we started somewhere in
> > the middle due to piggybacking with a synchronous scan.

> Looks reasonable. I edited the comments and the variable names a bit, to my
> liking, and committed. Thanks!

Brief post-commit review:

+        * phs_nallocated tracks how many pages have been allocated to workers
+        * already.  When phs_nallocated >= rs_nblocks, all blocks have been
+        * allocated.

allocated seems a bit of a confusing terminology.


@@ -1635,8 +1637,8 @@ heap_parallelscan_initialize(ParallelHeapScanDesc target, 
Relation relation,
                !RelationUsesLocalBuffers(relation) &&
                target->phs_nblocks > NBuffers / 4;
        SpinLockInit(&target->phs_mutex);
-       target->phs_cblock = InvalidBlockNumber;
        target->phs_startblock = InvalidBlockNumber;
+       pg_atomic_write_u64(&target->phs_nallocated, 0);
        SerializeSnapshot(snapshot, target->phs_snapshot_data);
 }

It's not ok to initialize an atomic with pg_atomic_write_u64 - works
well enough for "plain" atomics, but the fallback implementation isn't
ok with it. You're probably going to get a failure on the respective
buildfarm animal soon.


- Andres


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

Reply via email to