This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1523

repack: delete excessively big packfiles before incremental repack
ClosedPublic

Authored by phillco on Nov 27 2017, 4:37 PM.

Details

Reviewers
durham
Group Reviewers
Restricted Project
Commits
rFBHGXa3fa61f9c38f: repack: delete excessively big packfiles before incremental repack
Summary

Abnormally huge packfiles (20 GB+) are unwanted and can cause more problems
than they solve. Let's add a config to simply delete them at the beginning of
the repack proces.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

phillco created this revision.Nov 27 2017, 4:37 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 27 2017, 4:37 PM
phillco retitled this revision from repck: delete excessively big packfiles before incremental repack to repack: delete excessively big packfiles before incremental repack.Nov 27 2017, 4:38 PM
phillco updated this revision to Diff 3895.
durham accepted this revision.Nov 27 2017, 11:27 PM
durham added a subscriber: durham.
durham added inline comments.
remotefilelog/repack.py
156

I'd use ui.configbytes(...), that way users can specify values like '512M' and '20G'

184

I'd probably make this two lines for clarity.

files = osutil.listdir(packpath, stat=True)
files = _deletebigpacks(repo, packpath, files)
This revision is now accepted and ready to land.Nov 27 2017, 11:27 PM
phillco added a subscriber: simpkins.Nov 28 2017, 3:03 PM

Based on yesterday's conversation, do we need to limit this to remotefilelog for now? @simpkins

durham added a comment.Nov 28 2017, 7:33 PM

Probably safe to limit it to remotefilelog. We can expand it as needed.

singhsrb added a subscriber: singhsrb.Nov 29 2017, 4:36 PM
singhsrb added inline comments.
remotefilelog/repack.py
289

You can even remove the big files at this point. Something like

if size > maxsize:
    # Delete the file
elif size > limit:
    # Use the current code.

Would reuse a lot of work.

phillco marked 2 inline comments as done.Nov 30 2017, 3:33 PM
phillco updated this revision to Diff 4016.
phillco added inline comments.Nov 30 2017, 3:34 PM
remotefilelog/repack.py
289

I think this is the size of the index file, not the data file.

phillco closed this revision.Nov 30 2017, 3:47 PM

Landed, but I could submit a rewrite to reuse the existing iteration points.

Revision Contents
Changeset List

Diff 4016

remotefilelog/repack.py

return ((localpackpath, localdatastores, localhistorystores), return ((localpackpath, localdatastores, localhistorystores),
(sharedpackpath, shareddatastores, sharedhistorystores)) (sharedpackpath, shareddatastores, sharedhistorystores))
def _topacks(packpath, files, constructor): def _topacks(packpath, files, constructor):
paths = list(os.path.join(packpath, p) for p in files) paths = list(os.path.join(packpath, p) for p in files)
packs = list(constructor(p) for p in paths) packs = list(constructor(p) for p in paths)
return packs return packs
def _deletebigpacks(repo, folder, files):
"""Deletes packfiles that are bigger than ``packs.maxpacksize``.
Returns ``files` with the removed files omitted."""
maxsize = repo.ui.configbytes("packs", "maxpacksize")
durhamUnsubmitted
Done

I'd use ui.configbytes(...), that way users can specify values like '512M' and '20G'

durham: I'd use ui.configbytes(...), that way users can specify values like '512M' and '20G'
if maxsize <= 0:
return files
# This only considers datapacks today, but we could broaden it to include
# historypacks.
VALIDEXTS = [".datapack", ".dataidx"]
# Either an oversize index or datapack will trigger cleanup of the whole
# pack:
oversized = set([os.path.splitext(path)[0] for path, ftype, stat in files
if (stat.st_size > maxsize and (os.path.splitext(path)[1]
in VALIDEXTS))])
for rootfname in oversized:
rootpath = os.path.join(folder, rootfname)
for ext in VALIDEXTS:
path = rootpath + ext
repo.ui.debug('removing oversize packfile %s (%s)\n' %
(path, util.bytecount(os.stat(path).st_size)))
os.unlink(path)
return [row for row in files if os.path.basename(row[0]) not in oversized]
def _incrementalrepack(repo, datastore, historystore, packpath, category, def _incrementalrepack(repo, datastore, historystore, packpath, category,
allowincompletedata=False, options=None): allowincompletedata=False, options=None):
shallowutil.mkstickygroupdir(repo.ui, packpath) shallowutil.mkstickygroupdir(repo.ui, packpath)
files = osutil.listdir(packpath, stat=True) files = osutil.listdir(packpath, stat=True)
files = _deletebigpacks(repo, packpath, files)
durhamUnsubmitted
Done

I'd probably make this two lines for clarity.

files = osutil.listdir(packpath, stat=True)
files = _deletebigpacks(repo, packpath, files)
durham: I'd probably make this two lines for clarity. ``` files = osutil.listdir(packpath, stat=True)…
datapacks = _topacks(packpath, datapacks = _topacks(packpath,
_computeincrementaldatapack(repo.ui, files), _computeincrementaldatapack(repo.ui, files),
datapack.datapack) datapack.datapack)
datapacks.extend(s for s in datastore datapacks.extend(s for s in datastore
if not isinstance(s, datapack.datapackstore)) if not isinstance(s, datapack.datapackstore))
historypacks = _topacks(packpath, historypacks = _topacks(packpath,
_computeincrementalhistorypack(repo.ui, files), _computeincrementalhistorypack(repo.ui, files),
generations.append([]) generations.append([])
sizes = {} sizes = {}
for prefix, mode, stat in _allpackfileswithsuffix(files, packsuffix, for prefix, mode, stat in _allpackfileswithsuffix(files, packsuffix,
indexsuffix): indexsuffix):
size = stat.st_size size = stat.st_size
sizes[prefix] = size sizes[prefix] = size
for i, limit in enumerate(limits): for i, limit in enumerate(limits):
if size > limit: if size > limit:
singhsrbUnsubmitted
Not Done

You can even remove the big files at this point. Something like

if size > maxsize:
    # Delete the file
elif size > limit:
    # Use the current code.

Would reuse a lot of work.

singhsrb: You can even remove the big files at this point. Something like ``` if size > maxsize: #…
phillcoAuthorUnsubmitted
Not Done

I think this is the size of the index file, not the data file.

phillco: I think this is the size of the index file, not the data file.
generations[i].append(prefix) generations[i].append(prefix)
break break
# Steps for picking what packs to repack: # Steps for picking what packs to repack:
# 1. Pick the largest generation with >2 pack files. # 1. Pick the largest generation with >2 pack files.
# 2. Take the smallest three packs. # 2. Take the smallest three packs.
# 3. While total-size-of-packs < repacksizelimit: add another pack # 3. While total-size-of-packs < repacksizelimit: add another pack

tests/test-remotefilelog-repack.t

1406e7411862 aee31534993a 12 2 1406e7411862 aee31534993a 12 2
Total: 36 20 (80.0% bigger) Total: 36 20 (80.0% bigger)
y: y:
Node Delta Base Delta Length Blob Size Node Delta Base Delta Length Blob Size
577959738234 000000000000 8 8 577959738234 000000000000 8 8
Total: 8 8 (0.0% bigger) Total: 8 8 (0.0% bigger)
Test huge pack cleanup using different values of packs.maxpacksize:
$ hg repack --incremental --debug
$ hg repack --incremental --debug --config packs.maxpacksize=512
removing oversize packfile $TESTTMP/hgcache/master/packs/a2731c9a16403457b67337a620931797fce8c821.datapack (365 bytes)
removing oversize packfile $TESTTMP/hgcache/master/packs/a2731c9a16403457b67337a620931797fce8c821.dataidx (1.21 KB)