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.

Sign up for GitHub

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

Jump to bottom

refactor: metrics to fix post visits cannot be migrated #2870

Merged
merged 8 commits into from
Dec 13, 2022
Merged

refactor: metrics to fix post visits cannot be migrated #2870

merged 8 commits into from
Dec 13, 2022

Conversation

guqing
Copy link
Member

@guqing guqing commented Dec 7, 2022

What type of PR is this?

/kind improvement
/area core

What this PR does / why we need it:

重构访问量统计逻辑

  1. 去掉了访问量总数存储在 Meter Counter 中的逻辑，因为迁移时是直接像 Counter 自定义模型创建数据，而文章被访问时是存储在 Meter Counter 后定时同步到数据库，这就存在双向同步问题且都有新数据无法知道该如何合并数据。
  2. 目前访问时会发送一个事件，当得到事件后会缓存在队列中，每隔一分钟将增量更新到数据库中
  3. 评论统计也去掉了 Meter Counter 改为事件队列处理
  4. 如果后续要暴露 Metrics 应该使用 Gauge 监控 Counter 自定义模型
  5. Counter 自定义模型的查询优化后续可以使用 Indexer 或者加缓存来实现而非将 Meter Counter 当作缓存

Which issue(s) this PR fixes:

Fixes #2820

Special notes for your reviewer:

  1. 测试迁移导入看文章访问量是否正确
  2. 创建评论及回复观察未读回复数量、评论回复数、最新回复时间是否正确
  3. 多创建一些回复然后删除评论，看是否正确删除

/cc @halo-dev/sig-halo

Does this PR introduce a user-facing change?

重构访问量统计逻辑，修复文章visits无法迁移的问题

@f2c-ci-robot f2c-ci-robot bot added kind/improvement Categorizes issue or PR as related to a improvement. release-note Denotes a PR that will be considered when it comes time to generate release notes. area/core Issues or PRs related to the Halo Core labels Dec 7, 2022
@guqing
Copy link
Member Author

guqing commented Dec 7, 2022

/milestone 2.0.1

@f2c-ci-robot f2c-ci-robot bot added this to the 2.0.1 milestone Dec 7, 2022
@guqing guqing changed the title Refactor/2820 refactor: metrics to fix post visits cannot be migrated Dec 7, 2022
JohnNiang
JohnNiang reviewed Dec 7, 2022
View reviewed changes
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.

温馨提示：尽可能不要在一个 PR 内完成多个任务。修复问题就应是单纯修复问题，不应引入其他不相关的代码修改。这样不仅造成 Review 和测试难度飙升，还无法作为 patch 合并到 2.0.1 中。

@JohnNiang JohnNiang removed this from the 2.0.1 milestone Dec 7, 2022
@ruibaby
Copy link
Member

ruibaby commented Dec 7, 2022

这个放到 2.1.0 吧。

/milestone 2.1.x

@f2c-ci-robot f2c-ci-robot bot added this to the 2.1.x milestone Dec 7, 2022
@guqing
Copy link
Member Author

guqing commented Dec 7, 2022

温馨提示：尽可能不要在一个 PR 内完成多个任务。修复问题就应是单纯修复问题，不应引入其他不相关的代码修改。这样不仅造成 Review 和测试难度飙升，还无法作为 patch 合并到 2.0.1 中。

ok，不过要修复导入问题就需要修改原来的实现方式，下次最小化修改

@ruibaby
Copy link
Member

ruibaby commented Dec 9, 2022

尝试导入了 halo.run 的数据，符合预期。

1.6:

image

2.0:

image

image

@guqing guqing requested a review from JohnNiang December 13, 2022 03:23
JohnNiang
JohnNiang approved these changes Dec 13, 2022
View reviewed changes
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.

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2022
@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Dec 13, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JohnNiang

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2022
@f2c-ci-robot f2c-ci-robot bot merged commit a9a65dd into halo-dev:main Dec 13, 2022
@guqing guqing deleted the refactor/2820 branch December 13, 2022 04:32
@JohnNiang JohnNiang removed this from the 2.1.x milestone Dec 14, 2022
@JohnNiang JohnNiang added this to the 2.1.0 milestone Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnNiang JohnNiang JohnNiang approved these changes

Assignees

@JohnNiang JohnNiang

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/improvement Categorizes issue or PR as related to a improvement. 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
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.

2.0.0 文章visits无法迁移
3 participants
@guqing @ruibaby @JohnNiang