Repository navigation
cherry-pick: Preserve 5.2 rollback compatibility for first-time table creates (conflicts → v5.3) - #3124
Merged
Conversation
Keep legacy names until a table name has local history, journal unstamped creates independently, and preserve drop ownership during recovery. Co-Authored-By: OpenAI Codex GPT-6.1 <noreply@openai.com>
Retirement journals can outlive a rollback that reuses bare store names. Preserve stores owned by the live catalog and their blobs, and verify the 5.2 recreate/re-upgrade path. Co-Authored-By: OpenAI Codex GPT-6.1 <noreply@openai.com>
Guard retirement of the dropper’s own physical handles against live catalog ownership, then preserve write settlement and blob cleanup. Document untimed name history and older-reader crash recovery limits. Co-Authored-By: OpenAI Codex GPT-6.1 <noreply@openai.com>
Reopen the storage engine before the final rollback-recreate and index assertions, so retained handles cannot mask an erroneously reclaimed family. Co-Authored-By: OpenAI Codex GPT-6.1 <noreply@openai.com>
Snapshot journal phases under the catalog lock and defer creating-row primary retirement to any pending retired journal. Verify both journal orderings unlink blobs before retirement completes. Co-Authored-By: OpenAI Codex GPT-6.1 <noreply@openai.com>
The conflict hunks pulled in main's #2962 lifecycle machinery (timed drop markers, createdTime stamps, replication helpers), which is milestoned v5.4 and absent here; it is dropped. #3118 keys "this name has history" off the /dropped/<table> row that #2962 writes at every drop completion, so v5.3 gets only the untimed form #3118 itself introduced: both RocksDB completion paths (retireRocksStores, completeInterruptedDrop) record it before removing the tombstone, and the load parser skips /dropped/ rows. 5.4 reads the same row as its untimed marker. The design notes keep v5.3's section heading and drop the lifecycle-stamp paragraph. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QYAaU3uVxkbjHe19UNWRfK Dispatch-Task: cherry-resolve-kriszyp_harper_3118-6e6e74cf
…w contract v5.3's dropTable() takes no options, so the picked tests' localOnly argument only repeated the ordinary drop; the test now names what it checks. The recordTableNameHistory comment states the row-shape constraint instead of issue history. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QYAaU3uVxkbjHe19UNWRfK Dispatch-Task: cherry-resolve-kriszyp_harper_3118-6e6e74cf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of PR #3118 onto `v5.3` produced conflicts on commit(s): `4db030f2a0b8f4d064dfdf60446b4181ec597a40 b8fca4e 5f8f49e`.
Resolve the conflict markers on branch `cherry-pick/v5.3/pr-3118` and merge this PR.
@claude please review branch `cherry-pick/v5.3/pr-3118` and suggest a patch that resolves the conflict markers (
<<<<<<</=======/>>>>>>>) introduced by cherry-picking PR #3118 onto `v5.3`. Post the suggested patch as a comment here — do not push.Resolution on v5.3
The conflict markers are resolved in two new commits on top of the Action's commits. No history was rewritten. The resolution started from #3118's squashed diff on main (4dba9f5), not from the nested markers.
databases.tsconflict block was the drop-marker andcreatedTimecode from Stamp table generations and keep a durable drop marker… (#2962). That PR is milestoned v5.4 and is not on v5.3, so the block is gone. This removeswriteTableDropMarker,getTableDrops,recordTableDrop,tableLifecycleTime, thecreatedTimestamps and the related helpers./dropped/<table>row, a journal row, or aT/column family (check indeclareTable). On main, only Stamp table generations and keep a durable drop marker so a peer that missed a replicated drop_table cannot bring the table back #2962's drop-completion code writes that row. On v5.3,recordTableNameHistorywrites the untimed{ table, tableId }row Preserve 5.2 rollback compatibility for first-time table creates #3118 added on main. It runs at both RocksDB drop-completion points, under the catalog lock and before the tombstone is removed:retireRocksStoresin Table.ts andcompleteInterruptedDrop. Without it, a recreate after the journal is cleaned up (about 2 s) falls back to bare names, and naming depends on timing. Look hardest here./dropped/rows. Downgrading to 5.2 or 5.3.0/5.3.1: the old loader sees a table''with no primary key and skips it with one warning. 5.2 already does this for/generation/rows. Upgrading to 5.4: the row is read as its untimed marker, and a timed drop overwrites it.dropTable()takes no options, so thegetTableDropsassertion and the ignoredlocalOnlyarguments were removed.Every other production hunk matches #3118's squash on main.
Verification
dropTableGeneration24/24, and the ghost-table and cross-worker drop files pass. LMDB passes on the same files. Fulltest:unit:resourcespassed on c6ff208: 4024 passing, 0 failing.completeInterruptedDropwrite, or the loader skip, fails the new test.first-create-downgraderan the real 5.2.15 round trip and passed. The tables it drops leave/dropped/rows, so 5.2 booted with them present.Unit Test (Node.js v26)fails 7 tests on Node 26.11.1 (withNodeAdapter×4,install_node_modulesdry-run ×3). These are v5.3's own baseline failures, and v5.3's push runs ofunit-test.ymlare red too. The Node 22 and 24 legs pass.— Claude Opus 5.5
🤖 Generated with Claude Code
https://claude.ai/code/session_01QYAaU3uVxkbjHe19UNWRfK
Related PRs: #3118 overlaps, #2962 overlaps, #3119 overlaps, 17 others independent
Review-Coverage: authored=claude; ran=gemini,codex,cursor-composer; adjudicated=domain; blocked=cursor-grok(failed); declined=cursor-kimi,cursor-muse; rounds=2; full=1 @ 8e83d2d
Review-Attention: deep ~30m (critical: Table.ts, databases.ts; decisions: main-first-follow-ups, keep-legacy-writer-comment) @ 8e83d2d