Skip to content

Conversation

@Intrepidd
Copy link
Contributor

Necessary in order to implement this : rails/mission_control-jobs#114

I added the index on all executions tables but this may well be overkill, happy to change it if you believe we should only add it on failed executions for now

@Intrepidd Intrepidd force-pushed the created-at-indexes branch from bd8d55d to f2afb35 Compare April 12, 2024 08:55
@rosa
Copy link
Member

rosa commented Apr 12, 2024

Thanks @Intrepidd! I continued thinking about that one... and what if we sorted failed_executions on job_id, but descending? This is what we do for recurring_executions in Mission Control. In that case the sort is going to mimic very well the creation date order because recurring executions are created at the same time as the job. For failed executions, sorting on the job_id won't correspond exactly to the same order jobs failed because it depends on how long a job takes to fail, but I think it should be a good approximation in most cases. In this way, we wouldn't need any new indexes.

@Intrepidd
Copy link
Contributor Author

I thought about it but it would not correctly handle retries would it ? You probably want on top of the pages the most recent failures.

Imagine a job failed a few days ago sitting in page 3, if you retry it and it fails again you want to see it on top of page one don't you ?

@rosa
Copy link
Member

rosa commented Apr 12, 2024

Hmm yes, that's true 🤔 Perhaps we could sort by primary key instead 🤔 That is, failed_executions.id.

@rosa
Copy link
Member

rosa commented Apr 12, 2024

Depending on the filtering, no index could be used for sorting, though. But the same would be true for a new index on created_at anyway. It'd only be used when no filtering is done. I had suggested job_id because I think, with the joins, we do for filtering, we could also use the index on job_id for sorting, but the retries prevent that, as you said.

@Intrepidd
Copy link
Contributor Author

Hmm yes, that's true 🤔 Perhaps we could sort by primary key instead 🤔 That is, failed_executions.id.

This seems obvious now :D I'll make the changes in the other PR

@rosa
Copy link
Member

rosa commented Apr 12, 2024

Awesome! Thank you 🙏🏻

@Intrepidd Intrepidd closed this Apr 12, 2024
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.

2 participants