Re: [HACKERS] Parallel Index Scans

On Wed, Jan 18, 2017 at 6:25 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
>
>
> On Mon, Jan 16, 2017 at 11:11 PM, Amit Kapila <amit.kapila16@gmail.com>
> wrote:
>>
>
> + * index_beginscan_parallel - join parallel index scan
>
> The name and the description doesn't sync properly, any better description?
>

This can be called by both the worker and leader of parallel index
scan.  What problem do you see with it.  heap_beginscan_parallel has
similar description, so not sure changing here alone makes sense.

>
> +extern BlockNumber _bt_parallel_seize(IndexScanDesc scan, bool *status);
> +extern void _bt_parallel_release(IndexScanDesc scan, BlockNumber
> scan_page);
>
> Any better names for the above functions, as these function will provide/set
> the next page that needs to be read.
>

These functions also set the state of scan.  IIRC, these names were
being agreed between Robert and Rahila as well (suggested offlist by
Robert).  I am open to change if you or others have any better
suggestions.

>
> + /* reset (parallel) index scan */
> + if (node->iss_ScanDesc)
> + {
>
> Why this if check required? There is an assert check in later function
> calls.
>

This is required because we don't initialize the scan descriptor for
parallel-aware nodes during ExecInitIndexScan.  It got initialized
later at the time of execution when we initialize dsm.  Now, it is
quite possible that Gather node can occur on inner side of join in
which case Rescan can be called before even execution starts. This is
the reason why we have similar check in ExecReScanSeqScan which is
added during parallel sequential scans (f0661c4e). Does that answer
your question?