Skip to content

Fix FetchIntoObjectPool writing outside of a transaction

Sami Hiltunen requested to merge smh-fix-fetch-into-object-pool into master

FetchIntoObjectPool has custom logic for managing its transaction as opposed to using the general logic from the middleware. The custom logic has multiple issues which lead to disk access outside of a transaction.

  1. The transaction is only started in FetchFromOrigin. This leads to non-transactional disk access already in the handler in objectpool.FromProto. That function for example checks whether the repository exists on the disk which would lead to an isolation issue if something was to create or remove it concurrently.
  2. FetchFromOrigin has a similar issue as it checks whether the object pool exists before starting a transaction.
  3. The transaction closure is operating on the object pool directly by fetching into it and running various housekeeping utilities on it. Only the read access in the origin repository happens within a transaction.
  4. Finally, the LogRepositoryInfo call in FetchIntoObjectPool is running outside of a transaction and accessing the repository directly.

This commit addresses all of those issues. We remove the exclusion in the transaction middleware to ensure the handler only gets access to a repository that has already been rewritten. This prevents future failures as the operations don't by default target the repository directly but the transaction's snapshot.

As OptimizeRepository runs in a different transaction, we commit the current one before calling it. This way OptimizeRepository works on the state that was written out by the fetch.

Finally, the LogRepositoryInfo call is ran in its own transaction. This is necessary currently as the housekeeping has been embedded in the transaction manager's commit process rather than running outside of it. We can't gather the statistics before the transaction is committed. This can be improved in the future as we extract the housekeeping back into the 'housekeeping' package it belongs in.

Closes #6138 (closed)

Edited by Sami Hiltunen

Merge request reports