This is an archive of the discontinued Mercurial Phabricator instance.

Differential D61

repack: better error handling
ClosedPublic

Authored by durham on Jul 12 2017, 12:51 PM.
Tags
None
Subscribers
None

Details

Reviewers
quark
Group Reviewers
Restricted Project
Commits
rFBHGX09ed28810217: repack: better error handling
Summary

We've seen certain machines end up with just datapack files and no dataidx
files. While we can't repro this, the only time this could possibly happen is
between the rename of the two temp files. So let's add error handling around
that logic.

Test Plan

Ran the tests. Manually inserted an exception between the renames, ran the tests with --keep-tmpdir, and verified there were no packs left over when the exception fired.

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

durham created this revision.Jul 12 2017, 12:51 PM
durham edited the test plan for this revision. (Show Details)Jul 12 2017, 12:55 PM
quark accepted this revision.Jul 13 2017, 1:26 PM
This revision is now accepted and ready to land.Jul 13 2017, 1:26 PM
Closed by commit rFBHGX09ed28810217: repack: better error handling (authored by durham). · Explain WhyJul 13 2017, 3:56 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mremotefilelog/basepack.py (31 lines)

Diff 114

remotefilelog/basepack.py

def writeraw(self, data): def writeraw(self, data):
self.packfp.write(data) self.packfp.write(data)
self.sha.update(data) self.sha.update(data)
def close(self, ledger=None): def close(self, ledger=None):
if self._closed: if self._closed:
return return
try:
sha = self.sha.hexdigest() sha = self.sha.hexdigest()
self.packfp.close() self.packfp.close()
self.writeindex() self.writeindex()
if len(self.entries) == 0: if len(self.entries) == 0:
# Empty pack # Empty pack
self.opener.unlink(self.packpath) self.opener.unlink(self.packpath)
self.opener.unlink(self.idxpath) self.opener.unlink(self.idxpath)
self._closed = True self._closed = True
return None return None
self.opener.rename(self.packpath, sha + self.PACKSUFFIX) self.opener.rename(self.packpath, sha + self.PACKSUFFIX)
self.opener.rename(self.idxpath, sha + self.INDEXSUFFIX) self.opener.rename(self.idxpath, sha + self.INDEXSUFFIX)
except Exception:
for path in [self.packpath, self.idxpath,
sha + self.PACKSUFFIX, sha + self.INDEXSUFFIX]:
try:
self.opener.unlink(path)
except Exception:
pass
raise
self._closed = True self._closed = True
result = self.opener.join(sha) result = self.opener.join(sha)
if ledger: if ledger:
ledger.addcreated(result) ledger.addcreated(result)
return result return result
def writeindex(self): def writeindex(self):