On Wed, Jan 18, 2017 at 8:03 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Tue, Jan 17, 2017 at 11:27 PM, Robert Haas <robertmh...@gmail.com> wrote: >> On Mon, Jan 16, 2017 at 7:11 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: >> WAIT_EVENT_PARALLEL_INDEX_SCAN is in fact btree-specific. There's no >> guarantee that any other AMs the implement parallel index scans will >> use that wait event, and they might have others instead. I would make >> it a lot more specific, like WAIT_EVENT_BTREE_PAGENUMBER. (Waiting >> for the page number needed to continue a parallel btree scan to become >> available.) > > WAIT_EVENT_BTREE_PAGENUMBER - NUMBER sounds slightly inconvenient. How > about just WAIT_EVENT_BTREE_PAGE? We can keep the description as > suggested by you?
Sure. >> Why do we need separate AM support for index_parallelrescan()? Why >> can't index_rescan() cover that case? > > The reason is that sometime index_rescan is called when we have to > just update runtime scankeys in index and we don't want to reset > parallel scan for that. Refer ExecReScanIndexScan() changes in patch > parallel_index_opt_exec_support_v4. Rescan is called from below place > during index scan. Hmm, tricky. OK, I'll think about that some more. >> I think it's a bad idea to add a ParallelIndexScanDesc argument to >> index_beginscan(). That function is used in lots of places, and >> somebody might think that they are allowed to actually pass a non-NULL >> value there, which they aren't: they must go through >> index_beginscan_parallel. I think that the additional argument should >> only be added to index_beginscan_internal, and >> index_beginscan_parallel should remain unchanged. > > If we go that way then we need to set few parameters like heapRelation > and xs_snapshot in index_beginscan_parallel as we are doing in > index_beginscan. Does going that way sound better to you? It's pretty minor code duplication; I don't think it's an issue. > I think there is value in retaining index_beginscan_parallel as that > is parallel to heap_beginscan_parallel. OK. -- 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
