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

feat: optimized post reconciliation process for enhanced performance and resource utilization #5250

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

guqing
Copy link
Member

@guqing guqing commented Jan 25, 2024

What type of PR is this?

/kind feature
/milestone 2.12.x
/area core

What this PR does / why we need it:

我们为文章自定义模型的数据调协过程引入了重要的优化。
在以前,当数据量大(例如,50,000篇文章)的情况下,每次系统重启都会触发耗时且资源密集的所有数据的协调过程,即使大部分数据并不需要调协。这导致了不必要的数据库查询和高资源消耗。

为了解决这个问题,我们在文章自定义模型的 status 中添加了一个新的 Long observedVersion 属性。
每次协调后,此属性将更新为 metadata.version,还调整了 syncAllOnStart 条件,只有当 status.observedVersion < metadata.version 时才会调协数据。

这个改变确保了只有在启动时需要的数据会被协调,从而减少了资源使用和不必要的协调过程。
因此,Halo 的数据承载能力得到了显著提高。

how to test it?
使用此 PR 测试:启动时文章只有首次会执行 reconcile,再次重启时则不会再执行,如果直接修改数据去除掉 status.observedVersion 来模拟迁移或漏 reconcile 的过程则启动时该数据会被再次执行 reconcile

Which issue(s) this PR fixes:

Fixes #5147

Does this PR introduce a user-facing change?

优化文章数据的调协过程以降低 Halo 启动时文章的调协耗时同时提高性能和资源利用率

@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/feature Categorizes issue or PR as related to a new feature. labels Jan 25, 2024
@f2c-ci-robot f2c-ci-robot bot added this to the 2.12.x milestone Jan 25, 2024
@f2c-ci-robot f2c-ci-robot bot added the area/core Issues or PRs related to the Halo Core label Jan 25, 2024
@ruibaby
Copy link
Member

ruibaby commented Jan 25, 2024

Genius

Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (3f27f6f) 57.06% compared to head (aec8dea) 57.06%.
Report is 1 commits behind head on main.

Files Patch % Lines
.../app/core/extension/reconciler/PostReconciler.java 20.00% 4 Missing ⚠️
...ain/java/run/halo/app/infra/SchemeInitializer.java 75.00% 1 Missing and 1 partial ⚠️
...java/run/halo/app/core/extension/content/Post.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main    #5250   +/-   ##
=========================================
  Coverage     57.06%   57.06%           
- Complexity     3364     3365    +1     
=========================================
  Files           585      585           
  Lines         19185    19195   +10     
  Branches       1428     1429    +1     
=========================================
+ Hits          10947    10954    +7     
- Misses         7652     7654    +2     
- Partials        586      587    +1     

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

@JohnNiang
Copy link
Member

文章数据(包括量非常大其他数据)倾向启动时不同步数据。

@guqing
Copy link
Member Author

guqing commented Jan 26, 2024

文章数据(包括量非常大其他数据)倾向启动时不同步数据。

建议可以测试一下,只有漏跑的才会在启动时被执行,否则等于启动时没有执行同步,除此之外没有任何副作用

如果启动时不同步那么当创建了文章堆积在 queue 里没有执行 halo 被停止了怎么办呢,文章就只能手动更新才可以再次执行

@JohnNiang
Copy link
Member

文章数据(包括量非常大其他数据)倾向启动时不同步数据。

建议可以测试一下,只有漏跑的才会在启动时被执行,否则等于启动时没有执行同步,除此之外没有任何副作用

如果启动时不同步那么当创建了文章堆积在 queue 里没有执行 halo 被停止了怎么办呢,文章就只能手动更新才可以再次执行

不同步数据的副作用肯定是有的,就如你上面所提到的。

我的关注点在数据量上,如果数据量本身可能会无限增长,此时就不再适合用 Reconciler 来实现相关逻辑,例如文章,评论等数据。尝试用另外的方式实现文章的发布,更新及删除等可能会更好。

@guqing
Copy link
Member Author

guqing commented Jan 26, 2024

我的关注点在数据量上,如果数据量本身可能会无限增长,此时就不再适合用 Reconciler 来实现相关逻辑,例如文章,评论等数据。尝试用另外的方式实现文章的发布,更新及删除等可能会更好。

但是通过此 PR 解决了启动时不同步又没有副作用的情况下,无论数据怎么增长都不会影响到启动吧,添加一篇文章 Reconciler 就处理一篇是没有问题的处理完之后启动时就不会在无效执行,没理解你考虑的点在哪里

Note 当用户升级到 2.12 之后对于以前的文章数据启动时还是会同步一次,但是之后在重启或升级就不会在启动时被执行了

Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

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

/approve

@f2c-ci-robot f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 2024
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 Jan 26, 2024
Copy link

f2c-ci-robot bot commented Jan 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JohnNiang, ruibaby

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

The pull request process is described 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

@f2c-ci-robot f2c-ci-robot bot merged commit 17a0fb9 into halo-dev:main Jan 26, 2024
7 checks passed
@ruibaby ruibaby modified the milestones: 2.12.x, 2.12.0 Jan 26, 2024
@guqing guqing deleted the feature/5147 branch January 26, 2024 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/core Issues or PRs related to the Halo Core kind/feature Categorizes issue or PR as related to a new feature. 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.
Projects
None yet
3 participants