Re: [HACKERS] Parallel Index Scans
| От | Rahila Syed |
|---|---|
| Тема | Re: [HACKERS] Parallel Index Scans |
| Дата | |
| Msg-id | CAH2L28t34tBmPgtB43EE9BcdmRORZf4ESFa=9D+1evBUVeZc4Q@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: [HACKERS] Parallel Index Scans (Haribabu Kommi <kommi.haribabu@gmail.com>) |
| Ответы |
Re: [HACKERS] Parallel Index Scans
(Haribabu Kommi <kommi.haribabu@gmail.com>)
|
| Список | pgsql-hackers |
>+ /* 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?Consider following array scan keys
select * from test2 where j=ANY(ARRAY[1000,2000,3000]);
By the time the current worker has finished reading heap tuples corresponding
to array key 1000(arrayKeyCount = 0), some other worker might have advanced the scan to the
array key 2000(btps_arrayKeyCount =1). In this case when the current worker fetches next page to scan,
it must advance its scan keys before scanning the next page of parallel scan.
I hope this helps.
I hope this helps.
>+BlockNumber
>+_bt_parallel_seize(
>The return value of the above function is validated only in _bt_first
>function, but in other cases it is not.
In other cases it is validated in _bt_readnextpage() which is called after
In other cases it is validated in _bt_readnextpage() which is called after
_bt_parallel_seize().
>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?
In case of the next block being P_NONE and status true, the code
calls _bt_parallel_done() to notify other workers followed by
BTScanPosInvalidate(). Similar check for block = P_NONE also
happens in existing code. See following in _bt_readnextpage(),
if (blkno == P_NONE || !so->currPos.moreRight)
{
_bt_parallel_done(scan);
BTScanPosInvalidate(so->currPos);
return false;
}
if (blkno == P_NONE || !so->currPos.moreRight)
{
_bt_parallel_done(scan);
BTScanPosInvalidate(so->currPos);
return false;
}
So to keep it consistent with the existing code, the check
is kept outside _bt_parallel_seize().
Thank you,
Rahila Syed
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:
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 scanThe 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 lessthan btps_arrayKeyCount?+BlockNumber+_bt_parallel_seize(IndexScanDesc scan, bool *status) The return value of the above function is validated only in _bt_firstfunction, but in other cases it is not. From the function description,it is possible to return P_NONE for the workers also with status astrue. I feel it is better to handle the P_NONE case internally onlyso 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/setthe 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 findany 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 BabuFujitsu Australia
В списке pgsql-hackers по дате отправления:
Следующее
От: Kyotaro HORIGUCHIДата:
Сообщение: Re: [HACKERS] Floating point comparison inconsistencies of thegeometric types