Code review comment for ~nacc/git-ubuntu:lp1730734-cache-importer-progress

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I'm not sure I can help you with the "implications" as you asked on IRC without thinking much more into it. But I have a few questions that might help to clarify at least.

To do so I want to hear more details on the actual use-case that lets us run into this race:
1) Do we even want that two importers run concurrently on the same system?
   If not lets just lock the DB to be of single use - problem solved.

2) If we want multiple processes to be allowed we could make the use of
   the cache exclusive per package. (I beg your pardon if I don't see the point why this would
   fail). So only one worker can be in said [package] at any time.
   Processes working on different packages should never conflict right?
   Another worker with a different [package] can continue and we know there won't be a race.
   Essentially locking on [package] (yes this might need a better db to support
   rollback/timeouts on aborted processes if you want to lock in the DB).
   The tail of the processing would be updating sphhr and then unlocking the given package.

In general I think you should add tests along these commits that e.g. does "import-no-cache-used == import into cache == import with cache".

« Back to merge proposal