Fix/hot schema reload, missing metadata keys (#2283)

* fix metadata keys

* update changelog

* update increment sql

* update typing and tidy up

* update on review,  tidy up

* fix sync and migration tests

packages/node-core/CHANGELOG.md

@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Fixed
+- Fix missing incrememnt keys on `_metadata` table (#2283)
 ### Added
 - Support for Full Text Search (#2280)
 

packages/node-core/src/db/sync-helper.spec.ts

@@ -154,11 +154,11 @@ describe('sync-helper', () => {
     const expectedStatement = [
       'CREATE TABLE IF NOT EXISTS "test"."test-table" ("id" text NOT NULL,\n      "amount" numeric NOT NULL,\n      "date" timestamp NOT NULL,\n      "from_id" text NOT NULL,\n      "_id" UUID NOT NULL,\n      "_block_range" int8range NOT NULL,\n      "last_transfer_block" integer, PRIMARY KEY ("_id"));',
 
-      `COMMENT ON COLUMN "test"."test-table"."id" IS 'id field is always required and must look like this';`,
-      `COMMENT ON COLUMN "test"."test-table"."amount" IS 'Amount that is transferred';`,
-      `COMMENT ON COLUMN "test"."test-table"."date" IS 'The date of the transfer';`,
-      `COMMENT ON COLUMN "test"."test-table"."from_id" IS 'The account that transfers are made from';`,
-      `COMMENT ON COLUMN "test"."test-table"."last_transfer_block" IS 'The most recent block on which we see a transfer involving this account';`,
+      `COMMENT ON COLUMN "test"."test-table"."id" IS E'id field is always required and must look like this';`,
+      `COMMENT ON COLUMN "test"."test-table"."amount" IS E'Amount that is transferred';`,
+      `COMMENT ON COLUMN "test"."test-table"."date" IS E'The date of the transfer';`,
+      `COMMENT ON COLUMN "test"."test-table"."from_id" IS E'The account that transfers are made from';`,
+      `COMMENT ON COLUMN "test"."test-table"."last_transfer_block" IS E'The most recent block on which we see a transfer involving this account';`,
     ];
     expect(statement).toStrictEqual(expectedStatement);
   });
@@ -190,7 +190,7 @@ describe('sync-helper', () => {
     const statement = generateCreateTableQuery(mockModel, 'test', false);
     expect(statement).toStrictEqual([
       `CREATE TABLE IF NOT EXISTS "test"."test-table" ("id" text NOT NULL, PRIMARY KEY ("id"));`,
-      `COMMENT ON COLUMN "test"."test-table"."id" IS 'id field is always required and must look like this';`,
+      `COMMENT ON COLUMN "test"."test-table"."id" IS E'id field is always required and must look like this';`,
     ]);
   });
   it('Reference statement', () => {

packages/node-core/src/db/sync-helper.ts

@@ -72,12 +72,18 @@ function escapedName(...args: string[]): string {
   return args.map((a) => `"${a}"`).join('.');
 }
 
-function commentOn(type: 'CONSTRAINT' | 'TABLE' | 'COLUMN' | 'FUNCTION', entity: string, comment: string): string {
-  return `COMMENT ON ${type} ${entity} IS E'${comment}'`;
+function commentOn(
+  type: 'CONSTRAINT' | 'TABLE' | 'COLUMN' | 'FUNCTION',
+  entity: string,
+  comment: string,
+  constraint?: string
+): string {
+  const constraintPart = constraint ? `${constraint} ON ` : '';
+  return `COMMENT ON ${type} ${constraintPart}${entity} IS E'${comment}';`;
 }
 
 export function commentConstraintQuery(schema: string, table: string, constraint: string, comment: string): string {
-  return commentOn('CONSTRAINT', escapedName(schema, table), comment);
+  return commentOn('CONSTRAINT', escapedName(schema, table), comment, constraint);
 }
 
 export function commentTableQuery(schema: string, table: string, comment: string): string {

packages/node-core/src/indexer/storeCache/cacheMetadata.test.ts

@@ -0,0 +1,65 @@
+// Copyright 2020-2024 SubQuery Pte Ltd authors & contributors
+// SPDX-License-Identifier: GPL-3.0
+
+import {CacheMetadataModel, DbOption, MetadataFactory} from '@subql/node-core';
+import {QueryTypes, Sequelize} from '@subql/x-sequelize';
+
+const option: DbOption = {
+  host: process.env.DB_HOST ?? '127.0.0.1',
+  port: process.env.DB_PORT ? Number(process.env.DB_PORT) : 5432,
+  username: process.env.DB_USER ?? 'postgres',
+  password: process.env.DB_PASS ?? 'postgres',
+  database: process.env.DB_DATABASE ?? 'postgres',
+  timezone: 'utc',
+};
+
+describe('cacheMetadata integration', () => {
+  let sequelize: Sequelize;
+  let schema: string;
+
+  beforeAll(async () => {
+    sequelize = new Sequelize(
+      `postgresql://${option.username}:${option.password}@${option.host}:${option.port}/${option.database}`,
+      option
+    );
+    await sequelize.authenticate();
+  });
+
+  afterEach(async () => {
+    await sequelize.dropSchema(schema, {logging: false});
+  });
+  afterAll(async () => {
+    await sequelize.close();
+  });
+
+  it('Ensure increment keys are created on _metadata table', async () => {
+    schema = '"metadata-test-1"';
+    await sequelize.createSchema(schema, {});
+    const metaDataRepo = await MetadataFactory(sequelize, schema, false, '1');
+
+    await metaDataRepo.sync();
+
+    const cacheMetadataModel = new CacheMetadataModel(metaDataRepo);
+
+    // create key at 0
+    await (cacheMetadataModel as any).incrementJsonbCount('schemaMigrationCount');
+
+    // increment by 1
+    await (cacheMetadataModel as any).incrementJsonbCount('schemaMigrationCount');
+
+    // increase by 100
+    await (cacheMetadataModel as any).incrementJsonbCount('schemaMigrationCount', 100);
+
+    const v = (await sequelize.query(
+      `
+            SELECT * FROM ${schema}."_metadata"
+            WHERE key = 'schemaMigrationCount';
+        `,
+      {
+        type: QueryTypes.SELECT,
+      }
+    )) as any[];
+    expect(v.length).toBe(1);
+    expect(v[0].value).toBe(101);
+  });
+});

packages/node-core/src/indexer/storeCache/cacheMetadata.ts

@@ -11,6 +11,7 @@ import {ICachedModelControl} from './types';
 
 type MetadataKey = keyof MetadataKeys;
 const incrementKeys: MetadataKey[] = ['processedBlockCount', 'schemaMigrationCount'];
+type IncrementalMetadataKey = 'processedBlockCount' | 'schemaMigrationCount';
 
 export class CacheMetadataModel extends Cacheable implements ICachedModelControl {
   private setCache: Partial<MetadataKeys> = {};
@@ -77,19 +78,25 @@ export class CacheMetadataModel extends Cacheable implements ICachedModelControl
     metadata.map((m) => this.set(m.key, m.value));
   }
 
-  setIncrement(key: 'processedBlockCount' | 'schemaMigrationCount', amount = 1): void {
+  setIncrement(key: IncrementalMetadataKey, amount = 1): void {
     this.setCache[key] = (this.setCache[key] ?? 0) + amount;
   }
 
-  private async incrementJsonbCount(key: string, amount = 1, tx?: Transaction): Promise<void> {
-    const table = this.model.getTableName();
+  private async incrementJsonbCount(key: IncrementalMetadataKey, amount = 1, tx?: Transaction): Promise<void> {
+    const schemaTable = this.model.getTableName();
 
     if (!this.model.sequelize) {
       throw new Error(`Sequelize is not available on ${this.model.name}`);
     }
 
     await this.model.sequelize.query(
-      `UPDATE ${table} SET value = (COALESCE(value->0):: int + ${amount})::text::jsonb WHERE key ='${key}'`,
+      `
+          INSERT INTO ${schemaTable} (key, value, "createdAt", "updatedAt")
+          VALUES ('${key}', '0'::jsonb, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
+            ON CONFLICT (key) DO
+          UPDATE SET value = (COALESCE(${schemaTable}.value->>0)::int + '${amount}')::text::jsonb,
+            "updatedAt" = CURRENT_TIMESTAMP
+          WHERE ${schemaTable}.key = '${key}';`,
       tx && {transaction: tx}
     );
   }
@@ -117,7 +124,11 @@ export class CacheMetadataModel extends Cacheable implements ICachedModelControl
         updateOnDuplicate: ['key', 'value'],
       }),
       ...incrementKeys
-        .map((key) => this.setCache[key] && this.incrementJsonbCount(key, this.setCache[key] as number, tx))
+        .map((key) =>
+          this.setCache[key] !== undefined
+            ? this.incrementJsonbCount(key as IncrementalMetadataKey, this.setCache[key] as number, tx)
+            : undefined
+        )
         .filter(Boolean),
       this.model.destroy({where: {key: this.removeCache}}),
     ]);