Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug fix: auto_rebuild_query_cache gate was broken #8339

Merged
merged 3 commits into from
Feb 13, 2025
Merged

bug fix: auto_rebuild_query_cache gate was broken #8339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c05576
Skip SQL cache in cache recompilation
fantix Feb 13, 2025
e3c8e82
Complete the test
fantix Feb 13, 2025
6aea3df
bug fix: auto_rebuild_query_cache gate was broken
fantix Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions edb/server/config/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def lookup(
spec: Spec,
allow_unrecognized: bool = False,
) -> Any:
assert len(configs) > 0

try:
setting = spec[name]
Expand Down
16 changes: 12 additions & 4 deletions edb/server/dbview/dbview.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1262,6 +1262,9 @@ cdef class DatabaseConnectionView:
# Only recompile queries from the *latest* version,
# to avoid quadratic slowdown problems.
and req.schema_version == self.schema_version
# SQL queries require _amend_typedesc_in_sql() with a
# backend connection, which is not available here.
and req.input_language != enums.InputLanguage.SQL
):
g.create_task(recompile_request(req))

Expand Down Expand Up @@ -1437,7 +1440,7 @@ cdef class DatabaseConnectionView:
if unit.user_schema:
user_schema = unit.user_schema
user_schema_version = unit.user_schema_version
if user_schema and not self.server.config_lookup(
if user_schema and not self.config_lookup(
"auto_rebuild_query_cache",
):
user_schema = None
Expand All @@ -1447,9 +1450,14 @@ cdef class DatabaseConnectionView:
)

if use_metrics:
metrics.edgeql_query_compilations.inc(
1.0, self.tenant.get_instance_name(), 'compiler'
)
if query_req.input_language is enums.InputLanguage.EDGEQL:
metrics.edgeql_query_compilations.inc(
1.0, self.tenant.get_instance_name(), 'compiler'
)
else:
metrics.sql_compilations.inc(
1.0, self.tenant.get_instance_name()
)

source = query_req.source
if query_unit_group.force_non_normalized:
Expand Down
70 changes: 65 additions & 5 deletions tests/test_server_ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,12 +745,34 @@ def measure_sql_compilations(
con = await sd.connect()
try:
qry = 'select schema::Object { name }'
sql = '''
SELECT
n.nspname::text AS table_schema,
c.relname::text AS table_name,
CASE
WHEN c.relkind = 'r' THEN 'table'
WHEN c.relkind = 'v' THEN 'view'
WHEN c.relkind = 'm' THEN 'materialized_view'
END AS type,
c.relrowsecurity AS rls_enabled
FROM
pg_catalog.pg_class c
JOIN
pg_catalog.pg_namespace n
ON n.oid::text = c.relnamespace::text
WHERE
c.relkind IN ('r', 'v', 'm')
AND n.nspname = 'public';
'''

await con.query(qry)
await con.query_sql(sql)

# Querying a second time should hit the cache
with self.assertChange(measure_compilations(sd), 0):
await con.query(qry)
with self.assertChange(measure_sql_compilations(sd), 0):
await con.query_sql(sql)

await con.query('''
create type X
Expand All @@ -764,6 +786,14 @@ def measure_sql_compilations(
with self.assertChange(measure_compilations(sd), 0):
await con.query(qry)

# The SQL cache is reset after DDL, because recompiling SQL
# requires backend connection for amending in/out tids, and
# we don't have the infra for that yet.
with self.assertChange(measure_sql_compilations(sd), 1):
await con.query_sql(sql)
with self.assertChange(measure_sql_compilations(sd), 0):
await con.query_sql(sql)

# TODO: this does not behave the way I thing it should
# with self.assertChange(measure_compilations(sd), 1):
# con_c = con.with_config(apply_access_policies=False)
Expand Down Expand Up @@ -794,6 +824,12 @@ def measure_sql_compilations(

with self.assertChange(measure_compilations(sd), 1):
await con.query(qry)
with self.assertChange(measure_compilations(sd), 0):
await con.query(qry)
with self.assertChange(measure_sql_compilations(sd), 1):
await con.query_sql(sql)
with self.assertChange(measure_sql_compilations(sd), 0):
await con.query_sql(sql)

await con.execute(
"configure current database "
Expand All @@ -802,24 +838,24 @@ def measure_sql_compilations(

# Now, a similar thing for SQL queries

with self.assertChange(measure_compilations(sd), 1):
with self.assertChange(measure_sql_compilations(sd), 1):
await con.query_sql('select 1')

# cache hit
with self.assertChange(measure_compilations(sd), 0):
with self.assertChange(measure_sql_compilations(sd), 0):
await con.query_sql('select 1')

# changing globals: cache hit
with self.assertChange(measure_compilations(sd), 0):
with self.assertChange(measure_sql_compilations(sd), 0):
con_g = con.with_globals({'g': 'hello'})
await con_g.query_sql('select 1')

# normalization: pg_query_normalize is underwhelming
with self.assertChange(measure_compilations(sd), 1):
with self.assertChange(measure_sql_compilations(sd), 1):
await con.query_sql('sElEct 1')

# constant extraction: cache hit
with self.assertChange(measure_compilations(sd), 0):
with self.assertChange(measure_sql_compilations(sd), 0):
await con.query_sql('select 2')

# TODO: this does not behave the way I though it should
Expand All @@ -828,6 +864,27 @@ def measure_sql_compilations(
# con_c = con.with_config(apply_access_policies=False)
# await con_c.query_sql(qry_sql)

# At last, make sure recompilation switch works fine
await con.execute(
"configure current database "
"set auto_rebuild_query_cache := false"
)
await con.query('''
create type X
''')
with self.assertChange(measure_compilations(sd), 1):
await con.query(qry)
with self.assertChange(measure_compilations(sd), 0):
await con.query(qry)
with self.assertChange(measure_sql_compilations(sd), 1):
await con.query_sql(sql)
with self.assertChange(measure_sql_compilations(sd), 0):
await con.query_sql(sql)
await con.execute(
"configure current database "
"reset auto_rebuild_query_cache"
)

finally:
await con.aclose()

Expand Down Expand Up @@ -882,6 +939,9 @@ def measure_sql_compilations(
# It should hit the cache no problem.
with self.assertChange(measure_compilations(sd), 0):
await con.query(qry)
# TODO(fantix): persistent SQL cache not working?
# with self.assertChange(measure_sql_compilation(sd), 0):
# await con.query_sql(sql)

finally:
await con.aclose()
Expand Down
Loading