From 60d87966b8c81d39568fbb66e1fd9155b1007b76 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 9 Jul 2025 16:39:40 +0300 Subject: [PATCH] minor comment improvement --- .../src/worker_process/main_loop.rs | 7 +-- pgxn/neon/pagestore_smgr.c | 43 +++++++++++-------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/pgxn/neon/communicator/src/worker_process/main_loop.rs b/pgxn/neon/communicator/src/worker_process/main_loop.rs index 2ef82e7746..04586f302c 100644 --- a/pgxn/neon/communicator/src/worker_process/main_loop.rs +++ b/pgxn/neon/communicator/src/worker_process/main_loop.rs @@ -259,9 +259,10 @@ impl<'t> CommunicatorWorkerProcessStruct<'t> { // Is it possible that the last-written LSN is ahead of last flush LSN? Generally not, we // shouldn't evict a page from the buffer cache before all its modifications have been - // safely flushed. That's the "WAL before data" rule. However, such case does exist at index - // building: _bt_blwritepage logs the full page without flushing WAL before smgrextend - // (files are fsynced before build ends). + // safely flushed. That's the "WAL before data" rule. However, there are a few exceptions: + // + // - when creation an index: _bt_blwritepage logs the full page without flushing WAL before + // smgrextend (files are fsynced before build ends). // // XXX: If we make a request LSN greater than the current WAL flush LSN, the pageserver would // block waiting for the WAL arrive, until we flush it and it propagates through the diff --git a/pgxn/neon/pagestore_smgr.c b/pgxn/neon/pagestore_smgr.c index 9ef393b8ff..a3a33e9f4b 100644 --- a/pgxn/neon/pagestore_smgr.c +++ b/pgxn/neon/pagestore_smgr.c @@ -853,6 +853,30 @@ neon_create(SMgrRelation reln, ForkNumber forkNum, bool isRedo) RelFileInfoFmt(InfoFromSMgrRel(reln)), forkNum); + /* + * Newly created relation is empty, remember that in the relsize cache. + * + * Note that in REDO, this is called to make sure the relation fork + * exists, but it does not truncate the relation. So, we can only update + * the relsize if it didn't exist before. + * + * Also, in redo, we must make sure to update the cached size of the + * relation, as that is the primary source of truth for REDO's file length + * considerations, and as file extension isn't (perfectly) logged, we need + * to take care of that before we hit file size checks. + * + * FIXME: This is currently not just an optimization, but required for + * correctness. Postgres can call smgrnblocks() on the newly-created + * relation. Currently, we don't call SetLastWrittenLSN() when a new + * relation created, so if we didn't remember the size in the relsize + * cache, we might call smgrnblocks() on the newly-created relation before + * the creation WAL record has been received by the page server. + * + * XXX: with the new communicator, similar considerations apply. However, + * during replay, neon_get_write_lsn() returns the (end-)LSN of the record + * that's being replayed, so we should not have the correctness issue + * mentioned in previous paragraph. + */ if (neon_enable_new_communicator) { XLogRecPtr lsn = neon_get_write_lsn(); @@ -867,25 +891,6 @@ neon_create(SMgrRelation reln, ForkNumber forkNum, bool isRedo) } else { - /* - * Newly created relation is empty, remember that in the relsize cache. - * - * Note that in REDO, this is called to make sure the relation fork - * exists, but it does not truncate the relation. So, we can only update - * the relsize if it didn't exist before. - * - * Also, in redo, we must make sure to update the cached size of the - * relation, as that is the primary source of truth for REDO's file length - * considerations, and as file extension isn't (perfectly) logged, we need - * to take care of that before we hit file size checks. - * - * FIXME: This is currently not just an optimization, but required for - * correctness. Postgres can call smgrnblocks() on the newly-created - * relation. Currently, we don't call SetLastWrittenLSN() when a new - * relation created, so if we didn't remember the size in the relsize - * cache, we might call smgrnblocks() on the newly-created relation before - * the creation WAL record hass been received by the page server. - */ if (isRedo) { update_cached_relsize(InfoFromSMgrRel(reln), forkNum, 0);