From 56ad5a93248f09fb1aeec30d7e3eb5ace0a98b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C5=82eczek?= Date: Sat, 30 May 2026 19:19:04 +0200 Subject: [PATCH] refactor: get rid of dbQueryTimings from SchemaCache Schema cache query timings are only needed immediately after a schema-cache reload to emit SchemaCacheQueriedObs. Storing them inside SchemaCache makes the cache carry transient observability data that is not part of the cached schema state and is never used by request handling. This change makes querySchemaCache to return query timings in a tuple in parallel to SchemaCache and removes dbQueryTimings field. --- src/PostgREST/AppState.hs | 4 ++-- src/PostgREST/CLI.hs | 2 +- src/PostgREST/SchemaCache.hs | 13 +++++-------- test/observability/Main.hs | 2 +- test/spec/Main.hs | 2 +- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/PostgREST/AppState.hs b/src/PostgREST/AppState.hs index fdaac1a90..d1f14280d 100644 --- a/src/PostgREST/AppState.hs +++ b/src/PostgREST/AppState.hs @@ -351,7 +351,7 @@ retryingSchemaCacheLoad appState@AppState{stateObserver=observer, stateMainThrea observer $ SchemaCacheErrorObs configDbSchemas configDbExtraSearchPath e return Nothing - Right sCache -> do + Right (sCache, queryTimings) -> do -- IMPORTANT: While the pending schema cache state starts from running the above querySchemaCache, only at this stage we block API requests due to the usage of an -- IORef on putSchemaCache. This is why schema cache status is marked as pending here to signal the Admin server (using isPending) that we're on a recovery state. markSchemaCachePending appState @@ -361,7 +361,7 @@ retryingSchemaCacheLoad appState@AppState{stateObserver=observer, stateMainThrea -- We do it after successfully querying the schema cache (because this can fail and during retries we would flush the pool repeatedly unnecessarily) -- and after marking sCacheStatus as pending, flushPool appState - observer $ SchemaCacheQueriedObs resultTime $ dbQueryTimings sCache + observer $ SchemaCacheQueriedObs resultTime queryTimings observer $ SchemaCacheLoadedObs loadTime summary markSchemaCacheLoaded appState return $ Just sCache diff --git a/src/PostgREST/CLI.hs b/src/PostgREST/CLI.hs index 207c69d62..c5ca7fefc 100644 --- a/src/PostgREST/CLI.hs +++ b/src/PostgREST/CLI.hs @@ -68,7 +68,7 @@ dumpSchema appState = do let observer = AppState.getObserver appState observer $ SchemaCacheErrorObs configDbSchemas configDbExtraSearchPath e exitFailure - Right sCache -> return $ JSON.encode sCache + Right (sCache, _) -> return $ JSON.encode sCache -- | Command line interface options data CLI = CLI diff --git a/src/PostgREST/SchemaCache.hs b/src/PostgREST/SchemaCache.hs index 997d16ab2..e026dc145 100644 --- a/src/PostgREST/SchemaCache.hs +++ b/src/PostgREST/SchemaCache.hs @@ -89,11 +89,10 @@ data SchemaCache = SchemaCache -- Since index construction can be expensive, we build it once and store in the SchemaCache -- Haskell lazy evaluation ensures it's only built on first use and memoized afterwards , dbTablesFuzzyIndex :: TablesFuzzyIndex - , dbQueryTimings :: Maybe QueryTimings -- ^ cached time for the time each query took when debugging } deriving (Show) instance JSON.ToJSON SchemaCache where - toJSON (SchemaCache tabs rels routs reps hdlers tzs _ _) = JSON.object [ + toJSON (SchemaCache tabs rels routs reps hdlers tzs _) = JSON.object [ "dbTables" .= JSON.toJSON tabs , "dbRelationships" .= JSON.toJSON rels , "dbRoutines" .= JSON.toJSON routs @@ -103,7 +102,7 @@ instance JSON.ToJSON SchemaCache where ] showSummary :: SchemaCache -> Text -showSummary (SchemaCache tbls rels routs reps mediaHdlrs tzs _ _) = +showSummary (SchemaCache tbls rels routs reps mediaHdlrs tzs _) = T.intercalate ", " [ show (HM.size tbls) <> " Relations" , show (HM.size rels) <> " Relationships" @@ -154,7 +153,7 @@ type SqlQuery = ByteString maxDbTablesForFuzzySearch :: Int maxDbTablesForFuzzySearch = 500 -querySchemaCache :: AppConfig -> SQL.Transaction SchemaCache +querySchemaCache :: AppConfig -> SQL.Transaction (SchemaCache, Maybe QueryTimings) querySchemaCache conf@AppConfig{..} = do SQL.sql "set local schema ''" -- This voids the search path. The following queries need this for getting the fully qualified name(schema.name) of every db object tabs <- sqlTimedStmt gucTbls conf allTables @@ -179,7 +178,7 @@ querySchemaCache conf@AppConfig{..} = do let tabsWViewsPks = addViewPrimaryKeys tabs keyDeps rels = addInverseRels $ addM2MRels tabsWViewsPks $ addViewM2OAndO2ORels keyDeps m2oRels - return $ removeInternal schemas $ SchemaCache { + return (removeInternal schemas $ SchemaCache { dbTables = tabsWViewsPks , dbRelationships = getOverrideRelationshipsMap rels cRels , dbRoutines = funcs @@ -191,8 +190,7 @@ querySchemaCache conf@AppConfig{..} = do -- Only build fuzzy index for schemas with a reasonable number of tables -- Fuzzy.FuzzySet is memory heavy we just don't use it for large schemas Fuzzy.fromList <$> HM.filter ((< maxDbTablesForFuzzySearch) . length) (HM.fromListWith (<>) ((qiSchema &&& pure . qiName) <$> HM.keys tabsWViewsPks)) - , dbQueryTimings = qsTime - } + }, qsTime) where schemas = toList configDbSchemas isLogDebug = configLogLevel == LogDebug @@ -230,7 +228,6 @@ removeInternal schemas dbStruct = , dbMediaHandlers = dbMediaHandlers dbStruct , dbTimezones = dbTimezones dbStruct , dbTablesFuzzyIndex = dbTablesFuzzyIndex dbStruct - , dbQueryTimings = dbQueryTimings dbStruct } where hasInternalJunction ComputedRelationship{} = False diff --git a/test/observability/Main.hs b/test/observability/Main.hs index 5fcc737cc..227855362 100644 --- a/test/observability/Main.hs +++ b/test/observability/Main.hs @@ -71,4 +71,4 @@ main = do where loadSCache pool conf = - either (panic.show) id <$> P.use pool (HT.transaction HT.ReadCommitted HT.Read $ querySchemaCache conf) + either (panic.show) fst <$> P.use pool (HT.transaction HT.ReadCommitted HT.Read $ querySchemaCache conf) diff --git a/test/spec/Main.hs b/test/spec/Main.hs index e9d75f4b9..d951430d4 100644 --- a/test/spec/Main.hs +++ b/test/spec/Main.hs @@ -289,4 +289,4 @@ main = do where loadSCache pool conf = - either (panic.show) id <$> P.use pool (HT.transaction HT.ReadCommitted HT.Read $ querySchemaCache conf) + either (panic.show) fst <$> P.use pool (HT.transaction HT.ReadCommitted HT.Read $ querySchemaCache conf)