-
-
Notifications
You must be signed in to change notification settings - Fork 199
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
Add stats about job classes #1362
Conversation
arnaudlevy
commented
May 25, 2024
@arnaudlevy I'm excited where you're going with this! fyi, there is some discussion in #438 about such a feature. Let me know when you'd like me to review it and give you feedback. |
Hey @bensheldon, glad you like it! Maybe what I did there could be a first step. Tell me what you think about the names, so I can walk in your steps and adjust. I put all chart config back in ruby, is that ok? |
@bensheldon what do you think about denormalizing |
@arnaudlevy there's a little about storing the latency in #1053. I would say:
|
Sorry if that's a lot. Only two changes are truly necessary I think (calculate in DB, rename to Performance) |
Hey @bensheldon ! The idea, for us, is to spot the "slow elephants", the ones that are really long. |
Don't know if you noticed @bensheldon but @SebouChu added a fix to the filters |
Nice! Could you break that out into its own PR? On that same note, I'd like to isolate this PR just down to the |
Hi @bensheldon! The filters fix is now on this PR if you want: #1373 ! |
@bensheldon Just finished adjustments here, mainly having one SQL request for the Performances#index, and focusing on the page only. Right now, the request gets the execution duration with |
@bensheldon I made the adjustment in the Performance Controller to use duration instead of extracting EPOCH, should be good now! |
I had to open a new PR (#1388) in order to push up changes to it (it seems like when a PR is opened from a branch named |
Thanks @bensheldon, that's really cool! |
Perfect, thanks! |