Skip to content

restore secondary: refuse early when the backup lacks its full meta - #1185

Open
czs007 wants to merge 1 commit into
zilliztech:mainfrom
czs007:secondary-require-full-meta
Open

restore secondary: refuse early when the backup lacks its full meta#1185
czs007 wants to merge 1 commit into
zilliztech:mainfrom
czs007:secondary-require-full-meta

Conversation

@czs007

@czs007 czs007 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

issue: #1186

A secondary restore broadcasts DDL on the source's control channel and replays
the source's flush-all messages per pchannel. Those fields live only in
meta/full_meta.json. When that file is absent, meta.Read silently falls back
to the per-level meta files: the collection list still loads, the preflight
checks still run, and the restore fails at its first broadcast with
stream: no pch in message, which points nowhere near the cause.

The same shape occurs when the backup itself never recorded them -- a backup
taken with --strategy=skip_flush or meta_only (or the deprecated --force)
records no flush point by design. A plain restore accepts such a backup; a
secondary restore cannot, because it has no position in the source's stream to
start replication from.

Reported on a 2.6 deployment: an 88-line restore log where the only error was
the final no pch in message, preceded by twenty-odd INFO lines that read like
failures. The backup's full_meta.json was present and held all 66 collections
and 3,435 segments, but none of the three cluster-level fields.

Changes

  • meta.Read logs a warning when it falls back to the per-level files, naming
    the file it expected.
  • restore secondary checks up front that the backup carries a control channel,
    a pchannel list and flush-all messages, and refuses with a message that names
    meta/full_meta.json and both ways it ends up missing. This runs before any
    RPC, so it costs nothing and fires before the target is touched.
  • The database-creation error said broadcast create collection; it now says
    broadcast create database <name>.
  • checkTargetIsUnused reported every database-not-found as an INFO line with
    the full error object attached, once per collection. On the reported restore
    that was twenty-odd lines that looked like failures while being the expected
    state. It now reports once per database without the error object, and reserves
    a warning for errors that are not the expected not-found.

Verification

Unit tests cover the new check for both the API and metastore spellings plus the
negative cases, and the not-found classification.

End to end on a 2.6 cluster, with the released binary as the control:

backup taken normally     create logs "use bulk flush strategy", full_meta.json
                          carries all three fields, secondary restore completes
backup without the flush  full_meta.json key set matches the reported one;
  step                    before: fails at the first broadcast with
                          "no pch in message" after 88 lines
                          after:  refused by the new check before any RPC,
                          naming full_meta.json and the strategies that skip it
backup taken normally,    still reaches the existing checkpoint check and is
  target already used     refused there, unchanged

gofmt, golangci-lint v2.12.2 (the CI version) over the whole tree, go vet,
go build ./... and go test are clean.

A secondary restore broadcasts DDL on the source's control channel and replays
the source's flush-all messages per pchannel. Those fields live only in
meta/full_meta.json. When that file is absent -- a backup copied between
buckets with the file left behind is the case that surfaced this -- meta.Read
silently falls back to the per-level files, the collection list loads fine,
and the restore fails at its first broadcast with "stream: no pch in message",
which points nowhere near the cause.

- meta.Read now logs a warning when it falls back to the per-level files,
  naming the file it expected.
- restore secondary checks up front that the backup carries a control channel,
  a pchannel list and flush-all messages, and refuses with a message that names
  meta/full_meta.json and the two ways it ends up missing.
- The database-creation error said "broadcast create collection"; it now says
  which database failed.
- checkTargetIsUnused reported every database-not-found as an INFO line with
  the full error object attached, once per collection, which reads as a
  failure. It now reports once per database, without the error object, and
  reserves a warning for errors that are not the expected not-found.

Signed-off-by: zhenshan.cao <zhenshan.cao@zilliz.com>
@sre-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: czs007
To complete the pull request process, please assign alohaha22 after the PR has been reviewed.
You can assign the PR to them by writing /assign @alohaha22 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mergify

mergify Bot commented Aug 26, 2026

Copy link
Copy Markdown

@czs007 Please associate the related issue to the body of your Pull Request. (eg. “issue: #”)

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.24390% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.51%. Comparing base (79c7582) to head (5a9b532).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
core/restore/secondary/task.go 91.89% 3 Missing ⚠️
core/restore/secondary/database_task.go 0.00% 1 Missing ⚠️

❌ Your project status has failed because the head coverage (43.51%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1185      +/-   ##
==========================================
+ Coverage   43.36%   43.51%   +0.15%     
==========================================
  Files         135      135              
  Lines       12631    12670      +39     
==========================================
+ Hits         5477     5513      +36     
- Misses       6743     6746       +3     
  Partials      411      411              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants