Batch order in artifacts size refresh script can be problematic
Summary
This is from #360375 (comment 945736507).
The current way we fetch the next batch of artifacts which uses order(:created_at)
can cause problems.
Here's a sample scenario. Imagine we have limit: 3
and the following ci_job_artifacts
records:
id | created_at |
---|---|
1 | 00:01 |
2 | 00:02 |
3 | 00:01 |
4 | 00:04 |
5 | 00:05 |
This could happen because created_at
is set before the ID: https://stackoverflow.com/a/5819113
The next_batch(limit: 3)
could return the ids in the order 1, 3, 2
since we do order(:created_at)
. As we store last_job_artifact_id: batch.last.id
when we finish the batch (which is 2
in this example). We may count the record with ID 3
again in next_batch(limit: 3) --> 3, 4, 5
.
Possible fixes
We should order(:id)
so we have like-with-like comparison. But note that this may not be as simple as it sounds. We need to ensure that the index will still be used and the query will still be performant. In the past, when initially we tried to use order(:id)
here, it was taking very long.