Thread: Add test module for Custom WAL Resource Manager feature
Hi, Inspired by recent commits 9fcdf2c, e813e0e and many small test modules/extensions under src/test/modules, I would like to propose one such test module for Custom WAL Resource Manager feature introduced by commit 5c279a6. It not only covers the code a bit, but it also demonstrates usage of the feature. I'm attaching a patch herewith. Thoughts? Thanks Michael Paquier for an off list chat. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Attachment
On Fri, 2022-11-11 at 17:01 +0530, Bharath Rupireddy wrote:
> Hi,
>
> Inspired by recent commits 9fcdf2c, e813e0e and many small test
> modules/extensions under src/test/modules, I would like to propose
> one
> such test module for Custom WAL Resource Manager feature introduced
> by
> commit 5c279a6. It not only covers the code a bit, but it also
> demonstrates usage of the feature.
>
> I'm attaching a patch herewith. Thoughts?
Good idea. Can we take it a little further to exercise the decoding
path, as well?
For instance, we can do something like a previous proposal[1], except
it can now be done as an extension. If it's useful, we could even put
it in contrib with a real RMGR ID.
Though I'm also fine just adding a test module to start with.
Regards,
Jeff Davis
[1]
https://www.postgresql.org/message-id/20ee0b0ae6958804a88fe9580157587720faf664.camel@j-davis.com
On Sat, Nov 12, 2022 at 4:40 AM Jeff Davis <pgsql@j-davis.com> wrote: > > On Fri, 2022-11-11 at 17:01 +0530, Bharath Rupireddy wrote: > > Hi, > > > > Inspired by recent commits 9fcdf2c, e813e0e and many small test > > modules/extensions under src/test/modules, I would like to propose > > one > > such test module for Custom WAL Resource Manager feature introduced > > by > > commit 5c279a6. It not only covers the code a bit, but it also > > demonstrates usage of the feature. > > > > I'm attaching a patch herewith. Thoughts? > > Good idea. Thanks. > Can we take it a little further to exercise the decoding > path, as well? For instance, we can do something like a previous proposal[1], except > it can now be done as an extension. If it's useful, we could even put > it in contrib with a real RMGR ID. > > [1] > https://www.postgresql.org/message-id/20ee0b0ae6958804a88fe9580157587720faf664.camel@j-davis.com We have tests/modules defined for testing logical decoding, no? If the intention is to define rm_redo in this test module, I think it's not required. > Though I'm also fine just adding a test module to start with. Thanks. I would like to keep it simple. I've added some more comments and attached v2 patch herewith. Please review. I've also added a CF entry - https://commitfest.postgresql.org/41/4009/. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Attachment
On Mon, 2022-11-14 at 09:34 +0530, Bharath Rupireddy wrote: > Thanks. I would like to keep it simple. > > I've added some more comments and attached v2 patch herewith. Please > review. Committed with some significant revisions (ae168c794f): * changed to insert a deterministic message, rather than a random one, which allows more complete testing * fixed a couple bugs * used a static initializer for the RmgrData rather than memset, which shows a better example I also separately committed a patch to mark the argument of RegisterCustomRmgr as "const". -- Jeff Davis PostgreSQL Contributor Team - AWS
On Tue, Nov 15, 2022 at 04:29:08PM -0800, Jeff Davis wrote: > Committed with some significant revisions (ae168c794f): > > * changed to insert a deterministic message, rather than a random > one, which allows more complete testing > * fixed a couple bugs > * used a static initializer for the RmgrData rather than memset, > which shows a better example > > I also separately committed a patch to mark the argument of > RegisterCustomRmgr as "const". This is causing the CI job to fail for 32-bit builds. Here is one example in my own repository for what looks like an alignment issue: https://github.com/michaelpq/postgres/runs/9514121172 [01:17:23.152] ok 1 - custom WAL resource manager has successfully registered with the server [01:17:23.152] not ok 2 - custom WAL resource manager has successfully written a WAL record [01:17:23.152] 1..2 [01:17:23.152] # test failed [01:17:23.152] --- stderr --- [01:17:23.152] # Failed test 'custom WAL resource manager has successfully written a WAL record' [01:17:23.152] # at /tmp/cirrus-ci-build/src/test/modules/test_custom_rmgrs/t/001_basic.pl line 56. [01:17:23.152] # got: '0/151E088|test_custom_rmgrs|TEST_CUSTOM_RMGRS_MESSAGE|40|14|0|payload (10 bytes): payload123' [01:17:23.152] # expected: '0/151E088|test_custom_rmgrs|TEST_CUSTOM_RMGRS_MESSAGE|44|18|0|payload (10 bytes): payload123' [01:17:23.152] # Looks like you failed 1 test of 2. [01:17:23.152] Not many buildfarm members test 32b builds, but lapwing does. -- Michael
Attachment
On Wed, Nov 16, 2022 at 10:26:32AM +0900, Michael Paquier wrote: > Not many buildfarm members test 32b builds, but lapwing does. Well, it didn't take long: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing&dt=2022-11-16%2000%3A40%3A11 -- Michael
Attachment
On Wed, 2022-11-16 at 10:27 +0900, Michael Paquier wrote: > On Wed, Nov 16, 2022 at 10:26:32AM +0900, Michael Paquier wrote: > > Not many buildfarm members test 32b builds, but lapwing does. > > Well, it didn't take long: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing&dt=2022-11-16%2000%3A40%3A11 Fixed, thank you. I'll be more diligent about pushing to github CI first. Regards, Jeff Davis