Skip to content

Use correct LSN in GetPage@LSN calls when nothing has been evicted yet. #40

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

Merged
merged 1 commit into from
May 4, 2021
Merged

Use correct LSN in GetPage@LSN calls when nothing has been evicted yet. #40

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions src/backend/storage/smgr/pagestore_smgr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,21 +337,32 @@ zenith_get_request_lsn(bool nonrel)
}
else
{
lsn = GetLastWrittenPageLSN();
XLogRecPtr flushlsn = GetFlushRecPtr();

/*
* Use the latest LSN that was evicted from the buffer cache. Any
* pages modified by later WAL records must still in the buffer cache,
* so our request cannot concern those.
*/
lsn = GetLastWrittenPageLSN();
elog(DEBUG1, "zenith_get_request_lsn GetLastWrittenPageLSN lsn %X/%X ",
(uint32) ((lsn) >> 32), (uint32) (lsn));

if (lsn > GetFlushRecPtr())
XLogFlush(lsn);
if (lsn == InvalidXLogRecPtr)
{
/* we haven't evicted anything yet since the server was started */
lsn = GetFlushRecPtr();
elog(DEBUG1, "zenith_get_request_lsn GetFlushRecPtr lsn %X/%X request 0",
(uint32) ((lsn) >> 32), (uint32) (lsn));
lsn = InvalidXLogRecPtr;
/*
* We haven't evicted anything yet since the server was
* started. Then just use the latest flushed LSN. That's always
* safe, using the latest evicted LSN is really just an
* optimization.
*/
lsn = flushlsn;
elog(DEBUG1, "zenith_get_request_lsn GetFlushRecPtr lsn %X/%X",
(uint32) ((lsn) >> 32), (uint32) (lsn));
}

/* The record should've been flushed already, since it was evicted, but let's be safe */
if (lsn > flushlsn)
XLogFlush(lsn);
Copy link

@lubennikovaav lubennikovaav May 3, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@hlinnaka hlinnaka May 3, 2021

Choose a reason for hiding this comment

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

Hmm. Is it legal? I can't think of a legit scenario either

}
return lsn;
}
Expand Down