From 415aaec4f3689aa8b205455648725070d7de8698 Mon Sep 17 00:00:00 2001 From: Alexis King Date: Thu, 15 Oct 2020 16:35:12 -0500 Subject: [PATCH] server: Fix fine-grained incremental cache invalidation (fix #3759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This issue was very tricky to track down, but fortunately easy to fix. The interaction here is subtle enough that it’s difficult to put into English what would go wrong in what circumstances, but the new unit test captures precisely that interaction to ensure it remains fixed. --- CHANGELOG.md | 4 ++ .../Hasura/Incremental/Internal/Cache.hs | 6 ++- server/src-test/Hasura/IncrementalSpec.hs | 23 ++++++++++ .../introspect_enum_values.yaml | 44 +++++++++++++++++++ .../set_and_delayed_reload/setup.yaml | 24 ++++++++++ .../set_and_delayed_reload/teardown.yaml | 8 ++++ server/tests-py/test_v1_queries.py | 10 +++++ 7 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/introspect_enum_values.yaml create mode 100644 server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/setup.yaml create mode 100644 server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/teardown.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index d4edcbac8b49d..ce8296cfd8337 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,10 @@ This release contains the [PDV refactor (#4111)](https://github.com/hasura/graph (Add entries here in the order of: server, console, cli, docs, others) +- server: Fix fine-grained incremental cache invalidation (fix #3759) + + This issue could cause enum table values to sometimes not be properly reloaded without restarting `graphql-engine`. Now a `reload_metadata` API call (or clicking “Reload enum values” in the console) should consistently force a reload of all enum table values. + - server: add `--websocket-compression` command-line flag for enabling websocket compression (fix #3292) - server: some mutations that cannot be performed will no longer be in the schema (for instance, `delete_by_pk` mutations won't be shown to users that do not have select permissions on all primary keys) (#4111) - server: miscellaneous description changes (#4111) diff --git a/server/src-lib/Hasura/Incremental/Internal/Cache.hs b/server/src-lib/Hasura/Incremental/Internal/Cache.hs index f0a5f6bf50f1e..dbff448e70b7b 100644 --- a/server/src-lib/Hasura/Incremental/Internal/Cache.hs +++ b/server/src-lib/Hasura/Incremental/Internal/Cache.hs @@ -70,10 +70,12 @@ instance (MonadUnique m) => ArrowCache m (Rule m) where (k $! (s <> s')) (b, s') (listenAccesses r') cached accesses a b (Rule r) = Rule \s a' k -> if - | unchanged accesses a a' -> k s b (cached accesses a b (Rule r)) + | unchanged accesses a a' -> (k $! (s <> accesses)) b (cached accesses a b (Rule r)) | otherwise -> r s a' \s' (b', accesses') r' -> k s' b' (cached accesses' a' b' r') - newDependency = arrM \a -> newUniqueS <&> \u -> Dependency (DependencyRoot u) a + newDependency = Rule \s a k -> do + key <- DependencyRoot <$> newUniqueS + k s (Dependency key a) (arr (Dependency key)) {-# INLINABLE newDependency #-} dependOn = Rule \s (Dependency key v) k -> (k $! recordAccess key AccessedAll s) v dependOn diff --git a/server/src-test/Hasura/IncrementalSpec.hs b/server/src-test/Hasura/IncrementalSpec.hs index e532e2762368d..ba9523dbd93af 100644 --- a/server/src-test/Hasura/IncrementalSpec.hs +++ b/server/src-test/Hasura/IncrementalSpec.hs @@ -31,6 +31,29 @@ spec = do (_, state3) <- runStateT (Inc.rebuild result2 (True, True)) 0 state3 `shouldBe` 2 + it "tracks dependencies within nested uses of cache across multiple executions" do + let rule :: (MonadWriter String m, MonadUnique m) + => Inc.Rule m (Inc.InvalidationKey, Inc.InvalidationKey) () + rule = proc (key1, key2) -> do + dep1 <- Inc.newDependency -< key2 + (key1, dep1) >- Inc.cache (proc (_, dep2) -> + dep2 >- Inc.cache (proc dep3 -> do + Inc.dependOn -< dep3 + arrM tell -< "executed")) + returnA -< () + + let key1 = Inc.initialInvalidationKey + key2 = Inc.invalidate key1 + + (result1, log1) <- runWriterT $ Inc.build rule (key1, key1) + log1 `shouldBe` "executed" + + (result2, log2) <- runWriterT $ Inc.rebuild result1 (key2, key1) + log2 `shouldBe` "" + + (_, log3) <- runWriterT $ Inc.rebuild result2 (key2, key2) + log3 `shouldBe` "executed" + describe "keyed" $ do it "preserves incrementalization when entries don’t change" $ do let rule :: (MonadWriter (S.HashSet (String, Integer)) m, MonadUnique m) diff --git a/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/introspect_enum_values.yaml b/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/introspect_enum_values.yaml new file mode 100644 index 0000000000000..c42cb503a9da8 --- /dev/null +++ b/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/introspect_enum_values.yaml @@ -0,0 +1,44 @@ +- description: Insert a new enum value without reloading + url: /v1/query + status: 200 + query: + type: run_sql + args: + sql: | + INSERT INTO crust VALUES ('thick'); + +- description: Check that the previous enum values are cached + url: /v1/graphql + status: 200 + response: + data: + crust: + enumValues: + - name: thin + query: &crust_query + query: | + { + crust: __type(name: "crust_enum") { + enumValues { + name + } + } + } + +- description: Reload the metadata + url: /v1/query + status: 200 + query: + type: reload_metadata + args: {} + +- description: Check that the enum values are updated + url: /v1/graphql + status: 200 + response: + data: + crust: + enumValues: + - name: thick + - name: thin + query: *crust_query diff --git a/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/setup.yaml b/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/setup.yaml new file mode 100644 index 0000000000000..fb3c8e6d55139 --- /dev/null +++ b/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/setup.yaml @@ -0,0 +1,24 @@ +type: bulk +args: +- type: run_sql + args: + sql: | + CREATE TABLE crust (name text NOT NULL PRIMARY KEY); + INSERT INTO crust VALUES ('thin'); +- type: add_existing_table_or_view + args: crust +- type: set_table_is_enum + args: + table: crust + is_enum: true +- type: run_sql + args: + sql: | + CREATE TABLE pizza + ( id serial NOT NULL PRIMARY KEY + , name text NOT NULL + , crust text NOT NULL REFERENCES crust ); + + INSERT INTO pizza (name, crust) VALUES ('margarita', 'thin'); +- type: add_existing_table_or_view + args: pizza diff --git a/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/teardown.yaml b/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/teardown.yaml new file mode 100644 index 0000000000000..2dd1ea5c30bfd --- /dev/null +++ b/server/tests-py/queries/v1/set_table_is_enum/set_and_delayed_reload/teardown.yaml @@ -0,0 +1,8 @@ +type: bulk +args: +- type: run_sql + args: + sql: | + DROP TABLE pizza; + DROP TABLE crust; + cascade: true diff --git a/server/tests-py/test_v1_queries.py b/server/tests-py/test_v1_queries.py index e18966e708375..1a5f393b349bd 100644 --- a/server/tests-py/test_v1_queries.py +++ b/server/tests-py/test_v1_queries.py @@ -729,6 +729,16 @@ def test_add_test_schema_enum_table(self, hge_ctx): def test_relationship_with_inconsistent_enum_table(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/relationship_with_inconsistent_enum_table.yaml') +# regression test for issue #3759 +@usefixtures('per_method_tests_db_state') +class TestSetTableIsEnumSetAndDelayedReload: + @classmethod + def dir(cls): + return 'queries/v1/set_table_is_enum/set_and_delayed_reload' + + def test_introspect_enum_values(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/introspect_enum_values.yaml') + @usefixtures('per_method_tests_db_state') class TestSetTableCustomFields: