Merge pull request #66934 from rschu1ze/issue-66887

Un-flake test_runtime_configurable_cache_size

tests/integration/test_runtime_configurable_cache_size/configs/empty_query_cache.xml

@@ -1,7 +1,7 @@
 <clickhouse>
 
     <query_cache>
-        <max_entries>1</max_entries>
+        <max_entries>0</max_entries>
     </query_cache>
 
 </clickhouse>

tests/integration/test_runtime_configurable_cache_size/test.py

@@ -94,54 +94,61 @@ CONFIG_DIR = os.path.join(SCRIPT_DIR, "configs")
 
 
 def test_query_cache_size_is_runtime_configurable(start_cluster):
-    # the initial config specifies the maximum query cache size as 2, run 3 queries, expect 2 cache entries
     node.query("SYSTEM DROP QUERY CACHE")
+
+    # The initial config allows at most two query cache entries but we don't mind
     node.query("SELECT 1 SETTINGS use_query_cache = 1, query_cache_ttl = 1")
-    node.query("SELECT 2 SETTINGS use_query_cache = 1, query_cache_ttl = 1")
-    node.query("SELECT 3 SETTINGS use_query_cache = 1, query_cache_ttl = 1")
 
     time.sleep(2)
-    node.query("SYSTEM RELOAD ASYNCHRONOUS METRICS")
-    res = node.query(
-        "SELECT value FROM system.asynchronous_metrics WHERE metric = 'QueryCacheEntries'",
-    )
-    assert res == "2\n"
+    # At this point, the query cache contains one entry and it is stale
 
-    # switch to a config with a maximum query cache size of 1
+    res = node.query(
+        "SELECT count(*) FROM system.query_cache",
+    )
+    assert res == "1\n"
+
+    # switch to a config with a maximum query cache size of _0_
     node.copy_file_to_container(
-        os.path.join(CONFIG_DIR, "smaller_query_cache.xml"),
+        os.path.join(CONFIG_DIR, "empty_query_cache.xml"),
         "/etc/clickhouse-server/config.d/default.xml",
     )
 
     node.query("SYSTEM RELOAD CONFIG")
 
-    # check that eviction worked as expected
-    time.sleep(2)
-    node.query("SYSTEM RELOAD ASYNCHRONOUS METRICS")
     res = node.query(
-        "SELECT value FROM system.asynchronous_metrics WHERE metric = 'QueryCacheEntries'",
-    )
-    assert (
-        res == "2\n"
-    )  # "Why not 1?", you think. Reason is that QC uses the TTLCachePolicy that evicts lazily only upon insert.
-    # Not a real issue, can be changed later, at least there's a test now.
-
-    # Also, you may also wonder "why query_cache_ttl = 1"? Reason is that TTLCachePolicy only removes *stale* entries. With the default TTL
-    # (60 sec), no entries would be removed at all. Again: not a real issue, can be changed later and there's at least a test now.
-
-    # check that the new query cache maximum size is respected when more queries run
-    node.query("SELECT 4 SETTINGS use_query_cache = 1, query_cache_ttl = 1")
-    node.query("SELECT 5 SETTINGS use_query_cache = 1, query_cache_ttl = 1")
-
-    time.sleep(2)
-    node.query("SYSTEM RELOAD ASYNCHRONOUS METRICS")
-    res = node.query(
-        "SELECT value FROM system.asynchronous_metrics WHERE metric = 'QueryCacheEntries'",
+        "SELECT count(*) FROM system.query_cache",
     )
     assert res == "1\n"
+    # "Why not 0?", I hear you say. Reason is that QC uses the TTLCachePolicy that evicts lazily only upon insert.
+    # Not a real issue, can be changed later, at least there's a test now.
 
-    # restore the original config
+    # The next SELECT will find a single stale entry which is one entry too much according to the new config.
+    # This triggers the eviction of all stale entries, in this case the 'SELECT 1' result.
+    # Then, it tries to insert the 'SELECT 2' result but it also cannot be added according to the config.
+    node.query("SELECT 2 SETTINGS use_query_cache = 1, query_cache_ttl = 1")
+    res = node.query(
+        "SELECT count(*) FROM system.query_cache",
+    )
+    assert res == "0\n"
+
+    # The new maximum cache size is respected when more queries run
+    node.query("SELECT 3 SETTINGS use_query_cache = 1, query_cache_ttl = 1")
+    res = node.query(
+        "SELECT count(*) FROM system.query_cache",
+    )
+    assert res == "0\n"
+
+    # Restore the original config
     node.copy_file_to_container(
         os.path.join(CONFIG_DIR, "default.xml"),
         "/etc/clickhouse-server/config.d/default.xml",
     )
+
+    node.query("SYSTEM RELOAD CONFIG")
+
+    # It is possible to insert entries again
+    node.query("SELECT 4 SETTINGS use_query_cache = 1, query_cache_ttl = 1")
+    res = node.query(
+        "SELECT count(*) FROM system.query_cache",
+    )
+    assert res == "1\n"

tests/queries/0_stateless/02494_query_cache_asynchronous_metrics.reference

@@ -0,0 +1,2 @@
+1
+1

tests/queries/0_stateless/02494_query_cache_asynchronous_metrics.sql

@@ -0,0 +1,13 @@
+-- Tags: no-parallel
+-- Tag no-parallel: Messes with internal cache
+
+SYSTEM DROP QUERY CACHE;
+
+-- Create an entry in the query cache
+SELECT 1 SETTINGS use_query_cache = true;
+
+-- Asynchronous metrics must know about the entry
+SYSTEM RELOAD ASYNCHRONOUS METRICS;
+SELECT value FROM system.asynchronous_metrics WHERE metric = 'QueryCacheEntries';
+
+SYSTEM DROP QUERY CACHE;