Skip to content

Online GC: Add workers

João Pereira requested to merge 256-gc-agent into master

This is part of #256 (closed) and adds two worker implementations, one for blobs and another for manifests. Each worker processes GC tasks from its own queue and acts as documented in the spec. I've split this into two workers so that we can control and configure each one individually. These workers interact with both storage (storage backend) and datastore (database) packages, so I've added these to a new gc package at the same level as these.

There is a myriad of possible error scenarios involving storage and database backends. Even more important is how these are interleaved and how we react to a chain of events across backends. Some of these scenarios involve internal database/storage/network errors that we cannot simulate in integration tests. As GC is a critical and sensible process, we must test not only the happy path but, most importantly, every single error scenario, as these may lead to data corruption/inconsistencies.

For this reason, this MR introduces interfaces whenever necessary to enable us to mock and test the interactions between all components, asserting the GC behavior under all possible scenarios. While the most obvious option would be to rely on https://pkg.go.dev/github.com/stretchr/testify/mock (because we already use its assertions package, and it's used by other Go applications at GitLab, like Runner), this library lacks one key feature: the ability to assert the order of invocations. This is necessary for our use case, for example, to distinguish between a database rollback call that should happen at the very end of the call stack (deferred) and not at the beginning or in the middle. For this reason, I decided to use the official https://pkg.go.dev/github.com/golang/mock/gomock package. Mocks were generated automatically by its mockgen tool. A new CI job was added to guarantee that mocks remain up to date with the corresponding implementations.

With this strategy, I was able to achieve 100% test coverage. The only thing that these tests don't cover is the internal behavior/effect of the datastore and storage backend methods (as they're mocked). Still, these are already heavily tested in their own datastore and storage packages. Regardless, the next step (separate MR) will be adding an Agent component responsible for managing these two workers in the background. I intend to add a layer of integration tests on top of this, using database snapshots/golden files.

Edited by João Pereira

Merge request reports

Loading