# HG changeset patch # User Matt Harbison # Date 1561864987 14400 # Node ID 2c27b7fadcd3afabdcd9f72a6984166a8f6e39f4 # Parent a504aed0a78a06cc76f001cbd5fa562eefa0315f 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. diff -r a504aed0a78a -r 2c27b7fadcd3 mercurial/localrepo.py --- a/mercurial/localrepo.py Sat Jun 22 23:04:52 2019 -0400 +++ b/mercurial/localrepo.py Sat Jun 29 23:23:07 2019 -0400 @@ -1222,55 +1222,6 @@ @mixedrepostorecache(('bookmarks', 'plain'), ('bookmarks.current', 'plain'), ('00changelog.i', '')) def _bookmarks(self): - # Since the multiple files involved in the transaction cannot be - # written atomically (with current repository format), there is a race - # condition here. - # - # 1) changelog content A is read - # 2) outside transaction update changelog to content B - # 3) outside transaction update bookmark file referring to content B - # 4) bookmarks file content is read and filtered against changelog-A - # - # When this happens, bookmarks against nodes missing from A are dropped. - # - # Having this happening during read is not great, but it become worse - # when this happen during write because the bookmarks to the "unknown" - # nodes will be dropped for good. However, writes happen within locks. - # This locking makes it possible to have a race free consistent read. - # For this purpose data read from disc before locking are - # "invalidated" right after the locks are taken. This invalidations are - # "light", the `filecache` mechanism keep the data in memory and will - # reuse them if the underlying files did not changed. Not parsing the - # same data multiple times helps performances. - # - # Unfortunately in the case describe above, the files tracked by the - # bookmarks file cache might not have changed, but the in-memory - # content is still "wrong" because we used an older changelog content - # to process the on-disk data. So after locking, the changelog would be - # refreshed but `_bookmarks` would be preserved. - # Adding `00changelog.i` to the list of tracked file is not - # enough, because at the time we build the content for `_bookmarks` in - # (4), the changelog file has already diverged from the content used - # for loading `changelog` in (1) - # - # To prevent the issue, we force the changelog to be explicitly - # reloaded while computing `_bookmarks`. The data race can still happen - # without the lock (with a narrower window), but it would no longer go - # undetected during the lock time refresh. - # - # The new schedule is as follow - # - # 1) filecache logic detect that `_bookmarks` needs to be computed - # 2) cachestat for `bookmarks` and `changelog` are captured (for book) - # 3) We force `changelog` filecache to be tested - # 4) cachestat for `changelog` are captured (for changelog) - # 5) `_bookmarks` is computed and cached - # - # The step in (3) ensure we have a changelog at least as recent as the - # cache stat computed in (1). As a result at locking time: - # * if the changelog did not changed since (1) -> we can reuse the data - # * otherwise -> the bookmarks get refreshed. - self._refreshchangelog() return bookmarks.bmstore(self) def _refreshchangelog(self): diff -r a504aed0a78a -r 2c27b7fadcd3 tests/test-bookmarks-corner-case.t --- a/tests/test-bookmarks-corner-case.t Sat Jun 22 23:04:52 2019 -0400 +++ b/tests/test-bookmarks-corner-case.t Sat Jun 29 23:23:07 2019 -0400 @@ -200,7 +200,6 @@ $ cat push-output.txt pushing to ssh://user@dummy/bookrace-server searching for changes - remote has heads on branch 'default' that are not known locally: f26c3b5167d1 remote: setting raced push up remote: adding changesets remote: adding manifests @@ -220,7 +219,7 @@ | summary: A1 | | o changeset: 3:f26c3b5167d1 - | | bookmark: book-B + | | bookmark: book-B (false !) | | user: test | | date: Thu Jan 01 00:00:00 1970 +0000 | | summary: B1 @@ -243,4 +242,4 @@ $ hg -R bookrace-server book book-A 4:9ce3b28c16de - book-B 3:f26c3b5167d1 + book-B 3:f26c3b5167d1 (false !)