mirror of
https://github.com/facebook/sapling.git
synced 2024-10-07 15:27:13 +03:00
remotefilelog: make pack cleanup more robust
Summary: We're seeing exceptions from the unlink cleanup logic hide the actual exception from the code. In order to debug the actual exception, we need to eat the cleanup exceptions and in any place where we eat an exception inside of a catch block, we need to manually rethrow the original exception, otherwise we end up throwing the second caught exception (from the cleanup logic). Test Plan: Ran the tests. This is a particular odd edge case, so I need to run these bits in the production environment where it was occuring before I can understand why it was happening. Reviewers: #fbhgext, ms2316 Reviewed By: ms2316 Subscribers: ms2316 Differential Revision: https://phab.mercurial-scm.org/D586
This commit is contained in:
parent
d65b5a13b9
commit
b91fc3d106
@ -286,11 +286,7 @@ class mutablebasepack(versionmixin):
|
||||
|
||||
def abort(self):
|
||||
# Unclean exit
|
||||
try:
|
||||
self.opener.unlink(self.packpath)
|
||||
self.opener.unlink(self.idxpath)
|
||||
except Exception:
|
||||
pass
|
||||
self._cleantemppacks()
|
||||
|
||||
def writeraw(self, data):
|
||||
self.packfp.write(data)
|
||||
@ -307,20 +303,24 @@ class mutablebasepack(versionmixin):
|
||||
|
||||
if len(self.entries) == 0:
|
||||
# Empty pack
|
||||
self.opener.unlink(self.packpath)
|
||||
self.opener.unlink(self.idxpath)
|
||||
self._cleantemppacks()
|
||||
self._closed = True
|
||||
return None
|
||||
|
||||
self.opener.rename(self.packpath, sha + self.PACKSUFFIX)
|
||||
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.rename(self.idxpath, sha + self.INDEXSUFFIX)
|
||||
except Exception as ex:
|
||||
try:
|
||||
self.opener.unlink(path)
|
||||
self.opener.unlink(sha + self.PACKSUFFIX)
|
||||
except Exception:
|
||||
pass
|
||||
# Throw exception 'ex' explicitly since a normal 'raise' would
|
||||
# potentially throw an exception from the unlink cleanup.
|
||||
raise ex
|
||||
except Exception:
|
||||
# Clean up temp packs in all exception cases
|
||||
self._cleantemppacks()
|
||||
raise
|
||||
|
||||
self._closed = True
|
||||
@ -329,6 +329,16 @@ class mutablebasepack(versionmixin):
|
||||
ledger.addcreated(result)
|
||||
return result
|
||||
|
||||
def _cleantemppacks(self):
|
||||
try:
|
||||
self.opener.unlink(self.packpath)
|
||||
except Exception:
|
||||
pass
|
||||
try:
|
||||
self.opener.unlink(self.idxpath)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
def writeindex(self):
|
||||
rawindex = ''
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user