parallel bitmapscan isn't exercised in regression tests

Started by Andres Freundabout 9 years ago7 messageshackers
Jump to latest
#1Andres Freund
andres@anarazel.de

Hi,

The parallel code-path isn't actually exercised in the tests added in
[1]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=f35742ccb7aa53ee3ed8416bbb378b0c3eeb6bb9
fixed.

Greetings,

Andres Freund

[1]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=f35742ccb7aa53ee3ed8416bbb378b0c3eeb6bb9
[2]: https://coverage.postgresql.org/src/backend/executor/nodeBitmapHeapscan.c.gcov.html

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Dilip Kumar
dilipbalaut@gmail.com
In reply to: Andres Freund (#1)
Re: parallel bitmapscan isn't exercised in regression tests

On Sat, Apr 1, 2017 at 12:16 AM, Andres Freund <andres@anarazel.de> wrote:

Hi,

The parallel code-path isn't actually exercised in the tests added in
[1], as evidenced by [2] (they just explain). That imo needs to be
fixed.

Thanks for reporting. Attached patch fixes that.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Attachments:

parallel_bitmap_test.patchapplication/octet-stream; name=parallel_bitmap_test.patchDownload+7-0
#3Andres Freund
andres@anarazel.de
In reply to: Dilip Kumar (#2)
Re: parallel bitmapscan isn't exercised in regression tests

On 2017-04-01 17:23:04 +0530, Dilip Kumar wrote:

On Sat, Apr 1, 2017 at 12:16 AM, Andres Freund <andres@anarazel.de> wrote:

Hi,

The parallel code-path isn't actually exercised in the tests added in
[1], as evidenced by [2] (they just explain). That imo needs to be
fixed.

Thanks for reporting. Attached patch fixes that.

That's better than before, but I'd appreciate working on a bit more
coverage. E.g. rescans probably aren't exercised in that test, right?

If you have time & energy, it'd also be good to expand the tests to
cover the prefetching logic - it's quite bad that it's currently not
tested at all :(

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Dilip Kumar
dilipbalaut@gmail.com
In reply to: Andres Freund (#3)
Re: parallel bitmapscan isn't exercised in regression tests

On Mon, Apr 3, 2017 at 11:22 PM, Andres Freund <andres@anarazel.de> wrote:

That's better than before, but I'd appreciate working on a bit more
coverage. E.g. rescans probably aren't exercised in that test, right?

If you have time & energy, it'd also be good to expand the tests to
cover the prefetching logic - it's quite bad that it's currently not
tested at all :(

Sure I can do that, In attached patch, I only fixed the problem of not
executing the bitmap test. Now, I will add few cases to cover other
parts especially rescan and prefetching logic.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Dilip Kumar
dilipbalaut@gmail.com
In reply to: Dilip Kumar (#4)
Re: parallel bitmapscan isn't exercised in regression tests

On Tue, Apr 4, 2017 at 5:51 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:

Sure I can do that, In attached patch, I only fixed the problem of not
executing the bitmap test. Now, I will add few cases to cover other
parts especially rescan and prefetching logic.

I have added two test cases to cover rescan, prefetch and lossy pages
logic for parallel bitmap. I have removed the existing case because
these two new cases will be enough to cover that part as well.

Now, nodeBitmapHeapScan.c has 95.5% of line coverage.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Attachments:

parallel_bitmap_test_v2.patchapplication/octet-stream; name=parallel_bitmap_test_v2.patchDownload+52-8
#6Andres Freund
andres@anarazel.de
In reply to: Dilip Kumar (#5)
Re: parallel bitmapscan isn't exercised in regression tests

On 2017-04-06 10:00:32 +0530, Dilip Kumar wrote:

On Tue, Apr 4, 2017 at 5:51 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:

Sure I can do that, In attached patch, I only fixed the problem of not
executing the bitmap test. Now, I will add few cases to cover other
parts especially rescan and prefetching logic.

I have added two test cases to cover rescan, prefetch and lossy pages
logic for parallel bitmap. I have removed the existing case because
these two new cases will be enough to cover that part as well.

Now, nodeBitmapHeapScan.c has 95.5% of line coverage.

Great! Pushed.

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#6)
Re: parallel bitmapscan isn't exercised in regression tests

On 2017-04-06 13:43:55 -0700, Andres Freund wrote:

On 2017-04-06 10:00:32 +0530, Dilip Kumar wrote:

On Tue, Apr 4, 2017 at 5:51 AM, Dilip Kumar <dilipbalaut@gmail.com> wrote:

Sure I can do that, In attached patch, I only fixed the problem of not
executing the bitmap test. Now, I will add few cases to cover other
parts especially rescan and prefetching logic.

I have added two test cases to cover rescan, prefetch and lossy pages
logic for parallel bitmap. I have removed the existing case because
these two new cases will be enough to cover that part as well.

Now, nodeBitmapHeapScan.c has 95.5% of line coverage.

Great! Pushed.

At some point it might also be a good idea to compare parallel and
non-parallel results. It's obviously quite possible to break semantics
with parallelism...

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers