Skip to content

fix: backfill schemaDefined on replicas missing the flag - #503

Merged
kriszyp merged 2 commits into
mainfrom
fix/schemadefined-on-replicated-tables
Jun 16, 2026
Merged

fix: backfill schemaDefined on replicas missing the flag#503
kriszyp merged 2 commits into
mainfrom
fix/schemadefined-on-replicated-tables

Conversation

@ldt1996

@ldt1996 ldt1996 commented May 11, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@ldt1996
ldt1996 requested a review from a team as a code owner May 11, 2026 20:19
Comment thread resources/databases.ts Outdated
@claude

claude Bot commented May 11, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know much about this, but makes sense. It would be nice if it was easy to add a test that demonstrates this fixes things.

@kriszyp

kriszyp commented May 12, 2026

Copy link
Copy Markdown
Member

What is the situation where this can occur? I thought line 907 in databases.ts consistently set this flag.
And reminder: please try to use /harper-engineering-guidelines skill to file PRs, this should automatically ensure there is an AI review prior to the PR being filed.

ldt1996 added a commit that referenced this pull request May 26, 2026
When the on-disk primary-attribute descriptor in attributesDbi lacks
schemaDefined (the state replica nodes were left in by 4.7.x deploys with
replicated=true), re-entering table() must:
- heal Table.schemaDefined in memory, and
- rewrite the descriptor with schemaDefined: true on disk.

The test strips the flag from a freshly created descriptor, drops the
in-memory state via resetDatabases (mimicking a process restart on a
replica), re-calls table(), and asserts both the in-memory class and the
persisted descriptor have schemaDefined: true. Addresses the PR #503
review request to exercise the needsSchemaDefinedBackfill branch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@ldt1996

ldt1996 commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

@kriszyp I think that line just sets the local variable... from what I can tell it only gets persisted via the new table branch, and the existing table branch doesn't seem to rewrite the descriptor, so once a replica ends up on disk without the flag the defaulting probably can't reach back. My guess is that's what 4.7.x replicated=true deploys were producing

kriszyp pushed a commit that referenced this pull request Jun 2, 2026
When the on-disk primary-attribute descriptor in attributesDbi lacks
schemaDefined (the state replica nodes were left in by 4.7.x deploys with
replicated=true), re-entering table() must:
- heal Table.schemaDefined in memory, and
- rewrite the descriptor with schemaDefined: true on disk.

The test strips the flag from a freshly created descriptor, drops the
in-memory state via resetDatabases (mimicking a process restart on a
replica), re-calls table(), and asserts both the in-memory class and the
persisted descriptor have schemaDefined: true. Addresses the PR #503
review request to exercise the needsSchemaDefinedBackfill branch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@kriszyp
kriszyp force-pushed the fix/schemadefined-on-replicated-tables branch from 1f2f850 to 196b585 Compare June 2, 2026 19:20
ldt1996 and others added 2 commits June 16, 2026 07:14
When the on-disk primary-attribute descriptor in attributesDbi lacks
schemaDefined (the state replica nodes were left in by 4.7.x deploys with
replicated=true), re-entering table() must:
- heal Table.schemaDefined in memory, and
- rewrite the descriptor with schemaDefined: true on disk.

The test strips the flag from a freshly created descriptor, drops the
in-memory state via resetDatabases (mimicking a process restart on a
replica), re-calls table(), and asserts both the in-memory class and the
persisted descriptor have schemaDefined: true. Addresses the PR #503
review request to exercise the needsSchemaDefinedBackfill branch.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirrors the fix Kris applied to northwind.test.mjs (48179a1). On Bun shard 2,
embed-directive tear-down (HNSW flush) starves the job processor, so csv_data_load
sits IN_PROGRESS past the default 30s. Same suite, same shard, same fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kriszyp
kriszyp force-pushed the fix/schemadefined-on-replicated-tables branch from 7e8cf9c to 1393cb9 Compare June 16, 2026 13:15
@kriszyp
kriszyp merged commit a43f6a6 into main Jun 16, 2026
79 of 82 checks passed
@kriszyp
kriszyp deleted the fix/schemadefined-on-replicated-tables branch June 16, 2026 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants