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

Added GoodJob::DiscreteExecution#duration column #1374

Merged
merged 12 commits into from
Jul 5, 2024

Conversation

SebouChu
Copy link
Contributor

@SebouChu SebouChu commented Jun 13, 2024

Added a duration attribute to GoodJob::DiscreteExecution, containing the monotonic execution time.

The runtime_latency method now uses this attribute if migrated correctly.

Should I also add this attribute to GoodJob::Execution and set it when we set the finished_at attribute? Because in #1362, I think we should work with GoodJob::Execution and not GoodJob::DiscreteExecution. What do you think @bensheldon?

@bensheldon
Copy link
Owner

Should I also add this attribute to GoodJob::Execution and set it when we set the finished_at attribute?

No, GoodJob::Execution as it currently is will be going away, and there will just be GoodJob::Job and GoodJob::DiscreteExecution (which will be renamed to GoodJob::Execution once that happens, just to be extra confusing to explain 😁 ).

The way I've described it in my draft v4 upgrade guide is:

GoodJob v4 changes how job and job execution records are stored in the database; moving from job and executions being commingled in the good_jobs table to separately and discretely storing job executions in good_job_executions.

So if we want to calculate across every execution of a job, we want to use DiscreteExecutions.

@SebouChu
Copy link
Contributor Author

@bensheldon okay perfect!

@SebouChu
Copy link
Contributor Author

Well PR should be ready then, and I'll make adjustments to @arnaudlevy's PR about the Performance tab

@bensheldon
Copy link
Owner

Thank you! This is great! I spoke with some Rails/Postgres experts and they recommended using a Postgres interval type (which would also mean we could just rename this to duration). I can make that change if you don't get to it before me.

@SebouChu
Copy link
Contributor Author

Thank you! This is great! I spoke with some Rails/Postgres experts and they recommended using a Postgres interval type (which would also mean we could just rename this to duration). I can make that change if you don't get to it before me.

@bensheldon Will do! Just checked, interval type is compatible with Rails starting from 6.1.0, but as GJ v4 will be Rails 6.1+, shouldn't be an issue!

@SebouChu
Copy link
Contributor Author

@bensheldon Tell me if it's good for you!

@bensheldon bensheldon changed the title Added GoodJob::DiscreteExecution#duration_ms Added GoodJob::DiscreteExecution#duration column Jul 5, 2024
@bensheldon
Copy link
Owner

For future reference, this was a nice description of difference between latency and duration: HangfireIO/Hangfire#95 (comment)

@bensheldon bensheldon merged commit 2b17bb7 into bensheldon:main Jul 5, 2024
@bensheldon bensheldon added the enhancement New feature or request label Jul 5, 2024
@SebouChu SebouChu deleted the add-execution-duration branch July 5, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants