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

Use miliseconds for statistics #43

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bendavies
Copy link
Contributor

@bendavies bendavies commented Sep 29, 2022

This is a rebased submission of #27

fixes #20
fixes #17
fixes #25

TODO:

  • document data migration

weaverryan
weaverryan previously approved these changes Sep 29, 2022
Copy link
Contributor

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

There's quite a bit going on here, but i'm 👍 and the tests look solid.

$table->addColumn('dispatched_at', Types::FLOAT)->setNotnull(true);
$table->addColumn('waiting_time', Types::FLOAT)->setNotnull(false);
$table->addColumn('handling_time', Types::FLOAT)->setNotnull(false);
$table->addColumn('failing_time', Types::FLOAT)->setNotnull(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just an FYI for us, this will require a migration - iirc, make:migration will see this changes, but it's been awhile so I'm not 100% sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like you can backfill these new columns, as they are calculated from the existing ones?
the existing ones can then be dropped

@weaverryan
Copy link
Contributor

@bendavies now that CI is running, there's one tiny failure related to precision. Can you have a look?

@bendavies
Copy link
Contributor Author

Interesting. That didn't happen on the original pr

@bendavies
Copy link
Contributor Author

@bendavies now that CI is running, there's one tiny failure related to precision. Can you have a look?

looks like this is due to floating point calculations in mariadb
image

i'll see if switching the column types from float to decimal fixes this.

weaverryan added a commit that referenced this pull request Oct 4, 2022
This PR was merged into the master branch.

Discussion
----------

add support for PostgreSQL

Adds support and fixes issues for PostgresSQL.

Should be merged before #43

Commits
-------

e0168b6 add support for PostgreSQL
@bendavies bendavies force-pushed the miliseconds-for-stats-rebased branch 2 times, most recently from 5648987 to ae900bf Compare October 4, 2022 19:53
@bendavies bendavies marked this pull request as draft October 6, 2022 12:43
@bendavies
Copy link
Contributor Author

using decimals has fixed the tests

nikophil and others added 2 commits May 25, 2023 09:46
store waiting_time in seconds+microseconds

store handling_time in seconds+microseconds

store failing_time in seconds+microseconds

Fixed statistics computing

Handle micro/milli seconds in twig time filter

Tested retry behavior

Fixed waiting_time for messages with delay
@bendavies bendavies force-pushed the miliseconds-for-stats-rebased branch from 469c5b4 to 1fed242 Compare May 25, 2023 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants