launchpad-reviewers team mailing list archive

[James Westby <jw+debian@xxxxxxxxxxxxxxx>]

On Wed, 30 May 2012 22:35:20 -0000, Robert Collins <robertc@xxxxxxxxxxxxxxxxx> wrote:
> Review: Needs Fixing
> 
> Do we have data on this? Replacing an unaltered file will cause apache
> to re-read it, which will increase load. That is, I believe, why the
> cmp is in there in the first place. Have we got data on how long the
> cmp is taking?

Good point. We have no data at this point, this was just something we
thought might help, but just based on considering the write side of
things, and not apache reading the files.

> I ask because this is exactly the sort of small tweak that can bite us rather hard.

Yeah, but so can performance optimisations done without data, such as
checking if the file has changed before overwriting it :-)

> Perhaps doing this under a feature flag will let it be phase in under
> examination and see if it has any impact.

If a feature flag is acceptable for this sort of thing then I agree that
it would be preferable to landing it without a trivial way to rollback
if it has an adverse affect.

Other ways to do that would be adding a config option, or a command line
option. A feature flag would definitely be cheaper if it is an
acceptable use for them.

Thanks,

James

-- 
https://code.launchpad.net/~james-w/launchpad/drop-filecmp/+merge/108021
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.