Skip to content

fix: backfill schemaDefined on replicas missing the flag#503

Open
ldt1996 wants to merge 6 commits into
mainfrom
fix/schemadefined-on-replicated-tables
Open

fix: backfill schemaDefined on replicas missing the flag#503
ldt1996 wants to merge 6 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
@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

ldt1996 and others added 2 commits June 2, 2026 13:08
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 4 commits June 8, 2026 16:01
…replicated-tables-rebase

# Conflicts:
#	resources/databases.ts
#	unitTests/resources/databases.test.js
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>
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