On Thu, Nov 7, 2024 at 5:44 AM Masahiro Ikeda <ikeda...@oss.nttdata.com> wrote:0-12 08:29, Peter Geoghegan wrote: > > On Thu, Oct 10, 2024 at 1:41 PM Peter Geoghegan <p...@bowt.ie> wrote: > > * We now reset currPos state (including even its moreLeft/moreRight > > fields) within _bt_parallel_seize, automatically and regardless of any > > other details. > > IIUC, the above change is the root cause. The commit 1bd4bc8 adds a > reset of > the currPos state in _bt_parallel_seize(). However, this change can > overwrite > currPos.moreRight which should be preserved before calling > _bt_readnextpage().
I can easily recreate the problem using your test case. I agree with your diagnosis. Thanks for the report! We could fix this by going back to how things were before -- don't unset moreLeft/moreRight within _bt_parallel_seize. But that doesn't seem like a good idea to me. It would be weird to depend on the currPos state from the last _bt_readpage call, even after seizing the scan for the next page in line. If the scan has already finished (according to moreLeft/moreRight), then why even try to seize the scan once again? Why not just end the scan instead (including its call to _bt_parallel_done)? > * Test case > -- Without commit 1bd4bc8, > the number was '236' in my environment. > Planning Time: 0.105 ms > Execution Time: 71.454 ms > (18 rows) With the attached patch, I get back to the good/correct behavior. I'm not sure that this is the exact right way to fix the bug, but it seems like the right general approach. -- Peter Geoghegan
v1-0001-Fix-confusion-with-nbtree-parallel-scan-currPos-s.patch
Description: Binary data
