Thread: Why do parallel scans require syncscans (but not really)?

Why do parallel scans require syncscans (but not really)?

From
Tomas Vondra
Date:
Hi,

While investigating something, I noticed it's not really possible to
disable syncscans for a particular parallel scan.

That is, with serial scans, we can do this:

    table_index_build_scan(heap, index, indexInfo, false, true,
                           callback, state, NULL);

where false means "allow_sync=false", i.e. disabling synchronized scans.

However, if this is tied to a parallel scan, that's not possible. If you
try doing this:

    table_index_build_scan(heap, index, indexInfo, false, true,
                           callback, state, parallel_scan);

it hits this code in heapam_index_build_range_scan()

  /*
   * Parallel index build.
   *
   * Parallel case never registers/unregisters own snapshot.  Snapshot
   * is taken from parallel heap scan, and is SnapshotAny or an MVCC
   * snapshot, based on same criteria as serial case.
   */
  Assert(!IsBootstrapProcessingMode());
  Assert(allow_sync);
  snapshot = scan->rs_snapshot;

Sadly, there's no explanation why parallel scans do not allow disabling
sync scans just like serial scans - and it's not quite obvious to me.

Just removing the assert is not sufficient to make this work, though.
Because just a little later we call heap_setscanlimits() which does:

  Assert(!scan->rs_inited); /* else too late to change */
  /* else rs_startblock is significant */
  Assert(!(scan->rs_base.rs_flags & SO_ALLOW_SYNC));

I don't quite understand what the comments for the asserts say, but
SO_ALLOW_SYNC seems to come from table_beginscan_parallel.

Perhaps the goal is to prevent mismatch between the parallel and serial
parts of the scans - OK, that probably makes sense. But I still don't
understand why table_beginscan_parallel forces the SO_ALLOW_SYNC instead
of having an allow_sync parameters too.

Is there a reason why all parallel plans should / must be synchronized?

Well, in fact they are not *required* because if I set

  synchronize_seqscans=off

this makes the initscan() to remove the SO_ALLOW_SYNC ...


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: Why do parallel scans require syncscans (but not really)?

From
Zhang Mingli
Date:
Hi,


Zhang Mingli
www.hashdata.xyz

Hi, Tomas
On Dec 31, 2023 at 07:10 +0800, Tomas Vondra <tomas.vondra@enterprisedb.com>, wrote:
Sadly, there's no explanation why parallel scans do not allow disabling
sync scans just like serial scans - and it's not quite obvious to me.

Feel confused too.

```
Assert(!IsBootstrapProcessingMode());
  Assert(allow_sync);
  snapshot = scan->rs_snapshot;
```

I dig this for a while and found that there a close relationship between field phs_syncscan(For parallel scan) and the allow_sync flag.

In table_block_parallelscan_initialize() ParallelTableScanDescData.phs_syncscan is set.

/* compare phs_syncscan initialization to similar logic in initscan */
bpscan->base.phs_syncscan = synchronize_seqscans &&
!RelationUsesLocalBuffers(rel) &&
bpscan->phs_nblocks > NBuffers / 4;

And the allow_sync is always set to true in initscan(), when phs_syncscan is not NULL.

if (scan->rs_base.rs_parallel != NULL)
{
/* For parallel scan, believe whatever ParallelTableScanDesc says. */
if (scan->rs_base.rs_parallel->phs_syncscan)
scan->rs_base.rs_flags |= SO_ALLOW_SYNC;
else
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
}

And phs_syncscan is used in table_block_parallelscan_startblock_init(),table_block_parallelscan_nextpage() to do sth of syncscan.

Back to the Assertion, else branch means param scan(parallel scan desc) is not null and we are in parallel scan mode.
else
{
/*
* Parallel index build.
*
* Parallel case never registers/unregisters own snapshot.  Snapshot
* is taken from parallel heap scan, and is SnapshotAny or an MVCC
* snapshot, based on same criteria as serial case.
*/
Assert(!IsBootstrapProcessingMode());
Assert(allow_sync);
snapshot = scan->rs_snapshot;
}

Agree with you that: why all parallel plans should / must be synchronized?
Parallel scan should have a choice about syncscan.
Besides that I think there is a risk Assert(allow_sync), at least should use phs_syncscan field  here to judge if allow_sync is true according to above.
So I guess, there should be an Assertion failure  of Assert(allow_sync) in theory when we use a parallel scan but phs_syncscan is false.

/* compare phs_syncscan initialization to similar logic in initscan */
bpscan->base.phs_syncscan = synchronize_seqscans &&
!RelationUsesLocalBuffers(rel) &&
bpscan->phs_nblocks > NBuffers / 4;

However, I didn’t produce it because phs_syncscan is set according to data size, even with some parallel cost GUCs set to 0. 
And if there is not enough data, we usually do not choose a parallel plan,
like this case: build a index with parallel scan on underlying tables.