Patchwork [1,of,4,V3] chgserver: implement validate command

login
register
mail settings
Submitter Jun Wu
Date March 5, 2016, 2 p.m.
Message ID <f9ac561d584323418775.1457186410@x1c>
Download mbox | patch
Permalink /patch/13614/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 5, 2016, 2 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1457186219 0
#      Sat Mar 05 13:56:59 2016 +0000
# Node ID f9ac561d5843234187751a508b6d557e8903ceac
# Parent  7cb2f2438f858d14af715682e9b888f8e3f2e475
chgserver: implement validate command

validate will load the repo config and check if the server has up-to-date
config to continue serve the client. In case it does not, the server will
send instructions to the client about what to do next, including to retry
with a different address or to unlink an outdated socket file to stop an
old server.
Yuya Nishihara - March 6, 2016, 10:26 a.m.
On Sat, 5 Mar 2016 14:00:10 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457186219 0
> #      Sat Mar 05 13:56:59 2016 +0000
> # Node ID f9ac561d5843234187751a508b6d557e8903ceac
> # Parent  7cb2f2438f858d14af715682e9b888f8e3f2e475
> chgserver: implement validate command

Queued this with a minor tweak, thanks.

> +    def validate(self):
> +        """reload the config and check if the server is up to date
> +
> +        read a list of '\0' separated arguments.
> +        write a non-empty list of '\0' separated instruction strings or '\0'
> +        if the list is empty.
> +        an instruction string could be either:
> +            - "unlink $path", the client should unlink the path to stop the
> +              outdated server.
> +            - "redirect $path", the client should try to connect to another
> +              server instead.
> +        """
> +        args = self._readlist()
> +        self.ui = _renewui(self.ui, args)
> +        newhash = hashstate.fromui(self.ui, self.hashstate.mtimepaths)
> +        insts = []
> +        if newhash.mtimehash != self.hashstate.mtimehash:
> +            addr = _hashaddress(self.baseaddress, self.hashstate.confighash)
> +            insts.append('unlink %s' % addr)
> +        if newhash.confighash != self.hashstate.confighash:
> +            addr = _hashaddress(self.baseaddress, newhash.confighash)
> +            insts.append('redirect %s' % addr)

I noticed the inconsistency between self.ui and self.hashstate.confighash,
but that's okay so long as the server process is forked per request. If the
server process is permanent, and if the client ignores the "redirect"
instruction and send "runcommand", new extension would be loaded.
Jun Wu - March 6, 2016, 12:38 p.m.
On 03/06/2016 10:26 AM, Yuya Nishihara wrote:
> I noticed the inconsistency between self.ui and self.hashstate.confighash,
> but that's okay so long as the server process is forked per request. If the
> server process is permanent, and if the client ignores the "redirect"
> instruction and send "runcommand", new extension would be loaded.

Right. The behavior relies on "fork". I don't think we can switch to
threads easily because SIGINT will be much harder to handle correctly.
Yuya Nishihara - March 6, 2016, 2 p.m.
On Sun, 6 Mar 2016 12:38:45 +0000, Jun Wu wrote:
> On 03/06/2016 10:26 AM, Yuya Nishihara wrote:
> > I noticed the inconsistency between self.ui and self.hashstate.confighash,
> > but that's okay so long as the server process is forked per request. If the
> > server process is permanent, and if the client ignores the "redirect"
> > instruction and send "runcommand", new extension would be loaded.
> 
> Right. The behavior relies on "fork". I don't think we can switch to
> threads easily because SIGINT will be much harder to handle correctly.

Threading won't be the way to go, but pre-forking will be doable if we can
get rid of globals.

Perhaps the problem can be avoided by discarding new ui if hash differs.

Patch

diff --git a/hgext/chgserver.py b/hgext/chgserver.py
--- a/hgext/chgserver.py
+++ b/hgext/chgserver.py
@@ -25,6 +25,9 @@ 
 'setumask' command
     set umask
 
+'validate' command
+    reload the config and check if the server is up to date
+
 'SIGHUP' signal
     reload configuration files
 
@@ -341,6 +344,9 @@ 
         self._oldios = []  # original (self.ch, ui.fp, fd) before "attachio"
         self.hashstate = hashstate
         self.baseaddress = baseaddress
+        if hashstate is not None:
+            self.capabilities = self.capabilities.copy()
+            self.capabilities['validate'] = chgcmdserver.validate
 
     def cleanup(self):
         # dispatch._runcatch() does not flush outputs if exception is not
@@ -387,6 +393,31 @@ 
 
         self.cresult.write(struct.pack('>i', len(clientfds)))
 
+    def validate(self):
+        """reload the config and check if the server is up to date
+
+        read a list of '\0' separated arguments.
+        write a non-empty list of '\0' separated instruction strings or '\0'
+        if the list is empty.
+        an instruction string could be either:
+            - "unlink $path", the client should unlink the path to stop the
+              outdated server.
+            - "redirect $path", the client should try to connect to another
+              server instead.
+        """
+        args = self._readlist()
+        self.ui = _renewui(self.ui, args)
+        newhash = hashstate.fromui(self.ui, self.hashstate.mtimepaths)
+        insts = []
+        if newhash.mtimehash != self.hashstate.mtimehash:
+            addr = _hashaddress(self.baseaddress, self.hashstate.confighash)
+            insts.append('unlink %s' % addr)
+        if newhash.confighash != self.hashstate.confighash:
+            addr = _hashaddress(self.baseaddress, newhash.confighash)
+            insts.append('redirect %s' % addr)
+        _log('validate: %s\n' % insts)
+        self.cresult.write('\0'.join(insts) or '\0')
+
     def _saveio(self):
         if self._oldios:
             return False