Add test module for Custom WAL Resource Manager feature

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]/messages/by-id/20ee0b0ae6958804a88fe9580157587720faf664.camel@j-davis.com, 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]: /messages/by-id/20ee0b0ae6958804a88fe9580157587720faf664.camel@j-davis.com
/messages/by-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]
/messages/by-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

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

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&amp;dt=2022-11-16%2000%3A40%3A11
--
Michael

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&amp;dt=2022-11-16%2000%3A40%3A11

Fixed, thank you. I'll be more diligent about pushing to github CI
first.

Regards,
Jeff Davis