search

bensheldon / good_job

Multithreaded, Postgres-based, Active Job backend for Ruby on Rails.
https://goodjob-demo.herokuapp.com/
MIT License
2.53k stars 190 forks source link

Added GoodJob::DiscreteExecution#duration column #1374

Closed SebouChu closed 1 week ago

SebouChu commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@bensheldon okay perfect!

SebouChu commented 1 month ago

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

bensheldon commented 4 weeks ago

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 commented 4 weeks ago

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 commented 4 weeks ago

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

bensheldon commented 1 week ago

For future reference, this was a nice description of difference between latency and duration: https://github.com/HangfireIO/Hangfire/issues/95#issue-33917419