Mercurial > public > mercurial-scm > hg
comparison mercurial/localrepo.py @ 42545:2c27b7fadcd3 stable
bookmarks: backout the attempt to fix the delete race
This backs out 044045dce23a because it broke a bunch of tests on Windows.
Yuya's theory is that we still rely on in-memory changelog data to be flushed
out of the transaction.
| author | Matt Harbison <matt_harbison@yahoo.com> |
|---|---|
| date | Sat, 29 Jun 2019 23:23:07 -0400 |
| parents | 044045dce23a |
| children | 904e0da2e195 |
comparison
equal
deleted
inserted
replaced
| 42515:a504aed0a78a | 42545:2c27b7fadcd3 |
|---|---|
| 1220 return cls(self, name, visibilityexceptions) | 1220 return cls(self, name, visibilityexceptions) |
| 1221 | 1221 |
| 1222 @mixedrepostorecache(('bookmarks', 'plain'), ('bookmarks.current', 'plain'), | 1222 @mixedrepostorecache(('bookmarks', 'plain'), ('bookmarks.current', 'plain'), |
| 1223 ('00changelog.i', '')) | 1223 ('00changelog.i', '')) |
| 1224 def _bookmarks(self): | 1224 def _bookmarks(self): |
| 1225 # Since the multiple files involved in the transaction cannot be | |
| 1226 # written atomically (with current repository format), there is a race | |
| 1227 # condition here. | |
| 1228 # | |
| 1229 # 1) changelog content A is read | |
| 1230 # 2) outside transaction update changelog to content B | |
| 1231 # 3) outside transaction update bookmark file referring to content B | |
| 1232 # 4) bookmarks file content is read and filtered against changelog-A | |
| 1233 # | |
| 1234 # When this happens, bookmarks against nodes missing from A are dropped. | |
| 1235 # | |
| 1236 # Having this happening during read is not great, but it become worse | |
| 1237 # when this happen during write because the bookmarks to the "unknown" | |
| 1238 # nodes will be dropped for good. However, writes happen within locks. | |
| 1239 # This locking makes it possible to have a race free consistent read. | |
| 1240 # For this purpose data read from disc before locking are | |
| 1241 # "invalidated" right after the locks are taken. This invalidations are | |
| 1242 # "light", the `filecache` mechanism keep the data in memory and will | |
| 1243 # reuse them if the underlying files did not changed. Not parsing the | |
| 1244 # same data multiple times helps performances. | |
| 1245 # | |
| 1246 # Unfortunately in the case describe above, the files tracked by the | |
| 1247 # bookmarks file cache might not have changed, but the in-memory | |
| 1248 # content is still "wrong" because we used an older changelog content | |
| 1249 # to process the on-disk data. So after locking, the changelog would be | |
| 1250 # refreshed but `_bookmarks` would be preserved. | |
| 1251 # Adding `00changelog.i` to the list of tracked file is not | |
| 1252 # enough, because at the time we build the content for `_bookmarks` in | |
| 1253 # (4), the changelog file has already diverged from the content used | |
| 1254 # for loading `changelog` in (1) | |
| 1255 # | |
| 1256 # To prevent the issue, we force the changelog to be explicitly | |
| 1257 # reloaded while computing `_bookmarks`. The data race can still happen | |
| 1258 # without the lock (with a narrower window), but it would no longer go | |
| 1259 # undetected during the lock time refresh. | |
| 1260 # | |
| 1261 # The new schedule is as follow | |
| 1262 # | |
| 1263 # 1) filecache logic detect that `_bookmarks` needs to be computed | |
| 1264 # 2) cachestat for `bookmarks` and `changelog` are captured (for book) | |
| 1265 # 3) We force `changelog` filecache to be tested | |
| 1266 # 4) cachestat for `changelog` are captured (for changelog) | |
| 1267 # 5) `_bookmarks` is computed and cached | |
| 1268 # | |
| 1269 # The step in (3) ensure we have a changelog at least as recent as the | |
| 1270 # cache stat computed in (1). As a result at locking time: | |
| 1271 # * if the changelog did not changed since (1) -> we can reuse the data | |
| 1272 # * otherwise -> the bookmarks get refreshed. | |
| 1273 self._refreshchangelog() | |
| 1274 return bookmarks.bmstore(self) | 1225 return bookmarks.bmstore(self) |
| 1275 | 1226 |
| 1276 def _refreshchangelog(self): | 1227 def _refreshchangelog(self): |
| 1277 """make sure the in memory changelog match the on-disk one""" | 1228 """make sure the in memory changelog match the on-disk one""" |
| 1278 if ('changelog' in vars(self) and self.currenttransaction() is None): | 1229 if ('changelog' in vars(self) and self.currenttransaction() is None): |