Re: [HACKERS] Parallel Index Scans

Поиск
Список
Период
Сортировка
От Haribabu Kommi
Тема Re: [HACKERS] Parallel Index Scans
Дата
Msg-id CAJrrPGdHxbOFNkia0qvK9b_PREgnSs6C3pUvrg9hzEz9Dv9ipw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [HACKERS] Parallel Index Scans  (Amit Kapila <amit.kapila16@gmail.com>)
Ответы Re: [HACKERS] Parallel Index Scans  (Rahila Syed <rahilasyed90@gmail.com>)
Re: [HACKERS] Parallel Index Scans  (Amit Kapila <amit.kapila16@gmail.com>)
Re: [HACKERS] Parallel Index Scans  (Amit Kapila <amit.kapila16@gmail.com>)
Список pgsql-hackers
Дерево обсуждения


On Mon, Jan 16, 2017 at 11:11 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:

Changed as per suggestion.


I have also rebased the optimizer/executor support patch
(parallel_index_opt_exec_support_v4.patch) and added a test case in
it.

Thanks for the patch. Here are comments found during review.

parallel_index_scan_v4.patch:


+ amtarget = (char *) ((void *) target) + offset;

The above calcuation can be moved after NULL check?

+ * index_beginscan_parallel - join parallel index scan

The name and the description doesn't sync properly, any better description?

+ BTPARALLEL_DONE,
+ BTPARALLEL_IDLE
+} PS_State;

The order of above two enum values can be changed according to their use.

+ /* Check if the scan for current scan keys is finished */
+ if (so->arrayKeyCount < btscan->btps_arrayKeyCount)
+ *status = false;

I didn't clearly understand, in which scenario the arrayKeyCount is less
than btps_arrayKeyCount?


+BlockNumber
+_bt_parallel_seize(IndexScanDesc scan, bool *status)

The return value of the above function is validated only in _bt_first
function, but in other cases it is not. From the function description,
it is possible to return P_NONE for the workers also with status as
true. I feel it is better to handle the P_NONE case internally only
so that callers just check for the status. Am i missing anything?


+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.


parallel_index_opt_exec_support_v4.patch:

+#include "access/parallel.h"

Why is it required to be include nbtree.c? i didn't find
any code changes in the patch.


+ /* reset (parallel) index scan */
+ if (node->iss_ScanDesc)
+ {

Why this if check required? There is an assert check in later function calls.


Regards,
Hari Babu
Fujitsu Australia

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tomas Vondra
Дата:
Сообщение: Re: [HACKERS] PoC: Grouped base relation
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: [HACKERS] Re: Clarifying "server starting" messaging in pg_ctlstart without --wait