Code review comment for lp:~stevenk/launchpad/populate-bprc

Revision history for this message
William Grant (wgrant) wrote :

> [2]
>
> + value = getUtility(IMemcacheClient).get('populate-bprc')
> + if not value:
> + self.start_at = 0
> + else:
> + self.start_at = value
>
> Woah! :)
>
> There are four memcache servers in production, all configured with the
> same weighting, so this code has a 3-in-4 chance that it will not
> retrieve what was written at then end of the last time through
> PopulateBinaryPackageReleaseContents.__call__().

This is not correct: memcached clients use a hash of the key to determine which server is to be used.

> Also, memcache is expected to forget things. Combined with the above
> gives a less than 1-in-4 chance of starting the loop again from where
> it was last terminated.

This is a pattern that we've used before. Production memcached is under very little memory pressure and this value is frequently updated, so it rarely forgets things like this. Even if it does forget, the job just has to redo a bit of work on the failed imports. Harmless.

« Back to merge proposal