Project statistics: introduce database buffered counters

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Close this issue

From !139990 (comment 1727464820):

🔥 Problem: sidekiq killed

This was one of my suspicions during the meeting we had on those negative sizes on project statistics.

For a different background job, we noticed pretty quickly that:

• the job execution could get ended.
• it is not ended in a nice way. Even rescue blocks are not be executed (<- 😱).

It was very noticeable because on that job, we had a state machine. When the worker would pick up the next element to work on, its state would change to ongoing. We noticed that with time, we had ongoing elements left behind even though their related jobs was previously started several days ago. (context is here).

I was told that Sidekiq restarts could be done in a "not so nice" way. This is issue Send TSTP signal to sidekiq before upgrade (gitlab-com/gl-infra/delivery#603). From what I see, it is still opened today. I would assume that this is still happening.

For our ongoing left overs, we had a different background job that would look for those "stale" executions and simply restart them.

🔍 Monitoring this

As we said background jobs get killed in a pretty abrupt way. However, Sidekiq will still re-enqueue the job that is ended and log a message. We will see something like Pushed job <jid> back to queue <queue_name>.

In Improve logs of the Sidekiq reliable fetcher (!139932 - merged), I updated the logs so that we also have the job class. This way, we can check if the FlushCounterIncrementsWorker gets terminated by Sidekiq restarts.

Here is the Kibana search.

So far, I didn't see any occurence. Thus, I can't confirm the above suspicion.

It can happen.

Impact

What happens if FlushCounterIncrementsWorker gets terminated?

Well, I only have a basic understanding of that worker but I think we can summarize it as two operations (order is not that important):

1. Redis operation.
2. Database operation.

The problem we can see here is that the job could get "cut" right between these two operations. Which means that only one of them has been executed. When that happens, then we miss a statistic update (or we could apply it twice). That could explain how we can have sync issues between the updates buffered on redis and the database.

What we want is really a transaction: either both operations succeed or neither of them succeed.

💡 Possible solution (aka Wild Idea ™️)

As said above, we need a transaction. Now, a transaction with two different systems (redis / database) seems quite challenging. I'm not even sure that this exists.

Which solutions do we have?

I've been wondering about this wild idea: could we get rid of Redis? Basically, we have a Redis buffered counter. Could we have a Database buffered counter?

We can't directly update the project statistics because due to the amount of updates it receives, it can lead to slowdowns (due to row locks I assume).

What if we have a table in which we write (or better said buffer) the updates that are required? That table would be as simply as project_id, statistic, size_update (notice that we don't really need a primary key).

Then, have a similar worker than the FlushCounterIncrementsWorker: every ~15min, we have a worker that will aggregate things by project_id and do a single update on the related project statistic row.

Basically:

• When a project statistic update is required, insert a row in the new table.
  • That insert should not slow down things because I don't think we need any locks here (my database expertise is not that great here so this would need confirmation).
  • Additionally, this can be part of a transaction which would make those updates more reliable. Example: when a new package is uploaded, we need to bump the packages statistic. With that new table we could insert the new package in the packages table and insert the update in this "buffered updates" table within the same transaction. Same applies when a package is deleted.
• Because updates are buffered for 15min max, then it wouldn't be that challenging for the flush worker to directly aggregate all the rows to get the final update to do. Eg. publishing 3 packages will insert 3 updates in the new table. The flush worker can aggregate those 3 rows to get the actual update to do.
• Finally and this is the nicest thing, because the flush worker works only with the database, it can use a single transaction to:
  • update the related project statistics.
  • delete the rows in this new table. (we could use the created_at timestamp to improve the accuracy of this delete)
    • From https://medium.com/@timanovsky/ultra-fast-asynchronous-counters-in-postgres-44c5477303c3, we could DELETE and return the rows at the same time. Thanks @iamricecake for this link.
      • example:

        WITH deleted AS 
        (DELETE FROM packages_package_files WHERE package_id = 38 RETURNING *) 
        SELECT SUM(size) FROM deleted;

      • We could even try to delete, sum and update the project statistics in a single query. (this might not be a great idea).
  • -> This worker can get killed right in the middle of the execution, it doesn't matter. The database will always be coherent and the background job can be re-enqueued/restarted.

The above solution was a quick thought from my part. Take it with several grains of salt 😸

Use a feature flag for such deep change.