Patchwork [2,of,2] fsmonitor: don't attempt state-leave if we didn't state-enter

login
register
mail settings
Submitter Wez Furlong
Date May 18, 2017, 9:22 p.m.
Message ID <726d02dfef6afc703024.1495142523@devbig310.prn1.facebook.com>
Download mbox | patch
Permalink /patch/20691/
State Accepted
Headers show

Comments

Wez Furlong - May 18, 2017, 9:22 p.m.
# HG changeset patch
# User Wez Furlong <wez@fb.com>
# Date 1495136950 25200
#      Thu May 18 12:49:10 2017 -0700
# Node ID 726d02dfef6afc7030243cdbd43a4fc2ffb8be86
# Parent  f4b76c106f3af915667db7696e27780601e93074
fsmonitor: don't attempt state-leave if we didn't state-enter

The state-enter command may not have been successful; for example, the watchman
client session may have timed out if the user was busy/idle for a long period
during a merge conflict resolution earlier in processing a rebase for a stack
of diffs.

It's cleaner (from the perspective of the watchman logs) to avoid issuing the
state-leave command in these cases.

Test Plan:
ran

`hg rebase --tool :merge -r '(draft() & date(-14)) - master::' -d master`

and didn't observe any errors in the watchman logs or in the output from

`watchman -p -j <<<'["subscribe", "/data/users/wez/fbsource", "wez", {"expression": ["name", ".hg/updatestate"]}]'`
Augie Fackler - May 19, 2017, 9:59 p.m.
On Thu, May 18, 2017 at 02:22:03PM -0700, Wez Furlong wrote:
> # HG changeset patch
> # User Wez Furlong <wez@fb.com>
> # Date 1495136950 25200
> #      Thu May 18 12:49:10 2017 -0700
> # Node ID 726d02dfef6afc7030243cdbd43a4fc2ffb8be86
> # Parent  f4b76c106f3af915667db7696e27780601e93074
> fsmonitor: don't attempt state-leave if we didn't state-enter

queued, thanks

Patch

diff --git a/hgext/fsmonitor/__init__.py b/hgext/fsmonitor/__init__.py
--- a/hgext/fsmonitor/__init__.py
+++ b/hgext/fsmonitor/__init__.py
@@ -601,6 +601,7 @@ 
         self.distance = distance
         self.partial = partial
         self._lock = None
+        self.need_leave = False
 
     def __enter__(self):
         # We explicitly need to take a lock here, before we proceed to update
@@ -609,20 +610,21 @@ 
         # immediately anyway, so this is effectively extending the lock
         # around a couple of short sanity checks.
         self._lock = self.repo.wlock()
-        self._state('state-enter')
+        self.need_leave = self._state('state-enter')
         return self
 
     def __exit__(self, type_, value, tb):
         try:
-            status = 'ok' if type_ is None else 'failed'
-            self._state('state-leave', status=status)
+            if self.need_leave:
+                status = 'ok' if type_ is None else 'failed'
+                self._state('state-leave', status=status)
         finally:
             if self._lock:
                 self._lock.release()
 
     def _state(self, cmd, status='ok'):
         if not util.safehasattr(self.repo, '_watchmanclient'):
-            return
+            return False
         try:
             commithash = self.repo[self.node].hex()
             self.repo._watchmanclient.command(cmd, {
@@ -637,10 +639,12 @@ 
                     # whether the working copy parent is changing
                     'partial': self.partial,
             }})
+            return True
         except Exception as e:
             # Swallow any errors; fire and forget
             self.repo.ui.log(
                 'watchman', 'Exception %s while running %s\n', e, cmd)
+            return False
 
 # Bracket working copy updates with calls to the watchman state-enter
 # and state-leave commands.  This allows clients to perform more intelligent