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(slow_subs): Fix the unit error of the deliver_begin_at timestamp #8981

Merged
merged 2 commits into from
Sep 26, 2022

Conversation

lafirest
Copy link
Member

@lafirest lafirest commented Sep 16, 2022

The unit of deliver_begin_at is incorrectly used the seconds, the resulting is incorrect when stats_type is internal or response.
When stats_type is internal, the time span is always negative (seconds - milliseconds) and will not be counted
When stats_type is response, the time span is a large value (milliseconds - seconds)

Copy link
Member

@zmstone zmstone left a comment

Choose a reason for hiding this comment

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

  • Add description
  • Add appup
  • Update CHANGES
  • App version bump
  • Test case should verify interval as expected (at least around the expected interval).

@lafirest lafirest marked this pull request as ready for review September 20, 2022 04:28
@lafirest lafirest requested review from zmstone and a team September 20, 2022 04:28
HJianBo
HJianBo previously approved these changes Sep 23, 2022
Copy link
Member

@HJianBo HJianBo left a comment

Choose a reason for hiding this comment

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

LGTM; Could you plz check it again? @zmstone @xiangfangyang-tech

ieQu1
ieQu1 previously approved these changes Sep 23, 2022
The unit of `deliver_begin_at` is incorrectly used the seconds,
this will cause the `internal` stat type to never working and the `response` type time span error
@lafirest lafirest changed the base branch from main-v4.4 to release-v44 September 26, 2022 09:37
@lafirest lafirest merged commit d3f0209 into emqx:release-v44 Sep 26, 2022
@lafirest lafirest deleted the v4.4 branch September 26, 2022 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants