From 2e34fe03c7e48be1feebea0004b1cc5b384bd613 Mon Sep 17 00:00:00 2001 From: Kosntantin Knizhnik Date: Fri, 27 Jun 2025 21:56:32 +0300 Subject: [PATCH] Replace flags with enum --- pgxn/neon/pagestore_client.h | 21 ++++++++++--------- pgxn/neon/pagestore_smgr.c | 39 +++++++++++++---------------------- pgxn/neon/relkind_cache.c | 40 +++++++++++++++++------------------- 3 files changed, 44 insertions(+), 56 deletions(-) diff --git a/pgxn/neon/pagestore_client.h b/pgxn/neon/pagestore_client.h index 8d7288921b..a11b20ab87 100644 --- a/pgxn/neon/pagestore_client.h +++ b/pgxn/neon/pagestore_client.h @@ -305,28 +305,29 @@ extern void forget_cached_relsize(NRelFileInfo rinfo, ForkNumber forknum); * Finally UNLOGGED_BUILD is set for unlogged build (building of GIST/SPGIST/GIN... indexes). * Relation itself can be logged or unlogged. */ -enum RelKindEntryFlags +typedef enum { - RELKIND_UNLOGGED = 1, /* relation is temp or unlogged */ - RELKIND_UNLOGGED_BUILD = 2, /* unlogged index build */ - RELKIND_RAW = 4 /* relation persistence is not known */ -}; + RELKIND_UNKNOWN, + RELKIND_PERMANENT, + RELKIND_UNLOGGED, + RELKIND_UNLOGGED_BUILD //* buildig index for permanent relation */ +} RelKind; /* utils for neon relkind cache */ typedef struct { NRelFileInfo rel; - uint8 flags; /* See RelKindEntryFlags */ + uint8 relkind; /* See RelKind */ uint16 access_count; dlist_node lru_node; /* LRU list node */ } RelKindEntry; extern void relkind_hash_init(void); -extern RelKindEntry* set_cached_relkind(NRelFileInfo rinfo, uint8 flags); -extern RelKindEntry* get_cached_relkind(NRelFileInfo rinfo, uint8* flags); -extern void store_cached_relkind(RelKindEntry* entry, uint8 flags); -extern void clear_cached_relkind_flags(RelKindEntry* entry, uint8 flags); +extern RelKindEntry* set_cached_relkind(NRelFileInfo rinfo, RelKind relkind); +extern RelKindEntry* get_cached_relkind(NRelFileInfo rinfo, RelKind* relkind); +extern void store_cached_relkind(RelKindEntry* entry, RelKind relkind); +extern void update_cached_relkind(RelKindEntry* entry, RelKind relkind); extern void unpin_cached_relkind(RelKindEntry* entry); extern void unlock_cached_relkind(void); extern void forget_cached_relkind(NRelFileInfo rinfo); diff --git a/pgxn/neon/pagestore_smgr.c b/pgxn/neon/pagestore_smgr.c index 984f8d8edc..28408eb559 100644 --- a/pgxn/neon/pagestore_smgr.c +++ b/pgxn/neon/pagestore_smgr.c @@ -1604,31 +1604,26 @@ neon_write(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, const vo { XLogRecPtr lsn; RelKindEntry *entry; - bool unlogged; - uint8 flags = 0; + RelKind relkind = RELKIND_UNKNOWN; switch (reln->smgr_relpersistence) { case 0: - entry = get_cached_relkind(InfoFromSMgrRel(reln), &flags); + entry = get_cached_relkind(InfoFromSMgrRel(reln), &relkind); if (entry) { /* We do not know relation persistence: let's determine it */ - unlogged = mdexists(reln, debug_compare_local ? INIT_FORKNUM : forknum); - store_cached_relkind(entry, unlogged ? RELKIND_UNLOGGED : 0); + relkind = mdexists(reln, debug_compare_local ? INIT_FORKNUM : forknum) ? RELKIND_UNLOGGED : RELKIND_PERMANENT; + store_cached_relkind(entry, relkind); } - else - { - unlogged = (flags & (RELKIND_UNLOGGED_BUILD|RELKIND_UNLOGGED)) != 0; - } - if (unlogged) + if (relkind == RELKIND_UNLOGGED || relkind == RELKIND_UNLOGGED_BUILD) { #if PG_MAJORVERSION_NUM >= 17 mdwritev(reln, forknum, blocknum, &buffer, 1, skipFsync); #else mdwrite(reln, forknum, blocknum, buffer, skipFsync); #endif - if (flags & RELKIND_UNLOGGED_BUILD) + if (relkind == RELKIND_UNLOGGED_BUILD) { unlock_cached_relkind(); } @@ -1694,28 +1689,22 @@ neon_writev(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno, const void **buffers, BlockNumber nblocks, bool skipFsync) { RelKindEntry *entry; - bool unlogged; - uint8 flags = 0; - + RelKind relkind = RELKIND_UNKNOWN; switch (reln->smgr_relpersistence) { case 0: - entry = get_cached_relkind(InfoFromSMgrRel(reln), &flags); + entry = get_cached_relkind(InfoFromSMgrRel(reln), &relkind); if (entry) { /* We do not know relation persistence: let's determine it */ - unlogged = mdexists(reln, debug_compare_local ? INIT_FORKNUM : forknum); - store_cached_relkind(entry, unlogged ? RELKIND_UNLOGGED : 0); + relkind = mdexists(reln, debug_compare_local ? INIT_FORKNUM : forknum) ? RELKIND_UNLOGGED : RELKIND_PERMANENT; + store_cached_relkind(entry, relkind); } - else - { - unlogged = (flags & (RELKIND_UNLOGGED_BUILD|RELKIND_UNLOGGED)) != 0; - } - if (unlogged) + if (relkind == RELKIND_UNLOGGED || relkind == RELKIND_UNLOGGED_BUILD) { /* It exists locally. Guess it's unlogged then. */ mdwritev(reln, forknum, blkno, buffers, nblocks, skipFsync); - if (flags & RELKIND_UNLOGGED_BUILD) + if (relkind == RELKIND_UNLOGGED_BUILD) { unlock_cached_relkind(); } @@ -2006,7 +1995,7 @@ neon_start_unlogged_build(SMgrRelation reln) case RELPERSISTENCE_TEMP: case RELPERSISTENCE_UNLOGGED: unlogged_build_rel_info = InfoFromSMgrRel(reln); - unlogged_build_rel_entry = set_cached_relkind(unlogged_build_rel_info, RELKIND_UNLOGGED|RELKIND_UNLOGGED_BUILD); + unlogged_build_rel_entry = set_cached_relkind(unlogged_build_rel_info, RELKIND_UNLOGGED); unlogged_build_phase = UNLOGGED_BUILD_NOT_PERMANENT; if (debug_compare_local) { @@ -2132,7 +2121,7 @@ neon_end_unlogged_build(SMgrRelation reln) InfoFromNInfoB(rinfob), MAIN_FORKNUM); - clear_cached_relkind_flags(unlogged_build_rel_entry, RELKIND_UNLOGGED_BUILD); + update_cached_relkind(unlogged_build_rel_entry, RELKIND_PERMANENT); /* Remove local copy */ for (int forknum = 0; forknum <= MAX_FORKNUM; forknum++) diff --git a/pgxn/neon/relkind_cache.c b/pgxn/neon/relkind_cache.c index 41582c43b6..86c95657b6 100644 --- a/pgxn/neon/relkind_cache.c +++ b/pgxn/neon/relkind_cache.c @@ -137,13 +137,13 @@ get_entry(NRelFileInfo rinfo, bool* found) hash_search(relkind_hash, &victim->rel, HASH_REMOVE, NULL); relkind_ctl->size -= 1; } - entry->flags = RELKIND_RAW; /* information about relation kind is not yet available */ + entry->relkind = RELKIND_UNKNOWN; /* information about relation kind is not yet available */ relkind_ctl->pinned += 1; entry->access_count = 1; } else if (entry->access_count++ == 0) { - Assert(!(entry->flags & RELKIND_RAW)); /* unpinned entry can not be raw */ + Assert(entry->relkind != RELKIND_UNKNOWN); dlist_delete(&entry->lru_node); relkind_ctl->pinned += 1; } @@ -156,7 +156,7 @@ get_entry(NRelFileInfo rinfo, bool* found) * Return pinned entry. It will be released by unpin_cached_relkind at the end of unlogged build. */ RelKindEntry* -set_cached_relkind(NRelFileInfo rinfo, uint8 flags) +set_cached_relkind(NRelFileInfo rinfo, RelKind relkind) { RelKindEntry *entry = NULL; bool found; @@ -164,7 +164,7 @@ set_cached_relkind(NRelFileInfo rinfo, uint8 flags) /* Use spinlock to prevent concurrent hash modifitcation */ SpinLockAcquire(&relkind_ctl->mutex); entry = get_entry(rinfo, &found); - entry->flags = flags; + entry->relkind = relkind; SpinLockRelease(&relkind_ctl->mutex); return entry; } @@ -172,12 +172,12 @@ set_cached_relkind(NRelFileInfo rinfo, uint8 flags) /* * Lookup entry and create new one if not exists. This function is called by neon_write to detenmine if changes should be written to the local disk. * In case of overflow removes least recently used entry. - * If entry is found and is not raw, then flags are stord in flags and NULL is returned. + * If entry is found and its relkind is known, then it is stored in provided location and NULL is returned. * If entry is not found then new one is created, pinned and returned. Entry should be updated using store_cached_relkind. * Shared lock is obtained if relation is involved in inlogged build. */ RelKindEntry* -get_cached_relkind(NRelFileInfo rinfo, uint8* flags) +get_cached_relkind(NRelFileInfo rinfo, RelKind* relkind) { RelKindEntry *entry; bool found; @@ -186,8 +186,8 @@ get_cached_relkind(NRelFileInfo rinfo, uint8* flags) entry = get_entry(rinfo, &found); if (found) { - /* If entry is not raw, then there is no need to pin it */ - if (!(entry->flags & RELKIND_RAW)) + /* If relation persistence is known, then there is no need to pin it */ + if (entry->relkind == RELKIND_UNKNOWN) { /* Fast path: normal (persistent) relation with kind stored in the cache */ if (--entry->access_count == 0) @@ -195,23 +195,23 @@ get_cached_relkind(NRelFileInfo rinfo, uint8* flags) dlist_push_tail(&relkind_ctl->lru, &entry->lru_node); } } - /* Need to set shared lock in case of unlogged build to prevent race condition on unlogged build end */ - if (entry->flags & RELKIND_UNLOGGED_BUILD) + /* Need to set shared lock in case of unlogged build to prevent race condition at unlogged build end */ + if (entry->relkind == RELKIND_UNLOGGED_BUILD) { /* Set shared lock to prevent unlinking relation files by backend completed unlogged build. * This backend will set exclsuive lock before unlinking files. * Shared locks allows other backends to perform write in parallel. */ LWLockAcquire(relkind_lock, LW_SHARED); - /* Recheck flags under lock */ - if (!(entry->flags & RELKIND_UNLOGGED_BUILD)) + /* Recheck relkind under lock */ + if (entry->relkind != RELKIND_UNLOGGED_BUILD) { /* Unlogged build is already completed: release lock - we do not need to do any writes to local disk */ LWLockRelease(relkind_lock); } } - *flags = entry->flags; - if (!(entry->flags & RELKIND_RAW)) + *relkind = entry->relkind; + if (entry->relkind != RELKIND_UNKNOWN) { /* We do not need this entry any more */ entry = NULL; @@ -222,14 +222,13 @@ get_cached_relkind(NRelFileInfo rinfo, uint8* flags) } /* - * Store relation persistence as a result of mdexists check. - * Unpin entry. + * Store relation kind as a result of mdexists check. Unpin entry. */ void -store_cached_relkind(RelKindEntry* entry, uint8 flags) +store_cached_relkind(RelKindEntry* entry, RelKind relkind) { SpinLockAcquire(&relkind_ctl->mutex); - entry->flags = flags; + entry->relkind = relkind; Assert(entry->access_count != 0); if (--entry->access_count == 0) { @@ -240,16 +239,15 @@ store_cached_relkind(RelKindEntry* entry, uint8 flags) SpinLockRelease(&relkind_ctl->mutex); } - /* * Change relation persistence. * This operation obtains exclusiove lock, preventing any concurrent writes. */ void -clear_cached_relkind_flags(RelKindEntry* entry, uint8 flags) +update_cached_relkind(RelKindEntry* entry, RelKind relkind) { LWLockAcquire(relkind_lock, LW_EXCLUSIVE); - entry->flags &= ~flags; + entry->relkind = relkind; LWLockRelease(relkind_lock); }