Skip to content

Commit 5d23732

Browse files
TheLastCicadaTheLastCicada
committed
fix: patch Sequelize DATE serialiser + keyset-paginate mirror backfill
Two addressable bugbot + CI findings on the mirror fixes branch: 1. MariaDB "Incorrect datetime value" errors continued after the prior `timezone: "+00:00"` config tweak. Root cause: Sequelize 6's base `DATE._stringify` emits "YYYY-MM-DD HH:mm:ss.SSS Z" (e.g. "2026-04-17 17:07:13.399 +00:00") which MariaDB strict mode rejects, and some runtime paths reach the base serialiser even when a dialect- specific MySQL class is wired up. Patch the base DATE._stringify to emit the MariaDB-safe "YYYY-MM-DD HH:mm:ss" format; shadow an own- property `_stringify` on `sqlite.DATE.prototype` that preserves the original offset-bearing format (SQLite's parse() round-trip depends on it). Add `tests/v2/integration/mirror-datetime-format.spec.js` to guard the patch against future Sequelize upgrades. 2. Backfill used offset-based pagination, which can silently skip rows if a row is deleted concurrently (the next OFFSET lands one row later than intended). Switch to keyset pagination (WHERE pk > :lastSeenPk ORDER BY pk ASC LIMIT N) in both V1 and V2. Composite-PK mirrors now throw explicitly instead of silently loading the whole table; the single-PK invariant is still asserted at load time by mirror-model-init.spec.js. Add a defensive break if a terminal row returns a null PK. Tighten V2's outer-catch log formatting to match V1's (defensive `error?.message` fallback plus stack on debug).
1 parent 44bcc2a commit 5d23732

4 files changed

Lines changed: 239 additions & 56 deletions

File tree

‎src/config/config.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,55 @@
1+
import Sequelize from 'sequelize';
2+
import moment from 'moment';
13
import { getConfig, getConfigV2 } from '../utils/config-loader';
24
import { getChiaRoot } from '../utils/chia-root.js';
35
import { logger } from './logger.js';
46
import { createHash } from 'crypto';
57

8+
// Force MariaDB-compatible datetime serialisation across all code paths.
9+
//
10+
// Sequelize 6's default BaseTypes.DATE._stringify formats dates as
11+
// "YYYY-MM-DD HH:mm:ss.SSS Z" (e.g. "2026-04-17 17:07:13.399 +00:00")
12+
// which recent MariaDB strict mode rejects:
13+
// Incorrect datetime value: '2026-04-17 17:07:13.399 +00:00'
14+
// for column `cadt_mirror_test`.`audit`.`createdAt` at row 1
15+
//
16+
// The mysql-specific subclass emits a safe format ("YYYY-MM-DD HH:mm:ss"),
17+
// but some runtime paths in Sequelize end up calling the BASE class's
18+
// _stringify even when the column belongs to a MySQL dialect. Observed
19+
// in CI: V2 mirror produced ~60 "Incorrect datetime value" errors per
20+
// live-api run even with `timezone: "+00:00"` set at the Sequelize
21+
// constructor level.
22+
//
23+
// Fix: patch BASE DATE._stringify to emit the MariaDB-safe format, and
24+
// explicitly restore the original "YYYY-MM-DD HH:mm:ss.SSS Z" format on
25+
// the SQLite subclass (whose `parse()` relies on the trailing offset
26+
// for round-trip). This leaves MySQL/MariaDB writes safe regardless of
27+
// which dispatch path Sequelize chooses, without breaking SQLite.
28+
const _origBaseStringify = Sequelize.DataTypes.DATE.prototype._stringify;
29+
Sequelize.DataTypes.DATE.prototype._stringify = function _stringify(
30+
date,
31+
options,
32+
) {
33+
if (!moment.isMoment(date)) {
34+
date = this._applyTimezone(date, options);
35+
}
36+
// Drop fractional seconds and the " Z" offset. MariaDB strict mode
37+
// rejects " +00:00". CADT doesn't use sub-second resolution on any
38+
// DATE column.
39+
return date.format('YYYY-MM-DD HH:mm:ss');
40+
};
41+
// SQLite's DATE class only overrides `parse`, not `_stringify`, so the
42+
// base-class patch above would otherwise silently change the format of
43+
// SQLite writes. SQLite's `parse()` appends options.timezone when the
44+
// string lacks a "+"/"-" offset - if we stripped the offset on write,
45+
// parse() would concatenate the offset on read, producing a value
46+
// different from what was written. Shadow an own-property `_stringify`
47+
// on `sqlite.DATE.prototype` that points to the original base
48+
// serialiser, so SQLite retains the "YYYY-MM-DD HH:mm:ss.SSS Z" format.
49+
if (Sequelize.DataTypes.sqlite && Sequelize.DataTypes.sqlite.DATE) {
50+
Sequelize.DataTypes.sqlite.DATE.prototype._stringify = _origBaseStringify;
51+
}
52+
653
const chiaRoot = getChiaRoot();
754
const persistanceFolder = `${chiaRoot}/cadt/v1`;
855
const v2PersistanceFolder = `${chiaRoot}/cadt/v2`;

‎src/database/index.js‎

Lines changed: 67 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -442,41 +442,82 @@ export const backfillMirror = async () => {
442442
const orphansRemoved = await sweepMirrorOrphans(source, mirror, name);
443443
totalOrphansRemoved += orphansRemoved;
444444

445-
const count = await source.count();
446-
if (count === 0) {
447-
logger.debug(`Mirror backfill: ${name} - no records to sync`);
448-
continue;
449-
}
450-
451445
const updateFields = Object.keys(mirror.rawAttributes).filter(
452446
(attr) => !mirror.primaryKeyAttributes.includes(attr),
453447
);
454448

455-
// Paginate with an explicit ORDER BY on the primary key - neither
456-
// SQLite nor MySQL guarantees consistent ordering across pages
457-
// without it, and concurrent writes mid-backfill could otherwise
458-
// shift rows between pages and cause some to be silently skipped.
449+
// Keyset pagination (WHERE pk > :lastSeenPk ORDER BY pk ASC LIMIT N)
450+
// instead of offset-based. Offset pagination under concurrent
451+
// writes can silently skip rows: a delete at position N shifts
452+
// later rows down, so the next page's OFFSET lands one row later
453+
// than intended. Keyset pagination avoids that page-shift-on-delete
454+
// hazard by using a lower-bound predicate instead of a position
455+
// count.
456+
//
457+
// Note: keyset pagination is NOT immune to concurrent INSERTs with
458+
// a PK less than `lastPk`. Such rows will be missed in the current
459+
// pass but picked up by the next reconnect backfill. A subsequent
460+
// steady-state safeMirrorDbHandler write also directly upserts
461+
// them, so the miss window is bounded.
462+
//
463+
// Requires a single-column comparable PK. Composite-PK models fall
464+
// back to a single unordered bulk read - safer than a wrong-order
465+
// keyset walk. No V1 models currently use composite PKs, and the
466+
// single-PK invariant is asserted at load time by
467+
// mirror-model-init.spec.js.
459468
const pkAttrs = mirror.primaryKeyAttributes || [];
460-
const order =
461-
pkAttrs.length === 1 ? [[pkAttrs[0], 'ASC']] : undefined;
462469

463470
let synced = 0;
464-
for (let offset = 0; offset < count; offset += BACKFILL_BATCH_SIZE) {
465-
const rows = await source.findAll({
466-
raw: true,
467-
offset,
468-
limit: BACKFILL_BATCH_SIZE,
469-
order,
470-
});
471-
472-
await mirror.bulkCreate(rows, {
473-
updateOnDuplicate: updateFields,
474-
});
475-
476-
synced += rows.length;
471+
if (pkAttrs.length === 1) {
472+
const pk = pkAttrs[0];
473+
let lastPk = null;
474+
// eslint-disable-next-line no-constant-condition
475+
while (true) {
476+
const where =
477+
lastPk === null ? undefined : { [pk]: { [Sequelize.Op.gt]: lastPk } };
478+
const rows = await source.findAll({
479+
raw: true,
480+
where,
481+
limit: BACKFILL_BATCH_SIZE,
482+
order: [[pk, 'ASC']],
483+
});
484+
if (rows.length === 0) break;
485+
await mirror.bulkCreate(rows, { updateOnDuplicate: updateFields });
486+
synced += rows.length;
487+
lastPk = rows[rows.length - 1][pk];
488+
// Defensive: a null/undefined terminal PK means we can't
489+
// continue the keyset walk safely (Op.gt: undefined is
490+
// ill-defined and would loop on the same page). Stop the
491+
// walk; subsequent reconnects will retry from scratch.
492+
if (lastPk == null) {
493+
logger.warn(
494+
`Mirror backfill: ${name} - terminal row had null PK; ` +
495+
`stopping keyset walk early (${synced} rows synced).`,
496+
);
497+
break;
498+
}
499+
if (rows.length < BACKFILL_BATCH_SIZE) break;
500+
}
501+
} else {
502+
// Composite-PK mirrors are not supported by keyset-paginated
503+
// backfill. mirror-model-init.spec.js asserts every current
504+
// mirror has a single-column PK; if someone introduces a
505+
// composite-PK mirror without extending this function, fail
506+
// loudly (caught by the per-table try/catch) rather than
507+
// silently loading the whole table into memory.
508+
throw new Error(
509+
`Mirror backfill: ${name} has a composite primary key ` +
510+
`(${pkAttrs.join(', ')}). Keyset pagination needs a ` +
511+
`single-column comparable PK. Either reduce to a single-column ` +
512+
`PK, or extend backfillMirror to handle composite keys.`,
513+
);
477514
}
478515

479-
logger.info(`Mirror backfill: ${name} - synced ${synced} records`);
516+
if (synced === 0) {
517+
logger.debug(`Mirror backfill: ${name} - no records to sync`);
518+
} else {
519+
logger.info(`Mirror backfill: ${name} - synced ${synced} records`);
520+
}
480521
totalSynced += synced;
481522
} catch (error) {
482523
logger.error(`Mirror backfill error for ${name}: ${error.message}`);

‎src/database/v2/index.js‎

Lines changed: 71 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -460,47 +460,85 @@ export const backfillMirrorV2 = async () => {
460460
const orphansRemoved = await sweepMirrorOrphansV2(source, mirror, name);
461461
totalOrphansRemoved += orphansRemoved;
462462

463-
const count = await source.count();
464-
if (count === 0) {
465-
loggerV2.debug(`[v2]: Mirror backfill: ${name} - no records to sync`);
466-
continue;
467-
}
468-
469463
// Determine which fields to update on duplicate key conflict.
470464
// Include all non-primary-key attributes so the mirror stays in sync
471465
// with any changes that occurred in the source.
472466
const updateFields = Object.keys(mirror.rawAttributes).filter(
473467
(attr) => !mirror.primaryKeyAttributes.includes(attr),
474468
);
475469

476-
// SQLite (and MySQL) do not guarantee consistent row order across
477-
// paginated queries without an explicit ORDER BY. Without it,
478-
// concurrent writes during backfill can shift rows between pages
479-
// and cause some to be skipped entirely - defeating the purpose
480-
// of the recovery backfill. Order by the primary key when it's a
481-
// single column (all V2 mirrors today); fall back to unordered
482-
// for the composite-PK case (none exist in V2 today).
470+
// Keyset pagination (WHERE pk > :lastSeenPk ORDER BY pk ASC LIMIT N)
471+
// instead of offset-based. Offset pagination under concurrent
472+
// writes can silently skip rows: a delete at position N shifts
473+
// later rows down, so the next page's OFFSET lands one row later
474+
// than intended. Keyset pagination avoids that page-shift-on-delete
475+
// hazard by using a lower-bound predicate instead of a position
476+
// count.
477+
//
478+
// Note: keyset pagination is NOT immune to concurrent INSERTs with
479+
// a PK less than `lastPk` (plausible with UUID PKs, which most V2
480+
// mirrors use). Such rows will be missed in the current pass but
481+
// picked up by the next reconnect backfill. A subsequent
482+
// steady-state safeMirrorDbHandler write also directly upserts
483+
// them, so the miss window is bounded.
484+
//
485+
// Requires a single-column comparable PK. For the (currently none)
486+
// composite-PK case, fall back to a single unordered query -
487+
// safer than a wrong-order keyset walk. The single-PK invariant
488+
// is asserted at load time by mirror-model-init.spec.js.
483489
const pkAttrs = mirror.primaryKeyAttributes || [];
484-
const order =
485-
pkAttrs.length === 1 ? [[pkAttrs[0], 'ASC']] : undefined;
486490

487491
let synced = 0;
488-
for (let offset = 0; offset < count; offset += BACKFILL_BATCH_SIZE) {
489-
const rows = await source.findAll({
490-
raw: true,
491-
offset,
492-
limit: BACKFILL_BATCH_SIZE,
493-
order,
494-
});
495-
496-
await mirror.bulkCreate(rows, {
497-
updateOnDuplicate: updateFields,
498-
});
499-
500-
synced += rows.length;
492+
if (pkAttrs.length === 1) {
493+
const pk = pkAttrs[0];
494+
let lastPk = null;
495+
// eslint-disable-next-line no-constant-condition
496+
while (true) {
497+
const where =
498+
lastPk === null ? undefined : { [pk]: { [Sequelize.Op.gt]: lastPk } };
499+
const rows = await source.findAll({
500+
raw: true,
501+
where,
502+
limit: BACKFILL_BATCH_SIZE,
503+
order: [[pk, 'ASC']],
504+
});
505+
if (rows.length === 0) break;
506+
await mirror.bulkCreate(rows, { updateOnDuplicate: updateFields });
507+
synced += rows.length;
508+
lastPk = rows[rows.length - 1][pk];
509+
// Defensive: a null/undefined terminal PK means we can't
510+
// continue the keyset walk safely (Op.gt: undefined is
511+
// ill-defined and would loop on the same page). Stop the
512+
// walk; subsequent reconnects will retry from scratch.
513+
if (lastPk == null) {
514+
loggerV2.warn(
515+
`[v2]: Mirror backfill: ${name} - terminal row had null PK; ` +
516+
`stopping keyset walk early (${synced} rows synced).`,
517+
);
518+
break;
519+
}
520+
if (rows.length < BACKFILL_BATCH_SIZE) break;
521+
}
522+
} else {
523+
// Composite-PK mirrors are not supported by keyset-paginated
524+
// backfill. mirror-model-init.spec.js asserts every current
525+
// mirror has a single-column PK; if someone introduces a
526+
// composite-PK mirror without extending this function, fail
527+
// loudly (caught by the per-table try/catch) rather than
528+
// silently loading the whole table into memory.
529+
throw new Error(
530+
`[v2]: Mirror backfill: ${name} has a composite primary key ` +
531+
`(${pkAttrs.join(', ')}). Keyset pagination needs a ` +
532+
`single-column comparable PK. Either reduce to a single-column ` +
533+
`PK, or extend backfillMirrorV2 to handle composite keys.`,
534+
);
501535
}
502536

503-
loggerV2.info(`[v2]: Mirror backfill: ${name} - synced ${synced} records`);
537+
if (synced === 0) {
538+
loggerV2.debug(`[v2]: Mirror backfill: ${name} - no records to sync`);
539+
} else {
540+
loggerV2.info(`[v2]: Mirror backfill: ${name} - synced ${synced} records`);
541+
}
504542
totalSynced += synced;
505543
} catch (error) {
506544
loggerV2.error(`[v2]: Mirror backfill error for ${name}: ${error.message}`);
@@ -512,7 +550,10 @@ export const backfillMirrorV2 = async () => {
512550
`[v2]: MySQL mirror backfill completed - ${totalSynced} records upserted, ${totalOrphansRemoved} orphan rows removed`,
513551
);
514552
} catch (error) {
515-
loggerV2.error('[v2]: MySQL mirror backfill failed:', error.message);
553+
loggerV2.error(
554+
`[v2]: MySQL mirror backfill failed: ${error?.message || error?.name || 'unknown error'}`,
555+
);
556+
loggerV2.debug(error?.stack || error);
516557
// Don't throw - allow main database to continue operating
517558
}
518559
};

‎tests/v2/integration/mirror-datetime-format.spec.js‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { expect } from 'chai';
2+
import Sequelize from 'sequelize';
3+
4+
// Import config.js (via the barrel) to install the DATE._stringify patch.
5+
// The patch is applied once at module load; this file only exercises it.
6+
// eslint-disable-next-line no-unused-vars
7+
import _config from '../../../src/config/config.js';
8+
9+
/**
10+
* Regression test for the Sequelize mirror datetime format.
11+
*
12+
* Recent MariaDB strict mode rejects Sequelize's default DATE serialization
13+
* of "YYYY-MM-DD HH:mm:ss.SSS Z" (e.g. "2026-04-17 17:07:13.399 +00:00")
14+
* with "Incorrect datetime value". `src/config/config.js` patches the base
15+
* DATE._stringify to emit "YYYY-MM-DD HH:mm:ss" for MySQL/MariaDB while
16+
* retaining the original format for SQLite (whose parse() depends on the
17+
* trailing offset for round-trip).
18+
*
19+
* If this test fails, CADT's MySQL mirror writes will silently drop every
20+
* insert that touches a DATE/DATETIME column in strict-mode MariaDB.
21+
*/
22+
describe('Mirror datetime serialization', function () {
23+
const sampleDate = new Date('2026-04-17T17:07:13.399Z');
24+
25+
it('BASE DATE._stringify must emit the MariaDB-safe format (no offset, no fractional seconds)', function () {
26+
const out = Sequelize.DataTypes.DATE.prototype.stringify(sampleDate, {
27+
timezone: '+00:00',
28+
});
29+
expect(out).to.equal('2026-04-17 17:07:13');
30+
expect(out).to.not.include('+');
31+
expect(out).to.not.include('Z');
32+
expect(out).to.not.include('.');
33+
});
34+
35+
it('SQLite DATE._stringify must retain the original format (with trailing offset) for round-trip', function () {
36+
const out = Sequelize.DataTypes.sqlite.DATE.prototype.stringify(
37+
sampleDate,
38+
{ timezone: '+00:00' },
39+
);
40+
// SQLite's own parse() needs this offset suffix to reconstruct a Date
41+
expect(out).to.include('+00:00');
42+
const parsed = Sequelize.DataTypes.sqlite.DATE.parse(out, {
43+
timezone: '+00:00',
44+
});
45+
expect(parsed.getTime()).to.equal(sampleDate.getTime());
46+
});
47+
48+
it('mysql DATE._stringify must continue to emit the MariaDB-safe format', function () {
49+
const out = Sequelize.DataTypes.mysql.DATE.prototype.stringify(sampleDate, {
50+
timezone: '+00:00',
51+
});
52+
expect(out).to.equal('2026-04-17 17:07:13');
53+
});
54+
});

0 commit comments

Comments
 (0)