Pull 99% of walproposer code into extension. #188

MMeent · 2022-08-05T10:47:13Z

There are now only a few traces of walproposer left in Community PostgreSQL, with the main ones that I can think of being the following:

the backpressure hook is now a proper hook
some initialization code remains for --sync-safekeepers

Apart from that, the entry point of this extension has been moved into neon.c, and significant amounts of code were copied from walsender.c into our extension, then modified to only contain the walsender paths. Most of this copy-edited code now resides in walproposer_utils.c.

…arning.

petuhovskiy

Looks good! I wonder should we copy ProcessStandbyReply and ProcessStandbyHSFeedback to walproposer_utils also?

contrib/neon/libpqwalproposer/Makefile

contrib/neon/walproposer.c

contrib/neon/walproposer_utils.c

- Include neon extensions into contrib Makefile - Configure libpqwalproposer more like other extensions

The function is instead available in walpropcompat.c, which is the authoritative version.

contrib/neon/walproposer_utils.c

…tic again.

kelvich · 2022-08-08T14:56:38Z

I think with the extension, there is no need to have a separate build for libpagestore -- it could be just a .c file in the extension root path.

MMeent · 2022-08-08T15:19:42Z

I think with the extension, there is no need to have a separate build for libpagestore -- it could be just a .c file in the extension root path.

I'm not sure about that -- we need to build against libpq which has #define FRONTEND, which isn't compatible with many other header files. That's also the reason why it's a separate build.

kelvich · 2022-08-08T19:07:13Z

I'm not sure about that -- we need to build against libpq which has #define FRONTEND, which isn't compatible with many other header files. That's also the reason why it's a separate build.

libpq avoidance in the backend is more of a deliberate decision, not that much of incompatibility. E.g. works well for these:

https://github.com/neondatabase/postgres/blob/main/contrib/dblink/dblink.c#L50
https://github.com/neondatabase/postgres/blob/main/contrib/postgres_fdw/connection.c#L407

…ser into vendor/neon

libpqwalproposer is not a separate binary anymore, so don't try to treat it as such.

contrib/neon/Makefile

MMeent · 2022-08-09T16:25:27Z

I think I've addressed all comments, and tests are green (up to the previous commit at least, with last commit only removing an outdated comment).

Feel free to merge when appropriate - I'll be OOO until the 17th.

arssher

I don't mind merging as is, though now a lot of code in walproposer_utils.c is just coped from walsender.c and large part of it can be removed/cleaned up.

src/backend/storage/ipc/ipci.c

hlinnaka · 2022-08-15T07:27:01Z

contrib/neon/pagestore_smgr.c

@@ -1336,7 +1336,7 @@ zenith_nblocks(SMgrRelation reln, ForkNumber forknum)
 /*
 *	zenith_db_size() -- Get the size of the database in bytes.
 */
-int64
+const int64


What does it mean for a function to be const?

I don't know - it was required for signature checks and was pre-existing in the signature for the db_size hook - I didn't want to change this annotation without knowing why it was annotated.

contrib/neon/neon.h

src/include/replication/walsender.h

src/include/replication/walpropshim.h

…n-extension # Conflicts: # src/backend/replication/walsender.c # src/include/replication/walsender.h

* Pull 99% of walproposer code into extension. * Annotate nbytes to show it's used for asserts only, fixing one more warning. * Fix makefiles: - Include neon extensions into contrib Makefile - Configure libpqwalproposer more like other extensions * Add comment about lack of PG timelines, and make StartReplication static again. * Fix some compiler warnings in vendor/postgres, and pull libpqwalproposer into vendor/neon * Fix issue with makefile that didn't get caught in the normal test envs.

MMeent added 3 commits August 5, 2022 12:31

Pull 99% of walproposer code into extension.

2f9a020

Fix unused variable warnings in walproposer_utils.c

88a4089

Annotate nbytes to show it's used for asserts only, fixing one more w…

25cca06

…arning.

petuhovskiy reviewed Aug 5, 2022

View reviewed changes

contrib/neon/libpqwalproposer/Makefile Outdated Show resolved Hide resolved

contrib/neon/walproposer.c Outdated Show resolved Hide resolved

contrib/neon/walproposer_utils.c Show resolved Hide resolved

petuhovskiy mentioned this pull request Aug 5, 2022

Extract WalProposer into the neon extension neondatabase/neon#2217

Merged

MMeent added 2 commits August 5, 2022 21:51

Fix makefiles:

aaf7df4

- Include neon extensions into contrib Makefile - Configure libpqwalproposer more like other extensions

Cleanup unused function WalProposerSync from walproposer.c

6e04347

The function is instead available in walpropcompat.c, which is the authoritative version.

lubennikovaav reviewed Aug 8, 2022

View reviewed changes

contrib/neon/walproposer_utils.c Outdated Show resolved Hide resolved

contrib/neon/walproposer_utils.c Show resolved Hide resolved

Add comment about lack of PG timelines, and make StartReplication sta…

73092bb

…tic again.

Fix some compiler warnings in vendor/postgres, and pull libpqwalpropo…

7996665

…ser into vendor/neon

MMeent force-pushed the feat/walproposer-as-an-extension branch from 5306ec7 to 7996665 Compare August 9, 2022 12:32

MMeent requested review from lubennikovaav and petuhovskiy August 9, 2022 12:40

petuhovskiy approved these changes Aug 9, 2022

View reviewed changes

MMeent added 3 commits August 9, 2022 14:56

Directly initialize libpqwalproposer

8cb526b

libpqwalproposer is not a separate binary anymore, so don't try to treat it as such.

Remove initialization

5de7cfd

Fix issue with makefile that didn't get caught in the normal test envs.

aba4c5e

kelvich reviewed Aug 9, 2022

View reviewed changes

contrib/neon/Makefile Show resolved Hide resolved

Remove now unapplicable warning

691f6a6

MMeent requested a review from kelvich August 9, 2022 16:25

kelvich approved these changes Aug 9, 2022

View reviewed changes

arssher approved these changes Aug 11, 2022

View reviewed changes

src/backend/storage/ipc/ipci.c Outdated Show resolved Hide resolved

hlinnaka reviewed Aug 15, 2022

View reviewed changes

MMeent added 2 commits August 18, 2022 10:17

Various cleanups

a973385

Merge remote-tracking branch 'origin/main' into feat/walproposer-as-a…

172ea61

…n-extension # Conflicts: # src/backend/replication/walsender.c # src/include/replication/walsender.h

MMeent merged commit 7e32bba into main Aug 18, 2022

MMeent deleted the feat/walproposer-as-an-extension branch August 18, 2022 13:50

petuhovskiy mentioned this pull request Aug 19, 2022

Move WALProposer into an extension neondatabase/neon#2157

Closed

bayandin mentioned this pull request Sep 7, 2022

Pass COPT and PG_CFLAGS to Extension's CFLAGS neondatabase/neon#2405

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pull 99% of walproposer code into extension. #188

Pull 99% of walproposer code into extension. #188

MMeent commented Aug 5, 2022

petuhovskiy left a comment

kelvich commented Aug 8, 2022

MMeent commented Aug 8, 2022

kelvich commented Aug 8, 2022

MMeent commented Aug 9, 2022

arssher left a comment

hlinnaka Aug 15, 2022

MMeent Aug 18, 2022

Pull 99% of walproposer code into extension. #188

Pull 99% of walproposer code into extension. #188

Conversation

MMeent commented Aug 5, 2022

petuhovskiy left a comment

Choose a reason for hiding this comment

kelvich commented Aug 8, 2022

MMeent commented Aug 8, 2022

kelvich commented Aug 8, 2022

MMeent commented Aug 9, 2022

arssher left a comment

Choose a reason for hiding this comment

hlinnaka Aug 15, 2022

Choose a reason for hiding this comment

MMeent Aug 18, 2022

Choose a reason for hiding this comment