Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: unique index conflict issue after backup restoration preventing startup #6701

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

guqing
Copy link
Member

@guqing guqing commented Sep 25, 2024

What type of PR is this?

/kind bug
/area core
/milestone 2.20.x
/sig docs

What this PR does / why we need it:

修复恢复备份后可能会因为与之前的数据冲突导致无法启动的问题

如果恢复时发生不可预知的错误,需要重启之后重新初始化再进行恢复

Which issue(s) this PR fixes:

Fixes #6672

Does this PR introduce a user-facing change?

修复恢复备份后可能会因为与恢复之前存在的数据冲突导致无法启动的问题

@f2c-ci-robot f2c-ci-robot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. labels Sep 25, 2024
@f2c-ci-robot f2c-ci-robot bot added this to the 2.20.x milestone Sep 25, 2024
@f2c-ci-robot f2c-ci-robot bot added the area/core Issues or PRs related to the Halo Core label Sep 25, 2024
@f2c-ci-robot f2c-ci-robot bot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Sep 25, 2024
Copy link

f2c-ci-robot bot commented Sep 25, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from guqing. For more information see the Kubernetes Code Review Process.

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

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

Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.83%. Comparing base (982a45b) to head (99d4b73).
Report is 23 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6701      +/-   ##
============================================
- Coverage     58.29%   57.83%   -0.46%     
- Complexity     3963     3990      +27     
============================================
  Files           680      704      +24     
  Lines         23338    23739     +401     
  Branches       1584     1572      -12     
============================================
+ Hits          13604    13729     +125     
- Misses         9108     9389     +281     
+ Partials        626      621       -5     

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

@JohnNiang
Copy link
Member

Hi @guqing ,因为之前的默认行为是合并数据,当前 PR 将会修改默认行为。所以我有以下几点建议:

  1. 给恢复接口提供一个参数,例如 strategy=deleteAll | merge | skipDuplicates。
  2. 在 Console 端给出足够提示,让用户知晓当前恢复操作会删除旧数据

@guqing
Copy link
Member Author

guqing commented Sep 26, 2024

Hi @guqing ,因为之前的默认行为是合并数据,当前 PR 将会修改默认行为。所以我有以下几点建议:

  1. 给恢复接口提供一个参数,例如 strategy=deleteAll | merge | skipDuplicates。
  2. 在 Console 端给出足够提示,让用户知晓当前恢复操作会删除旧数据

要检查冲突时跳过那就不能使用 ExtensionStoreRepository 来 save 了,而且 merge 和 skipDuplicates 应该会有重叠

@JohnNiang
Copy link
Member

我的建议:提供一个参数(例如 prune=true)用于表示是否需要在恢复前清除已经存在的所有数据(包括数据库中的数据和工作目录的文件)。不过默认情况下不清除。

另外,需要在 Console 侧给出足够的提示 @halo-dev/sig-halo-console 。

@guqing
Copy link
Member Author

guqing commented Sep 29, 2024

我的建议:提供一个参数(例如 prune=true)用于表示是否需要在恢复前清除已经存在的所有数据(包括数据库中的数据和工作目录的文件)。不过默认情况下不清除。

另外,需要在 Console 侧给出足够的提示 @halo-dev/sig-halo-console 。

即使添加了 prune 参数,也解决不了问题,因为不通过 prune=true 恢复后发生冲突也只能删除了重新初始化才行,而不能通过 prune 再次重试,可能得在恢复界面加一段说明来告知用户

Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2024
Copy link

sonarcloud bot commented Sep 30, 2024

Copy link

pkg-pr-new bot commented Sep 30, 2024

Open in Stackblitz

@halo-dev/api-client

pnpm add https://pkg.pr.new/@halo-dev/api-client@6701

@halo-dev/components

pnpm add https://pkg.pr.new/@halo-dev/components@6701

@halo-dev/ui-plugin-bundler-kit

pnpm add https://pkg.pr.new/@halo-dev/ui-plugin-bundler-kit@6701

@halo-dev/richtext-editor

pnpm add https://pkg.pr.new/@halo-dev/richtext-editor@6701

@halo-dev/console-shared

pnpm add https://pkg.pr.new/@halo-dev/console-shared@6701

commit: 99d4b73

Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

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

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core Issues or PRs related to the Halo Core kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/docs Categorizes an issue or PR as relevant to SIG Docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

备份恢复后重启出现 DuplicateNameException 异常
3 participants