Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/replicas #3793

Merged
merged 26 commits into from
Apr 27, 2023
Merged

Feature/replicas #3793

merged 26 commits into from
Apr 27, 2023

Conversation

MMeent
Copy link
Contributor

@MMeent MMeent commented Mar 10, 2023 •
edited
Loading

Describe your changes

Enable hot standby PostgreSQL replicas.

Notes:

  • This still needs UI support from the Console
  • I've not tuned any GUCs for PostgreSQL to make this work better
  • Safekeeper has gotten a tweak in which WAL is sent and how: It now sends zero-ed data from the start of the timeline's first segment up to the first byte of the timeline to be compatible with normal PostgreSQL WAL streaming.
  • This includes the commits of Add new neon_local option to spin up read-only replica. #3714

TODO:

Issue ticket number and link

#769

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
  • If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.
    Neon, and Neon's flavor of PostgreSQL, now support physical replicas.
    XXXX: This doens't mean cloud users can use this immediately, see the first note as well.

@MMeent MMeent requested review from a team as code owners March 10, 2023 14:11
@MMeent MMeent requested review from funbringer, arssher and hlinnaka and removed request for a team March 10, 2023 14:11
@MMeent MMeent force-pushed the feature/replicas branch from 8447c9a to 366c854 Compare March 10, 2023 14:25
@hlinnaka
Copy link
Contributor

Please add some regression tests, something under test_regress/regress/

@MMeent MMeent force-pushed the feature/replicas branch 3 times, most recently from d4ced80 to fbf664a Compare March 13, 2023 13:14
arssher
arssher reviewed Mar 13, 2023
View reviewed changes
Copy link
Contributor

@arssher arssher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked mostly at safekeeper and cplane changes.

control_plane/src/compute.rs Outdated Show resolved Hide resolved
safekeeper/src/wal_storage.rs Show resolved Hide resolved
@MMeent MMeent force-pushed the feature/replicas branch 2 times, most recently from b8fd4c8 to 58d5e08 Compare March 24, 2023 17:29
@MMeent MMeent requested review from hlinnaka and arssher March 27, 2023 15:10
arssher
arssher reviewed Mar 28, 2023
View reviewed changes
safekeeper/src/wal_storage.rs Show resolved Hide resolved
safekeeper/src/wal_storage.rs Show resolved Hide resolved
libs/postgres_ffi/src/xlog_utils.rs Outdated Show resolved Hide resolved
@arssher arssher mentioned this pull request Mar 28, 2023
Closed
lubennikovaav
lubennikovaav reviewed Mar 29, 2023
View reviewed changes
test_runner/regress/test_hot_standby.py Outdated Show resolved Hide resolved
test_runner/regress/test_hot_standby.py Show resolved Hide resolved
test_runner/regress/test_hot_standby.py Show resolved Hide resolved
pgxn/neon/pagestore_smgr.c Outdated Show resolved Hide resolved
@MMeent MMeent requested a review from a team as a code owner April 4, 2023 12:03
@MMeent MMeent requested review from chaporgin and removed request for a team April 4, 2023 12:03
MMeent added 14 commits April 13, 2023 17:37
@MMeent
Fixing issues w.r.t. replication, adding comments
80f96cc
@MMeent
Lint fixes, correctness fixes
8df1ebd
@MMeent
Lint fixes; fixing mypy pedantics
49e0c7e
@MMeent
fix test
f41f2e1
@MMeent
Fix issue in neon_get_request_lsn
b7d64c4 
While in recovery, we cannot always use the last replayed record's
LSN. Instead, we use the LwLSN cache as usual, but mark *latest as
false because we don't yet know the changes in future WAL.

It also adds some comments / documentation on the special handling
required for REDO block IO and locking.
@MMeent
Update comments and fix issues
ba03e14 
- Test updated to reliably detect hot standby (as opposed to only read-only transactions, which also appear on primary nodes)
- Update bail!() message in WAL send stream constructor
- Fix handling of hot standby responses from Safekeepers in WalSender
- Update comment on neon_redo_read_buffer_filter
- Fix generated WAL page headers for uninitialized WAL in packed WAL segments to use CONTRECORD bits
- Fix typo in control_plane/compute.rs
- Update compute_ctl: emit standby.signal for hot stanby instances (detected through `hot_stanby = on|true|yes`)
- Update compute_ctl: use 0/0 or given LSN (instead of sync_safekeepers) when it detects we're not starting a primary instance.
@MMeent
Lint fixes
0baac04
@MMeent
More lint fixes
36e86ba
@MMeent
Revert makefile changes
b7b5a6e
@MMeent
More lint fixes
b49c49b
@MMeent
Test fixes
3a66498
@MMeent
More test fixes
2d382e3 
It's quite bad to think hot standbys should support CREATE TABLE without issues.
@MMeent
Remove unused function from xlog utils
0ba5c1b 
We don't generate separate WAL blocks anymore, so let's not keep the function around.
@MMeent
Fixups from rebasing
471a269
@MMeent MMeent force-pushed the feature/replicas branch from 93db511 to 471a269 Compare April 13, 2023 18:29
@MMeent MMeent force-pushed the feature/replicas branch from 7bcf877 to 9da4449 Compare April 13, 2023 19:14
knizhnik
knizhnik reviewed Apr 24, 2023
View reviewed changes
compute_tools/src/compute.rs Show resolved Hide resolved
compute_tools/src/spec.rs Show resolved Hide resolved
control_plane/src/endpoint.rs Show resolved Hide resolved
pgxn/neon/file_cache.c Show resolved Hide resolved
pgxn/neon/pagestore_smgr.c Show resolved Hide resolved
proxy/src/compute.rs Outdated Show resolved Hide resolved
@MMeent
Update with latest PR suggestions and comments
0830618 
- Update comments on neon_redo_read_buffer_filter
- break early in lfc_evict's scan.
- Remove erronous whitespace in proxy/src/compute.rs
lubennikovaav
lubennikovaav approved these changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@lubennikovaav lubennikovaav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks good to me.
further fixes can be done in follow-up issues

@MMeent MMeent requested a review from knizhnik April 27, 2023 08:49
@MMeent MMeent merged commit e6ec240 into main Apr 27, 2023
@MMeent MMeent deleted the feature/replicas branch April 27, 2023 13:26
@arssher arssher mentioned this pull request Jul 19, 2023
Closed
@bayandin bayandin mentioned this pull request Aug 1, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lubennikovaav lubennikovaav lubennikovaav approved these changes

@knizhnik knizhnik knizhnik approved these changes

@funbringer funbringer Awaiting requested review from funbringer funbringer is a code owner automatically assigned from neondatabase/compute

@hlinnaka hlinnaka Awaiting requested review from hlinnaka

@chaporgin chaporgin Awaiting requested review from chaporgin chaporgin was automatically assigned from neondatabase/control-plane

@arssher arssher Awaiting requested review from arssher

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MMeent @hlinnaka @knizhnik @arssher @lubennikovaav