From be2b7cdc941bbd9385c4f6affc89b412b0e9f51b Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Fri, 11 Oct 2024 17:56:05 -0700 Subject: [PATCH] Trampoline instdata to fix error messages on catalog mismatch (#7850) Putting catalog version in the schema names broke the nice error message on catalog mismatch, and the original schema version broke it on major version mismatches. Trampoline the instdata table so we can read the data even if our version is wrong. --- edb/buildmeta.py | 2 +- edb/pgsql/metaschema.py | 63 +++++++++++++++++++++++++++++++ edb/server/bootstrap.py | 82 +++++++++++++++-------------------------- 3 files changed, 93 insertions(+), 54 deletions(-) diff --git a/edb/buildmeta.py b/edb/buildmeta.py index 391e3df226f..b73a40be734 100644 --- a/edb/buildmeta.py +++ b/edb/buildmeta.py @@ -60,7 +60,7 @@ # The merge conflict there is a nice reminder that you probably need # to write a patch in edb/pgsql/patches.py, and then you should preserve # the old value. -EDGEDB_CATALOG_VERSION = 2024_10_10_00_01 +EDGEDB_CATALOG_VERSION = 2024_10_11_00_01 EDGEDB_MAJOR_VERSION = 6 diff --git a/edb/pgsql/metaschema.py b/edb/pgsql/metaschema.py index 29d03221ff3..79422bd4323 100644 --- a/edb/pgsql/metaschema.py +++ b/edb/pgsql/metaschema.py @@ -38,6 +38,7 @@ from edb.common import debug from edb.common import exceptions +from edb.common import ordered from edb.common import uuidgen from edb.common import xdedent from edb.common.typeutils import not_none @@ -150,6 +151,38 @@ def __init__(self) -> None: ) +class InstDataTable(dbops.Table): + def __init__(self) -> None: + sname = V('edgedbinstdata') + super().__init__( + name=(sname, 'instdata'), + columns=[ + dbops.Column( + name='key', + type='text', + ), + dbops.Column( + name='bin', + type='bytea', + ), + dbops.Column( + name='text', + type='text', + ), + dbops.Column( + name='json', + type='jsonb', + ), + ], + constraints=ordered.OrderedSet([ + dbops.PrimaryKey( + table_name=(sname, 'instdata'), + columns=['key'], + ), + ]), + ) + + class DMLDummyTable(dbops.Table): """A empty dummy table used when we need to emit no-op DML. @@ -4852,6 +4885,12 @@ def _maybe_trampoline( and cmd.view.name[0].endswith(namespace) ): out.append(trampoline.make_view_trampoline(cmd.view)) + elif ( + isinstance(cmd, dbops.CreateTable) + and cmd.table.name[0].endswith(namespace) + ): + f, n = cmd.table.name + out.append(trampoline.make_table_trampoline((f, n))) def trampoline_functions( @@ -4886,6 +4925,7 @@ def get_fixed_bootstrap_commands() -> dbops.CommandGroup: dbops.CreateSchema(name='edgedbt'), dbops.CreateSchema(name='edgedbpub'), dbops.CreateSchema(name='edgedbstd'), + dbops.CreateSchema(name='edgedbinstdata'), dbops.CreateTable( DBConfigTable(), @@ -4922,6 +4962,29 @@ def get_fixed_bootstrap_commands() -> dbops.CommandGroup: return commands +def get_instdata_commands( +) -> tuple[dbops.CommandGroup, list[trampoline.Trampoline]]: + cmds = [ + dbops.CreateSchema(name=V('edgedbinstdata')), + dbops.CreateTable(InstDataTable()), + ] + + commands = dbops.CommandGroup() + commands.add_commands(cmds) + + return commands, trampoline_functions(cmds) + + +async def generate_instdata_table( + conn: PGConnection, +) -> list[trampoline.Trampoline]: + commands, trampolines = get_instdata_commands() + block = dbops.PLTopBlock() + commands.generate(block) + await _execute_block(conn, block) + return trampolines + + def get_bootstrap_commands( config_spec: edbconfig.Spec, ) -> tuple[dbops.CommandGroup, list[trampoline.Trampoline]]: diff --git a/edb/server/bootstrap.py b/edb/server/bootstrap.py index 4d0d1b22ea7..6b92d6d7eb2 100644 --- a/edb/server/bootstrap.py +++ b/edb/server/bootstrap.py @@ -56,7 +56,6 @@ from edb.common import debug from edb.common import devmode -from edb.common import ordered from edb.common import retryloop from edb.common import uuidgen @@ -242,7 +241,7 @@ async def _execute_block(conn, block: dbops.SQLBlock) -> None: await _execute(conn, stmt) -async def _execute_edgeql_ddl( +def _execute_edgeql_ddl( schema: s_schema.Schema_T, ddltext: str, stdmode: bool = True, @@ -1222,7 +1221,7 @@ class StdlibBits(NamedTuple): # (Oh, maybe testmode screws this idea up?) -async def _make_stdlib( +def _make_stdlib( ctx: BootstrapContext, testmode: bool, global_ids: Mapping[str, uuid.UUID], @@ -1282,7 +1281,7 @@ async def _make_stdlib( }};''', ]) - schema = await _execute_edgeql_ddl(schema, stdglobals) + schema = _execute_edgeql_ddl(schema, stdglobals) _, global_schema_version = s_std.make_global_schema_version(schema) schema, plan, tplan = _process_delta(ctx, global_schema_version, schema) @@ -1572,7 +1571,7 @@ async def _init_stdlib( stdlib_was_none = stdlib is None if stdlib is None: logger.info('Compiling the standard library...') - stdlib = await _make_stdlib( + stdlib = _make_stdlib( ctx, in_dev_mode or testmode, global_ids) config_spec = config.load_spec_from_schema(stdlib.stdschema) @@ -1590,12 +1589,21 @@ async def _init_stdlib( trampolines=bootstrap_trampolines + stdlib.trampolines ) + trampolines = [] + + # We need to set this up early, since later code depends on the + # backend_instance_params of the instdata table. But it also + # obviously can't go into the tpldbdump, since it is dynamic. + trampolines.extend(await metaschema.generate_instdata_table( + conn, + )) + await _populate_misc_instance_data(ctx) + backend_params = cluster.get_runtime_params() if not args.inplace_upgrade_prepare: logger.info('Creating the necessary PostgreSQL extensions...') await metaschema.create_pg_extensions(conn, backend_params) - trampolines = [] trampolines.extend(stdlib.trampolines) eff_tpldbdump = ( @@ -2095,43 +2103,6 @@ async def _populate_misc_instance_data( ctx: BootstrapContext, ) -> Dict[str, Any]: - sname = pg_common.versioned_schema('edgedbinstdata') - commands = dbops.CommandGroup() - commands.add_commands([ - dbops.CreateSchema(name=sname), - dbops.CreateTable(dbops.Table( - name=(sname, 'instdata'), - columns=[ - dbops.Column( - name='key', - type='text', - ), - dbops.Column( - name='bin', - type='bytea', - ), - dbops.Column( - name='text', - type='text', - ), - dbops.Column( - name='json', - type='jsonb', - ), - ], - constraints=ordered.OrderedSet([ - dbops.PrimaryKey( - table_name=(sname, 'instdata'), - columns=['key'], - ), - ]), - )) - ]) - - block = dbops.PLTopBlock() - commands.generate(block) - await _execute_block(ctx.conn, block) - mock_auth_nonce = scram.generate_nonce() json_instance_data = { 'version': dict(buildmeta.get_version_dict()), @@ -2166,6 +2137,7 @@ async def _populate_misc_instance_data( json.dumps(json_single_role_metadata), ) + assert backend_params.has_create_database if not backend_params.has_create_database: await _store_static_json_cache( ctx, @@ -2251,11 +2223,16 @@ def _pg_log_listener(severity, message): logger.log(level, message) -async def _get_instance_data(conn: metaschema.PGConnection) -> Dict[str, Any]: +async def _get_instance_data( + conn: metaschema.PGConnection, + *, + versioned: bool=True, +) -> Dict[str, Any]: + schema = 'edgedbinstdata_VER' if versioned else 'edgedbinstdata' data = await conn.sql_fetch_val( - trampoline.fixup_query(""" + trampoline.fixup_query(f""" SELECT json::json - FROM edgedbinstdata_VER.instdata + FROM {schema}.instdata WHERE key = 'instancedata' """).encode('utf-8'), ) @@ -2325,7 +2302,8 @@ async def _check_catalog_compatibility( ) try: - instancedata = await _get_instance_data(conn) + # versioned=False so we can properly fail on version/catalog mismatches. + instancedata = await _get_instance_data(conn, versioned=False) datadir_version = instancedata.get('version') if datadir_version: datadir_major = datadir_version.get('major') @@ -2351,8 +2329,8 @@ async def _check_catalog_compatibility( f'{expected_ver.major}' ), hint=( - f'You need to recreate the instance and upgrade ' - f'using dump/restore.' + f'You need to either recreate the instance and upgrade ' + f'using dump/restore, or do an inplace upgrade.' ) ) @@ -2368,8 +2346,8 @@ async def _check_catalog_compatibility( f'format version {expected_catver}' ), hint=( - f'You need to recreate the instance and upgrade ' - f'using dump/restore.' + f'You need to either recreate the instance and upgrade ' + f'using dump/restore, or do an inplace upgrade.' ) ) except Exception: @@ -2509,8 +2487,6 @@ async def _bootstrap( tmp_table_query = pgcon.SETUP_TEMP_TABLE_SCRIPT await _execute(tpl_ctx.conn, tmp_table_query) - await _populate_misc_instance_data(tpl_ctx) - stdlib, config_spec, compiler = await _init_stdlib( tpl_ctx, testmode=args.testmode,