Skip to content

Support offline checkpoint catalog inspection, CSV dump, and restore script generation#25324

Open
jiangxinmeng1 wants to merge 79 commits into
matrixorigin:mainfrom
jiangxinmeng1:view_ckp
Open

Support offline checkpoint catalog inspection, CSV dump, and restore script generation#25324
jiangxinmeng1 wants to merge 79 commits into
matrixorigin:mainfrom
jiangxinmeng1:view_ckp

Conversation

@jiangxinmeng1

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #25323

What this PR does / why we need it:

Support offline checkpoint catalog inspection, CSV dump, and restore script generation

XuPeng-SH and others added 30 commits June 7, 2026 01:30
- EarliestTS/LatestTS: skip entries with empty start/end timestamps
- EarliestTS also considers end timestamps, not just start
- LatestTS: skip empty end values
- Add parallel CSV pipeline with memory backpressure for storage mode
- Fix block read memory leak: release now cleans batch vectors

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… DDL names

- ListCatalogTables: merge catalog entries from raw and schema-projected mo_tables views
- add logicalViewDataOffset for header detection without hardcoding meta width
- normalizeCreateTableDDLName: replace catalog DDL table name with real name
- add SQL token parser for CREATE TABLE name normalization (CREATE TABLE IF NOT EXISTS)
- add checkpoint_test.go

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove the single-table dump by name (--table flag) functionality from ckp dump.
The resolveTableByName function and all related validation logic are removed.
Also change the default --jobs value from 1 to 5 for batch dump concurrency.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes. The feature direction is useful, but I found several correctness gaps in the new dump/restore and multipart paths that should be closed before merge.

  1. cmd/mo-object-tool/ckp/checkpoint.go:553 drops the output Close() error for single-table dumps. For local files this can lose a flush error; for --out-s3/--out-backend it is much more serious because fileServiceWriteCloser.Close() is where the background fs.Write / multipart completion result is returned. Today DumpTableCSVComposed can succeed, CompleteMultipartUpload can fail, and the command still prints Table ... dumped. Please close explicitly after the dump and combine dump/close errors the same way dumpOneTable already does.

  2. pkg/tools/checkpointtool/table_dump.go:1185 silently ignores every GetObjectEntries error. That turns checkpoint corruption, permission errors, remote read failures, or decode errors into either a partial export or a misleading no data entries for table error. This should only skip the known GC/missing-file case (isDataFileNotFound); all other errors need to be returned/aggregated. Please add an unhappy-path regression test for a non-missing checkpoint entry error.

  3. pkg/tools/checkpointtool/checkpoint_reader.go:424 does not actually pick the latest usable GCKP. loadEntries sorts entries newest-first, but ComposeAt iterates from len(entries)-1 down and breaks on the first end <= ts, which is the oldest qualifying GCKP. That can compose a snapshot from an unnecessarily old base plus too many incrementals, and can produce wrong catalog/table state when multiple global checkpoints exist. Iterate newest-to-oldest, skipping only GC'd bases, and cover this with a two-GCKP test.

  4. cmd/mo-object-tool/README.md:100 documents --row-order=storage as object/block/row scan order, but pkg/tools/checkpointtool/table_dump.go:2065 processes objects concurrently and writeCSVChunks writes chunks in completion order. The default output is therefore nondeterministic and no longer matches the documented storage-order contract. Either preserve order with a small reorder buffer keyed by (objectIdx, blockIdx) or change the contract/flag name and tests to make unordered streaming explicit.

  5. pkg/fileservice/aws_sdk_v2.go:713 and pkg/fileservice/qcloud_sdk.go:566 can resend the same pending buffer after sendJob fails. On context cancellation, max-part overflow, or worker-triggered cancellation, sendJob already records the error and may return the buffer to the pool, but the loop breaks with pendingN > 0, so lines 724/577 call sendJob again for the same buffer. This can double-return buffers, advance partNum incorrectly, or continue upload state after cancellation. Please clear pendingN/return immediately on sendJob failure and add tests that cancel/fail after at least one full pending chunk, not only before upload starts.

Local checks run:

  • make err-check
  • go test -count=1 ./pkg/tools/checkpointtool ./cmd/mo-object-tool/ckp ./pkg/sql/colexec/external
  • go test -count=1 ./pkg/tools/toolfs ./pkg/fileservice with macOS CGo rpath adjusted for local dylibs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature size/XXL Denotes a PR that changes 2000+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants