From bfb4b0991d8af1c594bd760b997483c2ee93faba Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 23 Jul 2025 01:40:42 +0300 Subject: [PATCH] Refactor the way lfc_get_stats() is implemented This reduces the boilerplate a little, and makes it more straightforward to dispatch the call to the old or the new communicator --- pgxn/neon/communicator_new.c | 19 +++++ pgxn/neon/communicator_new.h | 8 +- pgxn/neon/file_cache.c | 148 ++++++++--------------------------- pgxn/neon/file_cache.h | 8 +- pgxn/neon/lfc_prewarm.c | 1 - pgxn/neon/neon.c | 33 ++++++++ pgxn/neon/neon.h | 5 ++ 7 files changed, 96 insertions(+), 126 deletions(-) diff --git a/pgxn/neon/communicator_new.c b/pgxn/neon/communicator_new.c index f0663411fe..e94d7bc36b 100644 --- a/pgxn/neon/communicator_new.c +++ b/pgxn/neon/communicator_new.c @@ -1345,3 +1345,22 @@ communicator_new_approximate_working_set_size_seconds(time_t duration, bool rese memset(communicator_shmem_ptr->wss_estimation.regs, 0, sizeof(communicator_shmem_ptr->wss_estimation.regs)); return dc; } + + +/* + * Return an array of LfcStatsEntrys, terminated by an entry with NULL name + */ +LfcStatsEntry * +communicator_new_get_lfc_stats(void) +{ + LfcStatsEntry *entries; + int i = 0; + + // TODO +#define NUM_ENTRIES 0 + entries = palloc(sizeof(LfcStatsEntry) * (NUM_ENTRIES + 1)); + entries[i++] = (LfcStatsEntry) { NULL, false, 0 }; + Assert(i <= NUM_ENTRIES); + + return entries; +} diff --git a/pgxn/neon/communicator_new.h b/pgxn/neon/communicator_new.h index ec5d9aad07..9ec762f479 100644 --- a/pgxn/neon/communicator_new.h +++ b/pgxn/neon/communicator_new.h @@ -12,11 +12,11 @@ #ifndef COMMUNICATOR_NEW_H #define COMMUNICATOR_NEW_H -#include "lfc_prewarm.h" -#include "neon_pgversioncompat.h" - #include "storage/buf_internals.h" +#include "lfc_prewarm.h" +#include "neon.h" +#include "neon_pgversioncompat.h" #include "pagestore_client.h" /* initialization at postmaster startup */ @@ -60,7 +60,7 @@ extern void communicator_new_update_cached_rel_size(NRelFileInfo rinfo, ForkNumb /* other functions */ extern int32 communicator_new_approximate_working_set_size_seconds(time_t duration, bool reset); - extern FileCacheState *communicator_new_get_lfc_state(size_t max_entries); +extern LfcStatsEntry *communicator_new_get_lfc_stats(void); #endif /* COMMUNICATOR_NEW_H */ diff --git a/pgxn/neon/file_cache.c b/pgxn/neon/file_cache.c index 0370da3fbd..43aa420665 100644 --- a/pgxn/neon/file_cache.c +++ b/pgxn/neon/file_cache.c @@ -1534,128 +1534,44 @@ lfc_writev(NRelFileInfo rinfo, ForkNumber forkNum, BlockNumber blkno, LWLockRelease(lfc_lock); } -typedef struct +/* + * Return an array of LfcStatsEntrys, terminated by an entry with NULL name + */ +LfcStatsEntry * +get_lfc_stats(void) { - TupleDesc tupdesc; -} NeonGetStatsCtx; + LfcStatsEntry *entries; + int i = 0; -#define NUM_NEON_GET_STATS_COLS 2 +#define NUM_ENTRIES 10 + entries = palloc(sizeof(LfcStatsEntry) * (NUM_ENTRIES + 1)); -PG_FUNCTION_INFO_V1(neon_get_lfc_stats); -Datum -neon_get_lfc_stats(PG_FUNCTION_ARGS) -{ - FuncCallContext *funcctx; - NeonGetStatsCtx *fctx; - MemoryContext oldcontext; - TupleDesc tupledesc; - Datum result; - HeapTuple tuple; - char const *key; - uint64 value = 0; - Datum values[NUM_NEON_GET_STATS_COLS]; - bool nulls[NUM_NEON_GET_STATS_COLS]; + entries[i++] = (LfcStatsEntry) {"file_cache_chunk_size_pages", lfc_ctl == NULL, + lfc_ctl ? lfc_blocks_per_chunk : 0 }; + entries[i++] = (LfcStatsEntry) {"file_cache_misses", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->misses : 0}; + entries[i++] = (LfcStatsEntry) {"file_cache_hits", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->hits : 0 }; + entries[i++] = (LfcStatsEntry) {"file_cache_used", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->used : 0 }; + entries[i++] = (LfcStatsEntry) {"file_cache_writes", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->writes : 0 }; + entries[i++] = (LfcStatsEntry) {"file_cache_size", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->size : 0 }; + entries[i++] = (LfcStatsEntry) {"file_cache_used_pages", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->used_pages : 0 }; + entries[i++] = (LfcStatsEntry) {"file_cache_evicted_pages", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->evicted_pages : 0 }; + entries[i++] = (LfcStatsEntry) {"file_cache_limit", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->limit : 0 }; + entries[i++] = (LfcStatsEntry) {"file_cache_chunks_pinned", lfc_ctl == NULL, + lfc_ctl ? lfc_ctl->pinned : 0 }; + entries[i++] = (LfcStatsEntry) { NULL, false, 0 }; + Assert(i <= NUM_ENTRIES); - if (SRF_IS_FIRSTCALL()) - { - funcctx = SRF_FIRSTCALL_INIT(); - - /* Switch context when allocating stuff to be used in later calls */ - oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx); - - /* Create a user function context for cross-call persistence */ - fctx = (NeonGetStatsCtx *) palloc(sizeof(NeonGetStatsCtx)); - - /* Construct a tuple descriptor for the result rows. */ - tupledesc = CreateTemplateTupleDesc(NUM_NEON_GET_STATS_COLS); - - TupleDescInitEntry(tupledesc, (AttrNumber) 1, "lfc_key", - TEXTOID, -1, 0); - TupleDescInitEntry(tupledesc, (AttrNumber) 2, "lfc_value", - INT8OID, -1, 0); - - fctx->tupdesc = BlessTupleDesc(tupledesc); - funcctx->user_fctx = fctx; - - /* Return to original context when allocating transient memory */ - MemoryContextSwitchTo(oldcontext); - } - - funcctx = SRF_PERCALL_SETUP(); - - /* Get the saved state */ - fctx = (NeonGetStatsCtx *) funcctx->user_fctx; - - switch (funcctx->call_cntr) - { - case 0: - key = "file_cache_misses"; - if (lfc_ctl) - value = lfc_ctl->misses; - break; - case 1: - key = "file_cache_hits"; - if (lfc_ctl) - value = lfc_ctl->hits; - break; - case 2: - key = "file_cache_used"; - if (lfc_ctl) - value = lfc_ctl->used; - break; - case 3: - key = "file_cache_writes"; - if (lfc_ctl) - value = lfc_ctl->writes; - break; - case 4: - key = "file_cache_size"; - if (lfc_ctl) - value = lfc_ctl->size; - break; - case 5: - key = "file_cache_used_pages"; - if (lfc_ctl) - value = lfc_ctl->used_pages; - break; - case 6: - key = "file_cache_evicted_pages"; - if (lfc_ctl) - value = lfc_ctl->evicted_pages; - break; - case 7: - key = "file_cache_limit"; - if (lfc_ctl) - value = lfc_ctl->limit; - break; - case 8: - key = "file_cache_chunk_size_pages"; - value = lfc_blocks_per_chunk; - break; - case 9: - key = "file_cache_chunks_pinned"; - if (lfc_ctl) - value = lfc_ctl->pinned; - break; - default: - SRF_RETURN_DONE(funcctx); - } - values[0] = PointerGetDatum(cstring_to_text(key)); - nulls[0] = false; - if (lfc_ctl) - { - nulls[1] = false; - values[1] = Int64GetDatum(value); - } - else - nulls[1] = true; - - tuple = heap_form_tuple(fctx->tupdesc, values, nulls); - result = HeapTupleGetDatum(tuple); - SRF_RETURN_NEXT(funcctx, result); + return entries; } - /* * Function returning data from the local file cache * relation node/tablespace/database/blocknum and access_counter diff --git a/pgxn/neon/file_cache.h b/pgxn/neon/file_cache.h index fd79eee532..c3c6611874 100644 --- a/pgxn/neon/file_cache.h +++ b/pgxn/neon/file_cache.h @@ -12,6 +12,7 @@ #define FILE_CACHE_h #include "lfc_prewarm.h" +#include "neon.h" #include "neon_pgversioncompat.h" @@ -41,13 +42,10 @@ extern int lfc_cache_containsv(NRelFileInfo rinfo, ForkNumber forkNum, extern void lfc_init(void); extern bool lfc_prefetch(NRelFileInfo rinfo, ForkNumber forknum, BlockNumber blkno, const void* buffer, XLogRecPtr lsn); + extern FileCacheState* lfc_get_state(size_t max_entries); - extern int32 lfc_approximate_working_set_size_seconds(time_t duration, bool reset); - - -extern int32 lfc_approximate_working_set_size_seconds(time_t duration, bool reset); - +extern LfcStatsEntry *get_lfc_stats(void); static inline bool lfc_read(NRelFileInfo rinfo, ForkNumber forkNum, BlockNumber blkno, diff --git a/pgxn/neon/lfc_prewarm.c b/pgxn/neon/lfc_prewarm.c index 680272fb8a..1608fde628 100644 --- a/pgxn/neon/lfc_prewarm.c +++ b/pgxn/neon/lfc_prewarm.c @@ -541,7 +541,6 @@ lfc_prewarm_with_async_requests(FileCacheState *fcs) request_startblkno = request_endblkno = InvalidBlockNumber; } - Assert(prewarm_ctl->prewarm_canceled); elog(LOG, "LFC: complete prewarming: loaded %lu pages", (unsigned long) prewarm_ctl->prewarmed_pages); prewarm_ctl->completed = GetCurrentTimestamp(); diff --git a/pgxn/neon/neon.c b/pgxn/neon/neon.c index 655fdc6faa..650394c8b8 100644 --- a/pgxn/neon/neon.c +++ b/pgxn/neon/neon.c @@ -713,6 +713,39 @@ approximate_working_set_size(PG_FUNCTION_ARGS) PG_RETURN_INT32(dc); } +PG_FUNCTION_INFO_V1(neon_get_lfc_stats); +Datum +neon_get_lfc_stats(PG_FUNCTION_ARGS) +{ +#define NUM_NEON_GET_STATS_COLS 2 + ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + LfcStatsEntry *entries; + LfcStatsEntry *entry; + + InitMaterializedSRF(fcinfo, 0); + + if (neon_use_communicator_worker) + entries = communicator_new_get_lfc_stats(); + else + entries = get_lfc_stats(); + + entry = entries; + while (entry->metric_name != NULL) + { + Datum values[NUM_NEON_GET_STATS_COLS]; + bool nulls[NUM_NEON_GET_STATS_COLS]; + + values[0] = CStringGetTextDatum(entry->metric_name); + nulls[1] = entry->isnull; + values[1] = Int64GetDatum(entry->isnull ? 0 : entry->value); + tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls); + entry++; + } + + PG_RETURN_VOID(); +} + + /* * Initialization stage 2: make requests for the amount of shared memory we * will need. diff --git a/pgxn/neon/neon.h b/pgxn/neon/neon.h index 20c850864a..ad843553a5 100644 --- a/pgxn/neon/neon.h +++ b/pgxn/neon/neon.h @@ -84,5 +84,10 @@ extern void WalproposerShmemInit(void); extern void LwLsnCacheShmemInit(void); extern void NeonPerfCountersShmemInit(void); +typedef struct LfcStatsEntry { + const char *metric_name; + bool isnull; + uint64 value; +} LfcStatsEntry; #endif /* NEON_H */