Improve performance of replace_metadata on tracking tables. Closes #3802 (#4182)

* Improve performance of replace_metadata on tracking tables. Closes #3802 On the 1000 table track case this went from >20min to 8 seconds, the bottleneck being all the former calls (for each table) to buildSchemaCache. * refactor replace metadata Only save metadata in hdb_catalog schema and build schema cache (strictly) * remove `withBuildSchemaCache` function Co-authored-by: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Co-authored-by: Vamshi Surabhi <0x777@users.noreply.github.com>
2024-12-14 08:02:15 +03:00 · 2020-03-31 10:00:12 -04:00 · 2020-03-31 10:00:12 -04:00 · 58ba01f31c
commit 58ba01f31c
parent e33f3a8468
6 changed files with 72 additions and 86 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -30,6 +30,7 @@ Read more about check constraints on [Postgres Docs](https://www.postgresql.org/
 (close #3969) (#4145)

 ### Bug fixes and improvements
+- server: improve performance of replace_metadata tracking many tables (fix #3802)
 - server: option to reload remote schemas in 'reload_metadata' API (fix #3792, #4117)
 - server: fix various space leaks to avoid excessive memory consumption
 - server: fix postgres query error when computed fields included in mutation response (fix #4035)
--- a/server/src-lib/Hasura/GraphQL/Schema/Merge.hs
+++ b/server/src-lib/Hasura/GraphQL/Schema/Merge.hs
@ -93,12 +93,12 @@ checkSchemaConflicts gCtx remoteCtx = do

 checkConflictingNode
  :: (MonadError QErr m)
-  => GCtx
+  => TypeMap
+  -- ^ See 'GCtx'.
  -> G.Name
  -> m ()
-checkConflictingNode gCtx node = do
-  let typeMap = _gTypes gCtx
-      hQR     = _otiFields <$>
+checkConflictingNode typeMap node = do
+  let hQR     = _otiFields <$>
                (getObjTyM =<< Map.lookup hQRName typeMap)
      hMR     = _otiFields <$>
                (getObjTyM =<< Map.lookup hMRName typeMap)
--- a/server/src-lib/Hasura/RQL/DDL/Metadata.hs
+++ b/server/src-lib/Hasura/RQL/DDL/Metadata.hs
@ -1,3 +1,4 @@
+{-# LANGUAGE RecordWildCards #-}
 module Hasura.RQL.DDL.Metadata
  ( runReplaceMetadata
  , runExportMetadata
@ -26,10 +27,10 @@ import           Hasura.RQL.DDL.ComputedField       (dropComputedFieldFromCatalo
 import           Hasura.RQL.DDL.EventTrigger        (delEventTriggerFromCatalog, subTableP2)
 import           Hasura.RQL.DDL.Metadata.Types
 import           Hasura.RQL.DDL.Permission.Internal (dropPermFromCatalog)
-import           Hasura.RQL.DDL.RemoteSchema        (addRemoteSchemaP2, fetchRemoteSchemas,
+import           Hasura.RQL.DDL.RemoteSchema        (addRemoteSchemaToCatalog, fetchRemoteSchemas,
                                                     removeRemoteSchemaFromCatalog)
+import           Hasura.RQL.DDL.Schema.Catalog      (saveTableToCatalog)
 import           Hasura.RQL.Types
-import           Hasura.Server.Version              (HasVersion)
 import           Hasura.SQL.Types

 import qualified Database.PG.Query                  as Q
@ -41,8 +42,9 @@ import qualified Hasura.RQL.DDL.QueryCollection     as Collection
 import qualified Hasura.RQL.DDL.Relationship        as Relationship
 import qualified Hasura.RQL.DDL.Schema              as Schema

-clearMetadata :: Q.TxE QErr ()
-clearMetadata = Q.catchE defaultTxErrorHandler $ do
+-- | Purge all user-defined metadata; metadata with is_system_defined = false
+clearUserMetadata :: MonadTx m => m ()
+clearUserMetadata = liftTx $ Q.catchE defaultTxErrorHandler $ do
  Q.unitQ "DELETE FROM hdb_catalog.hdb_function WHERE is_system_defined <> 'true'" () False
  Q.unitQ "DELETE FROM hdb_catalog.hdb_permission WHERE is_system_defined <> 'true'" () False
  Q.unitQ "DELETE FROM hdb_catalog.hdb_relationship WHERE is_system_defined <> 'true'" () False
@ -60,7 +62,7 @@ runClearMetadata
  :: (MonadTx m, CacheRWM m)
  => ClearMetadata -> m EncJSON
 runClearMetadata _ = do
-  liftTx clearMetadata
+  clearUserMetadata
  buildSchemaCacheStrict
  return successMsg

@ -124,69 +126,53 @@ applyQP1 (ReplaceMetadata _ tables functionsMeta schemas collections
    getDups l =
      l L.\\ HS.toList (HS.fromList l)

-applyQP2
-  :: ( HasVersion
-     , MonadIO m
-     , MonadTx m
-     , CacheRWM m
-     , HasSystemDefined m
-     , HasHttpManager m
-     )
-  => ReplaceMetadata
-  -> m EncJSON
-applyQP2 (ReplaceMetadata _ tables functionsMeta
-          schemas collections allowlist customTypes actions) = do
-
-  liftTx clearMetadata
+applyQP2 :: (CacheRWM m, MonadTx m, HasSystemDefined m) => ReplaceMetadata -> m EncJSON
+applyQP2 replaceMetadata = do
+  clearUserMetadata
+  saveMetadata replaceMetadata
  buildSchemaCacheStrict
+  pure successMsg
+
+saveMetadata :: (MonadTx m, HasSystemDefined m) => ReplaceMetadata -> m ()
+saveMetadata (ReplaceMetadata _ tables functionsMeta
+              schemas collections allowlist customTypes actions) = do

  withPathK "tables" $ do
-    -- tables and views
-    indexedForM_ tables $ \tableMeta -> do
-      let tableName = tableMeta ^. tmTable
-          isEnum = tableMeta ^. tmIsEnum
-          config = tableMeta ^. tmConfiguration
-      void $ Schema.trackExistingTableOrViewP2 tableName isEnum config
+    indexedForM_ tables $ \TableMeta{..} -> do
+      -- Save table
+      saveTableToCatalog _tmTable _tmIsEnum _tmConfiguration

-    indexedForM_ tables $ \table -> do
      -- Relationships
      withPathK "object_relationships" $
-        indexedForM_ (table ^. tmObjectRelationships) $ \objRel ->
-        Relationship.insertRelationshipToCatalog (table ^. tmTable) ObjRel objRel
+        indexedForM_ _tmObjectRelationships $ \objRel ->
+        Relationship.insertRelationshipToCatalog _tmTable ObjRel objRel
      withPathK "array_relationships" $
-        indexedForM_ (table ^. tmArrayRelationships) $ \arrRel ->
-        Relationship.insertRelationshipToCatalog (table ^. tmTable) ArrRel arrRel
+        indexedForM_ _tmArrayRelationships $ \arrRel ->
+        Relationship.insertRelationshipToCatalog _tmTable ArrRel arrRel
+
      -- Computed Fields
      withPathK "computed_fields" $
-        indexedForM_ (table ^. tmComputedFields) $
+        indexedForM_ _tmComputedFields $
          \(ComputedFieldMeta name definition comment) ->
            ComputedField.addComputedFieldToCatalog $
-              ComputedField.AddComputedField (table ^. tmTable) name definition comment
+              ComputedField.AddComputedField _tmTable name definition comment

-    -- Permissions
-    indexedForM_ tables $ \table -> do
-      let tableName = table ^. tmTable
-      tabInfo <- modifyErrAndSet500 ("apply " <> ) $ askTableCoreInfo tableName
-      withPathK "insert_permissions" $ processPerms tabInfo $
-        table ^. tmInsertPermissions
-      withPathK "select_permissions" $ processPerms tabInfo $
-        table ^. tmSelectPermissions
-      withPathK "update_permissions" $ processPerms tabInfo $
-        table ^. tmUpdatePermissions
-      withPathK "delete_permissions" $ processPerms tabInfo $
-        table ^. tmDeletePermissions
+      -- Permissions
+      withPathK "insert_permissions" $ processPerms _tmTable _tmInsertPermissions
+      withPathK "select_permissions" $ processPerms _tmTable _tmSelectPermissions
+      withPathK "update_permissions" $ processPerms _tmTable _tmUpdatePermissions
+      withPathK "delete_permissions" $ processPerms _tmTable _tmDeletePermissions

-    indexedForM_ tables $ \table ->
+      -- Event triggers
      withPathK "event_triggers" $
-        indexedForM_ (table ^. tmEventTriggers) $ \etc ->
-        subTableP2 (table ^. tmTable) False etc
+        indexedForM_ _tmEventTriggers $ \etc -> subTableP2 _tmTable False etc

  -- sql functions
  withPathK "functions" $ case functionsMeta of
-      FMVersion1 qualifiedFunctions -> indexedForM_ qualifiedFunctions $
-        \qf -> void $ Schema.trackFunctionP2 qf Schema.emptyFunctionConfig
-      FMVersion2 functionsV2 -> indexedForM_ functionsV2 $
-        \(Schema.TrackFunctionV2 function config) -> void $ Schema.trackFunctionP2 function config
+    FMVersion1 qualifiedFunctions -> indexedForM_ qualifiedFunctions $
+      \qf -> Schema.saveFunctionToCatalog qf Schema.emptyFunctionConfig
+    FMVersion2 functionsV2 -> indexedForM_ functionsV2 $
+      \(Schema.TrackFunctionV2 function config) -> Schema.saveFunctionToCatalog function config

  -- query collections
  systemDefined <- askSystemDefined
@ -200,32 +186,30 @@ applyQP2 (ReplaceMetadata _ tables functionsMeta

  -- remote schemas
  withPathK "remote_schemas" $
-    indexedMapM_ (void . addRemoteSchemaP2) schemas
+    indexedMapM_ (liftTx . addRemoteSchemaToCatalog) schemas

-  CustomTypes.persistCustomTypes customTypes
-
-  for_ actions $ \action -> do
-    let createAction =
-          CreateAction (_amName action) (_amDefinition action) (_amComment action)
-    Action.persistCreateAction createAction
-    for_ (_amPermissions action) $ \permission -> do
-      let createActionPermission = CreateActionPermission (_amName action)
-                                   (_apmRole permission) Nothing (_apmComment permission)
-      Action.persistCreateActionPermission createActionPermission
-
-  buildSchemaCacheStrict
-  return successMsg
+  -- custom types
+  withPathK "custom_types" $
+    CustomTypes.persistCustomTypes customTypes

+  -- actions
+  withPathK "actions" $
+    indexedForM_ actions $ \action -> do
+      let createAction =
+            CreateAction (_amName action) (_amDefinition action) (_amComment action)
+      Action.persistCreateAction createAction
+      withPathK "permissions" $
+        indexedForM_ (_amPermissions action) $ \permission -> do
+          let createActionPermission = CreateActionPermission (_amName action)
+                                       (_apmRole permission) Nothing (_apmComment permission)
+          Action.persistCreateActionPermission createActionPermission
  where
-    processPerms tabInfo perms = indexedForM_ perms $ Permission.addPermP2 (_tciName tabInfo)
+    processPerms tableName perms = indexedForM_ perms $ Permission.addPermP2 tableName

 runReplaceMetadata
-  :: ( HasVersion
-     , MonadIO m
-     , MonadTx m
+  :: ( MonadTx m
     , CacheRWM m
     , HasSystemDefined m
-     , HasHttpManager m
     )
  => ReplaceMetadata -> m EncJSON
 runReplaceMetadata q = do
--- a/server/src-lib/Hasura/RQL/DDL/RemoteSchema.hs
+++ b/server/src-lib/Hasura/RQL/DDL/RemoteSchema.hs
@ -6,7 +6,7 @@ module Hasura.RQL.DDL.RemoteSchema
  , fetchRemoteSchemas
  , addRemoteSchemaP1
  , addRemoteSchemaP2Setup
-  , addRemoteSchemaP2
+  , addRemoteSchemaToCatalog
  ) where

 import           Hasura.EncJSON
--- a/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs
+++ b/server/src-lib/Hasura/RQL/DDL/Schema/Function.hs
@ -157,9 +157,12 @@ mkFunctionInfo qf systemDefined config rawFuncInfo =
        let argsText = T.intercalate "," $ map getFuncArgNameTxt args
        in "the function arguments " <> argsText <> " are not in compliance with GraphQL spec"

-saveFunctionToCatalog :: QualifiedFunction -> FunctionConfig -> SystemDefined -> Q.TxE QErr ()
-saveFunctionToCatalog (QualifiedObject sn fn) config systemDefined =
-  Q.unitQE defaultTxErrorHandler [Q.sql|
+saveFunctionToCatalog
+  :: (MonadTx m, HasSystemDefined m)
+  => QualifiedFunction -> FunctionConfig -> m ()
+saveFunctionToCatalog (QualifiedObject sn fn) config = do
+  systemDefined <- askSystemDefined
+  liftTx $ Q.unitQE defaultTxErrorHandler [Q.sql|
         INSERT INTO "hdb_catalog"."hdb_function"
           (function_schema, function_name, configuration, is_system_defined)
         VALUES ($1, $2, $3, $4)
@ -205,8 +208,7 @@ trackFunctionP1 qf = do
 trackFunctionP2 :: (MonadTx m, CacheRWM m, HasSystemDefined m)
                => QualifiedFunction -> FunctionConfig -> m EncJSON
 trackFunctionP2 qf config = do
-  systemDefined <- askSystemDefined
-  liftTx $ saveFunctionToCatalog qf config systemDefined
+  saveFunctionToCatalog qf config
  buildSchemaCacheFor $ MOFunction qf
  return successMsg

--- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs
+++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs
@ -4,7 +4,6 @@
 module Hasura.RQL.DDL.Schema.Table
  ( TrackTable(..)
  , runTrackTableQ
-  , trackExistingTableOrViewP2

  , TrackTableV2(..)
  , runTrackTableV2Q
@ -38,6 +37,7 @@ import           Hasura.SQL.Types

 import qualified Database.PG.Query                  as Q
 import qualified Hasura.GraphQL.Schema              as GS
+import qualified Hasura.GraphQL.Context             as GC
 import qualified Hasura.Incremental                 as Inc
 import qualified Language.GraphQL.Draft.Syntax      as G

@ -103,9 +103,8 @@ trackExistingTableOrViewP2
  :: (MonadTx m, CacheRWM m, HasSystemDefined m)
  => QualifiedTable -> Bool -> TableConfig -> m EncJSON
 trackExistingTableOrViewP2 tableName isEnum config = do
-  sc <- askSchemaCache
-  let defGCtx = scDefaultRemoteGCtx sc
-  GS.checkConflictingNode defGCtx $ GS.qualObjectToName tableName
+  typeMap <- GC._gTypes . scDefaultRemoteGCtx <$> askSchemaCache
+  GS.checkConflictingNode typeMap $ GS.qualObjectToName tableName
  saveTableToCatalog tableName isEnum config
  buildSchemaCacheFor (MOTable tableName)
  return successMsg
@ -212,9 +211,9 @@ processTableChanges ti tableDiff = do

      withNewTabName newTN = do
        let tnGQL = GS.qualObjectToName newTN
-            defGCtx = scDefaultRemoteGCtx sc
+            typeMap = GC._gTypes $ scDefaultRemoteGCtx sc
        -- check for GraphQL schema conflicts on new name
-        GS.checkConflictingNode defGCtx tnGQL
+        GS.checkConflictingNode typeMap tnGQL
        procAlteredCols sc tn
        -- update new table in catalog
        renameTableInCatalog newTN tn