( )⚙ D420 remotefilelog: limit number of changesets to be prefetched

This is an archive of the discontinued Mercurial Phabricator instance.

remotefilelog: limit number of changesets to be prefetched
ClosedPublic

Authored by ms2316 on Aug 16 2017, 1:55 PM.
Tags
None
Subscribers

Details

Summary

Added config option 'prefetchdays' which indicates that commits older than
'prefetchdays' days should not be prefetched. This option is necessary to avoid
prefetch of huge amount of data. The default value is set to 14 days.

Test Plan

Ensure that unit tests pass

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ms2316 created this revision.Aug 16 2017, 1:55 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 16 2017, 1:55 PM

The main change looks good, but I have some comments about the test updates.

remotefilelog/__init__.py
949–953

Is it worth adding a helper function that updates an input revset with the date
restriction from the prefetchdays config? Then you could use it both here
and above.

tests/test-remotefilelog-bgprefetch.t
34–35

The style checker doesn't run on the .t files, but I would vote for wrapping
these comments to 80 columns, just like we do for *.py files and most other
text files.

39

Isn't this expression setting prefetchdays to a number of seconds rather than
days? If I'm reading this correctly, this is 100 seconds in the future, as a
number of seconds from the epoch.

This is probably achieving what you want by asking for all commits in the last
4+ million years, but it doesn't match the behavior described in the comment.

Would it be worth updating the code to prefetch all commits if the prefetchdays
parameter is 0 or negative? This way you could set it to 0 here and not have
to worry about computing a large enough value here.

Thanks for the comment. I will make some changes now.

tests/test-remotefilelog-bgprefetch.t
39

Ooops, I forgot to divide it by 86400. But I quite liked your idea about setting it to zero and adding a helper function.

ms2316 marked 3 inline comments as done.Aug 16 2017, 4:06 PM
ms2316 updated this revision to Diff 1012.

Added a helper function that updates a revset by adding to it date restriction from prefetchdays config.

This revision is now accepted and ready to land.Aug 16 2017, 4:17 PM
ms2316 marked an inline comment as done.Aug 16 2017, 4:20 PM
ms2316 updated this revision to Diff 1013.

Corrected a spelling mistake

Pushed to @

This revision was automatically updated to reflect the committed changes.