Use correct LSN in GetPage@LSN calls when nothing has been evicted yet. #40
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@lubennikovaav, can you take a look at this please? I didn't understand the changes in commit 153dee9 |
knizhnik
approved these changes
Apr 30, 2021
lubennikovaav
approved these changes
May 3, 2021
lubennikovaav
left a comment
lubennikovaav
left a comment
There was a problem hiding this comment.
commit 153dee9 was introduced to fix a race when we were requesting a page before walreceiver started. To avoid eternal waiting, we simply request page with a special (invalid) LSN and pageserver just returns the latest version of the page.
see https://github.com/zenithdb/zenith/blob/main/pageserver/src/page_cache.rs#L873
Looks like this is not needed anymore, since we decided to handle shared relations and non-rels locally after initial setup.
|
|
||
| /* The record should've been flushed already, since it was evicted, but let's be safe */ | ||
| if (lsn > flushlsn) | ||
| XLogFlush(lsn); |
There was a problem hiding this comment.
My first thought was how is it even legal, let's use Assert instead.
But it seems to be a normal situation. Let's explain it in the comment.
Contributor
Author
There was a problem hiding this comment.
Hmm. Is it legal? I can't think of a legit scenario either
If no page has been evicted from page cache yet, use the latest flushed LSN. Commit 153dee9, "Request special lsn during bootstrap", changed this but it is not clear to me why. As far as I can see, this isn't related to bootstrapping, and using the latest flushed LSN should always work. While we're at it, rearrange the code slightly for readability, and add comments.
hlinnaka
force-pushed
the
fix-getpage-lsn-when-nothing-evicted-yet
branch
from
7ac02d9 to
65b907b
Compare
dimitri
pushed a commit
that referenced
this pull request
Jul 24, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 5, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 5, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 5, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 5, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 6, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 7, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 7, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 7, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 7, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 8, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 8, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 8, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
dimitri
pushed a commit
that referenced
this pull request
Aug 8, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
tristan957
pushed a commit
that referenced
this pull request
Aug 11, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
tristan957
pushed a commit
that referenced
this pull request
Aug 11, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
tristan957
pushed a commit
that referenced
this pull request
Aug 11, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
tristan957
pushed a commit
that referenced
this pull request
Aug 11, 2025
primary_conninfo string is too long with password. Wal receiver reads the password from NEON_AUTH_TOKEN instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If no page has been evicted from page cache yet, use the latest flushed
LSN.
Commit 153dee9, "Request special lsn during bootstrap", changed this
but it is not clear to me why. As far as I can see, this isn't related
to bootstrapping, and using the latest flushed LSN should always work.
While we're at it, rearrange the code slightly for readability, and add
comments.