All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Re-implement prserv on top of asyncrpc
@ 2021-05-28  8:42 Paul Barker
  2021-05-28  8:42 ` [PATCH 1/4] asyncrpc: Add ping method Paul Barker
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Paul Barker @ 2021-05-28  8:42 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt; +Cc: Paul Barker

These changes replace the old XML-based RPC system in prserv with the
new asyncrpc implementation originally used by hashserv. A couple of
improvments are required in asyncrpc to support this.

I finally stumbled across the issue which led to the hanging builds
seen on the autobuilder when testing the initial RFC series.
It was a fairly dumb mistake on my behalf and I'm not sure how it
didn't trigger in my initial testing! The
`PRServerClient.handle_export()` function was missing a call to
`self.write_message()` so the client just ended up stuck waiting for a
response that was never to come. This issue is fixed here.

I've ran these changes through both `bitbake-selftest` and
`oe-selftest -a` and all looks good on my end. A couple of failures
were seen in oe-selftest but these are related to my host system
configuration (socat not installed, firewall blocking ports, etc) so
I'm fairly confident they aren't caused by this patch series.

Paul Barker (4):
  asyncrpc: Add ping method
  asyncrpc: Avoid duplicate sockets in TCP server
  asyncrpc: Reduce verbosity
  prserv: Replace XML RPC with modern asyncrpc implementation

 lib/bb/asyncrpc/client.py |   7 +-
 lib/bb/asyncrpc/serv.py   |  16 ++-
 lib/prserv/serv.py        | 271 ++++++++++++++++++++------------------
 3 files changed, 163 insertions(+), 131 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] asyncrpc: Add ping method
  2021-05-28  8:42 [PATCH 0/4] Re-implement prserv on top of asyncrpc Paul Barker
@ 2021-05-28  8:42 ` Paul Barker
  2021-05-28  8:42 ` [PATCH 2/4] asyncrpc: Avoid duplicate sockets in TCP server Paul Barker
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Paul Barker @ 2021-05-28  8:42 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt; +Cc: Paul Barker

This method is needed to support startup of the prservice. As it is so
generic we can add it to the common asyncrpc module.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 lib/bb/asyncrpc/client.py | 7 ++++++-
 lib/bb/asyncrpc/serv.py   | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py
index 4cdad9ac3..79919c5be 100644
--- a/lib/bb/asyncrpc/client.py
+++ b/lib/bb/asyncrpc/client.py
@@ -103,13 +103,18 @@ class AsyncClient(object):
 
         return await self._send_wrapper(proc)
 
+    async def ping(self):
+        return await self.send_message(
+            {'ping': {}}
+        )
+
 
 class Client(object):
     def __init__(self):
         self.client = self._get_async_client()
         self.loop = asyncio.new_event_loop()
 
-        self._add_methods('connect_tcp', 'close')
+        self._add_methods('connect_tcp', 'close', 'ping')
 
     @abc.abstractmethod
     def _get_async_client(self):
diff --git a/lib/bb/asyncrpc/serv.py b/lib/bb/asyncrpc/serv.py
index cb3384639..fd91aa71a 100644
--- a/lib/bb/asyncrpc/serv.py
+++ b/lib/bb/asyncrpc/serv.py
@@ -28,6 +28,7 @@ class AsyncServerConnection(object):
         self.max_chunk = DEFAULT_MAX_CHUNK
         self.handlers = {
             'chunk-stream': self.handle_chunk,
+            'ping': self.handle_ping,
         }
         self.logger = logger
 
@@ -123,6 +124,10 @@ class AsyncServerConnection(object):
 
         await self.dispatch_message(msg)
 
+    async def handle_ping(self, request):
+        response = {'alive': True}
+        self.write_message(response)
+
 
 class AsyncServer(object):
     def __init__(self, logger, loop=None):
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] asyncrpc: Avoid duplicate sockets in TCP server
  2021-05-28  8:42 [PATCH 0/4] Re-implement prserv on top of asyncrpc Paul Barker
  2021-05-28  8:42 ` [PATCH 1/4] asyncrpc: Add ping method Paul Barker
@ 2021-05-28  8:42 ` Paul Barker
  2021-06-02 14:38   ` Joshua Watt
  2021-05-28  8:42 ` [PATCH 3/4] asyncrpc: Reduce verbosity Paul Barker
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Paul Barker @ 2021-05-28  8:42 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt; +Cc: Paul Barker

Calling asyncio.start_server with host='localhost' results in two
sockets being opened with different port numbers (one for IPv4 and one
for IPv6):

    NOTE: Listening on ('127.0.0.1', 38833)
    NOTE: Listening on ('::1', 42029, 0, 0)

This is unnecessary and can be avoided by passing host='127.0.0.1'.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 lib/bb/asyncrpc/serv.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/bb/asyncrpc/serv.py b/lib/bb/asyncrpc/serv.py
index fd91aa71a..003a118b7 100644
--- a/lib/bb/asyncrpc/serv.py
+++ b/lib/bb/asyncrpc/serv.py
@@ -142,6 +142,9 @@ class AsyncServer(object):
         self.logger = logger
 
     def start_tcp_server(self, host, port):
+        if host == 'localhost':
+            host = '127.0.0.1'
+
         self.server = self.loop.run_until_complete(
             asyncio.start_server(self.handle_client, host, port, loop=self.loop)
         )
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] asyncrpc: Reduce verbosity
  2021-05-28  8:42 [PATCH 0/4] Re-implement prserv on top of asyncrpc Paul Barker
  2021-05-28  8:42 ` [PATCH 1/4] asyncrpc: Add ping method Paul Barker
  2021-05-28  8:42 ` [PATCH 2/4] asyncrpc: Avoid duplicate sockets in TCP server Paul Barker
@ 2021-05-28  8:42 ` Paul Barker
  2021-05-28  8:42 ` [PATCH 4/4] prserv: Replace XML RPC with modern asyncrpc implementation Paul Barker
  2021-05-31 11:25 ` [PATCH 0/4] Re-implement prserv on top of asyncrpc Richard Purdie
  4 siblings, 0 replies; 9+ messages in thread
From: Paul Barker @ 2021-05-28  8:42 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt; +Cc: Paul Barker

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 lib/bb/asyncrpc/serv.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/bb/asyncrpc/serv.py b/lib/bb/asyncrpc/serv.py
index 003a118b7..cfe23c84a 100644
--- a/lib/bb/asyncrpc/serv.py
+++ b/lib/bb/asyncrpc/serv.py
@@ -150,7 +150,7 @@ class AsyncServer(object):
         )
 
         for s in self.server.sockets:
-            self.logger.info('Listening on %r' % (s.getsockname(),))
+            self.logger.debug('Listening on %r' % (s.getsockname(),))
             # Newer python does this automatically. Do it manually here for
             # maximum compatibility
             s.setsockopt(socket.SOL_TCP, socket.TCP_NODELAY, 1)
@@ -176,7 +176,7 @@ class AsyncServer(object):
         finally:
             os.chdir(cwd)
 
-        self.logger.info('Listening on %r' % path)
+        self.logger.debug('Listening on %r' % path)
 
         self._cleanup_socket = cleanup
         self.address = "unix://%s" % os.path.abspath(path)
@@ -195,7 +195,7 @@ class AsyncServer(object):
             self.logger.error('Error from client: %s' % str(e), exc_info=True)
             traceback.print_exc()
             writer.close()
-        self.logger.info('Client disconnected')
+        self.logger.debug('Client disconnected')
 
     def run_loop_forever(self):
         try:
@@ -215,7 +215,7 @@ class AsyncServer(object):
             self.server.close()
 
             self.loop.run_until_complete(self.server.wait_closed())
-            self.logger.info('Server shutting down')
+            self.logger.debug('Server shutting down')
         finally:
             if self.close_loop:
                 if sys.version_info >= (3, 6):
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] prserv: Replace XML RPC with modern asyncrpc implementation
  2021-05-28  8:42 [PATCH 0/4] Re-implement prserv on top of asyncrpc Paul Barker
                   ` (2 preceding siblings ...)
  2021-05-28  8:42 ` [PATCH 3/4] asyncrpc: Reduce verbosity Paul Barker
@ 2021-05-28  8:42 ` Paul Barker
  2021-05-31 11:25 ` [PATCH 0/4] Re-implement prserv on top of asyncrpc Richard Purdie
  4 siblings, 0 replies; 9+ messages in thread
From: Paul Barker @ 2021-05-28  8:42 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt; +Cc: Paul Barker

Update the prserv client and server classes to use the modern json and
asyncio based RPC system implemented by the asyncrpc module.

Signed-off-by: Paul Barker <pbarker@konsulko.com>
---
 lib/prserv/serv.py | 271 ++++++++++++++++++++++++---------------------
 1 file changed, 145 insertions(+), 126 deletions(-)

diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 5e322bf83..7e16f355f 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -4,157 +4,170 @@
 
 import os,sys,logging
 import signal, time
-from xmlrpc.server import SimpleXMLRPCServer, SimpleXMLRPCRequestHandler
 import socket
 import io
 import sqlite3
-import bb.server.xmlrpcclient
 import prserv
 import prserv.db
 import errno
 import multiprocessing
+import bb.asyncrpc
 
 logger = logging.getLogger("BitBake.PRserv")
 
-class Handler(SimpleXMLRPCRequestHandler):
-    def _dispatch(self,method,params):
-        try:
-            value=self.server.funcs[method](*params)
-        except:
-            import traceback
-            traceback.print_exc()
-            raise
-        return value
-
 PIDPREFIX = "/tmp/PRServer_%s_%s.pid"
 singleton = None
 
+class PRServerClient(bb.asyncrpc.AsyncServerConnection):
+    def __init__(self, reader, writer, table):
+        super().__init__(reader, writer, 'PRSERVICE', logger)
+        self.handlers.update({
+            'get-pr': self.handle_get_pr,
+            'import-one': self.handle_import_one,
+            'export': self.handle_export,
+        })
+        self.table = table
 
-class PRServer(SimpleXMLRPCServer):
-    def __init__(self, dbfile, logfile, interface):
-        ''' constructor '''
-        try:
-            SimpleXMLRPCServer.__init__(self, interface,
-                                        logRequests=False, allow_none=True)
-        except socket.error:
-            ip=socket.gethostbyname(interface[0])
-            port=interface[1]
-            msg="PR Server unable to bind to %s:%s\n" % (ip, port)
-            sys.stderr.write(msg)
-            raise PRServiceConfigError
-
-        self.dbfile=dbfile
-        self.logfile=logfile
-        self.host, self.port = self.socket.getsockname()
-
-        self.register_function(self.getPR, "getPR")
-        self.register_function(self.ping, "ping")
-        self.register_function(self.export, "export")
-        self.register_function(self.importone, "importone")
-        self.register_introspection_functions()
-
-        self.iter_count = 0
-        # 60 iterations between syncs or sync if dirty every ~30 seconds
-        self.iterations_between_sync = 60
-
-    def sigint_handler(self, signum, stack):
-        if self.table:
-            self.table.sync()
-
-    def sigterm_handler(self, signum, stack):
-        if self.table:
-            self.table.sync()
-        raise(SystemExit)
+    def validate_proto_version(self):
+        return (self.proto_version == (1, 0))
 
-    def process_request(self, request, client_address):
-        if request is None:
-            return
+    async def dispatch_message(self, msg):
         try:
-            self.finish_request(request, client_address)
-            self.shutdown_request(request)
-            self.iter_count = (self.iter_count + 1) % self.iterations_between_sync
-            if self.iter_count == 0:
-                self.table.sync_if_dirty()
+            await super().dispatch_message(msg)
         except:
-            self.handle_error(request, client_address)
-            self.shutdown_request(request)
             self.table.sync()
+            raise
+
         self.table.sync_if_dirty()
 
-    def serve_forever(self, poll_interval=0.5):
-        signal.signal(signal.SIGINT, self.sigint_handler)
-        signal.signal(signal.SIGTERM, self.sigterm_handler)
+    async def handle_get_pr(self, request):
+        version = request['version']
+        pkgarch = request['pkgarch']
+        checksum = request['checksum']
 
-        self.db = prserv.db.PRData(self.dbfile)
-        self.table = self.db["PRMAIN"]
-        return super().serve_forever(poll_interval)
-
-    def export(self, version=None, pkgarch=None, checksum=None, colinfo=True):
+        response = None
         try:
-            return self.table.export(version, pkgarch, checksum, colinfo)
+            value = self.table.getValue(version, pkgarch, checksum)
+            response = {'value': value}
+        except prserv.NotFoundError:
+            logger.error("can not find value for (%s, %s)",version, checksum)
         except sqlite3.Error as exc:
             logger.error(str(exc))
-            return None
 
-    def importone(self, version, pkgarch, checksum, value):
-        return self.table.importone(version, pkgarch, checksum, value)
+        self.write_message(response)
 
-    def ping(self):
-        return True
+    async def handle_import_one(self, request):
+        version = request['version']
+        pkgarch = request['pkgarch']
+        checksum = request['checksum']
+        value = request['value']
+
+        value = self.table.importone(version, pkgarch, checksum, value)
+        if value is not None:
+            response = {'value': value}
+        else:
+            response = None
+        self.write_message(response)
+
+    async def handle_export(self, request):
+        version = request['version']
+        pkgarch = request['pkgarch']
+        checksum = request['checksum']
+        colinfo = request['colinfo']
+
+        try:
+            (metainfo, datainfo) = self.table.export(version, pkgarch, checksum, colinfo)
+        except sqlite3.Error as exc:
+            logger.error(str(exc))
+            metainfo = datainfo = None
 
-    def getinfo(self):
-        return (self.host, self.port)
+        response = {'metainfo': metainfo, 'datainfo': datainfo}
+        self.write_message(response)
 
     def getPR(self, version, pkgarch, checksum):
         try:
-            return self.table.getValue(version, pkgarch, checksum)
-        except prserv.NotFoundError:
-            logger.error("can not find value for (%s, %s)",version, checksum)
-            return None
+            (metainfo, datainfo) = self.table.export(version, pkgarch, checksum, colinfo)
         except sqlite3.Error as exc:
             logger.error(str(exc))
-            return None
+            metainfo = datainfo = None
 
-class PRServSingleton(object):
-    def __init__(self, dbfile, logfile, interface):
+        response = {'metainfo': metainfo, 'datainfo': datainfo}
+        self.write_message(response)
+
+class PRServer(bb.asyncrpc.AsyncServer):
+    def __init__(self, dbfile, loop=None):
+        super().__init__(logger, loop)
         self.dbfile = dbfile
-        self.logfile = logfile
-        self.interface = interface
-        self.host = None
-        self.port = None
+        self.table = None
 
-    def start(self):
-        self.prserv = PRServer(self.dbfile, self.logfile, self.interface)
-        self.process = multiprocessing.Process(target=self.prserv.serve_forever)
-        self.process.start()
+    def accept_client(self, reader, writer):
+        return PRServerClient(reader, writer, self.table)
 
-        self.host, self.port = self.prserv.getinfo()
+    def serve_forever(self):
+        self.db = prserv.db.PRData(self.dbfile)
+        self.table = self.db["PRMAIN"]
 
-    def getinfo(self):
-        return (self.host, self.port)
+        logger.debug("Started PRServer with DBfile: %s, Address: %s, PID: %s" %
+                     (self.dbfile, self.address, str(os.getpid())))
 
-class PRServerConnection(object):
-    def __init__(self, host, port):
-        if is_local_special(host, port):
-            host, port = singleton.getinfo()
-        self.host = host
-        self.port = port
-        self.connection, self.transport = bb.server.xmlrpcclient._create_server(self.host, self.port)
+        super().serve_forever()
 
-    def getPR(self, version, pkgarch, checksum):
-        return self.connection.getPR(version, pkgarch, checksum)
+        self.table.sync_if_dirty()
+        self.db.disconnect()
 
-    def ping(self):
-        return self.connection.ping()
+    def signal_handler(self):
+        super().signal_handler()
+        if self.table:
+            self.table.sync()
 
-    def export(self,version=None, pkgarch=None, checksum=None, colinfo=True):
-        return self.connection.export(version, pkgarch, checksum, colinfo)
+class PRServSingleton(object):
+    def __init__(self, dbfile, logfile, host, port):
+        self.dbfile = dbfile
+        self.logfile = logfile
+        self.host = host
+        self.port = port
 
-    def importone(self, version, pkgarch, checksum, value):
-        return self.connection.importone(version, pkgarch, checksum, value)
+    def start(self):
+        self.prserv = PRServer(self.dbfile)
+        self.prserv.start_tcp_server(self.host, self.port)
+        self.process = multiprocessing.Process(target=self.prserv.serve_forever)
+        self.process.start()
 
-    def getinfo(self):
-        return self.host, self.port
+        if not self.port:
+            self.port = int(self.prserv.address.rsplit(':', 1)[1])
+
+class PRAsyncClient(bb.asyncrpc.AsyncClient):
+    def __init__(self):
+        super().__init__('PRSERVICE', '1.0', logger)
+
+    async def getPR(self, version, pkgarch, checksum):
+        response = await self.send_message(
+            {'get-pr': {'version': version, 'pkgarch': pkgarch, 'checksum': checksum}}
+        )
+        if response:
+            return response['value']
+
+    async def importone(self, version, pkgarch, checksum, value):
+        response = await self.send_message(
+            {'import-one': {'version': version, 'pkgarch': pkgarch, 'checksum': checksum, 'value': value}}
+        )
+        if response:
+            return response['value']
+
+    async def export(self, version, pkgarch, checksum, colinfo):
+        response = await self.send_message(
+            {'export': {'version': version, 'pkgarch': pkgarch, 'checksum': checksum, 'colinfo': colinfo}}
+        )
+        if response:
+            return (response['metainfo'], response['datainfo'])
+
+class PRClient(bb.asyncrpc.Client):
+    def __init__(self):
+        super().__init__()
+        self._add_methods('getPR', 'importone', 'export')
+
+    def _get_async_client(self):
+        return PRAsyncClient()
 
 def run_as_daemon(func, pidfile, logfile):
     """
@@ -240,15 +253,13 @@ def start_daemon(dbfile, host, port, logfile):
                             % pidfile)
         return 1
 
-    server = PRServer(os.path.abspath(dbfile), os.path.abspath(logfile), (ip,port))
-    run_as_daemon(server.serve_forever, pidfile, os.path.abspath(logfile))
+    dbfile = os.path.abspath(dbfile)
+    def daemon_main():
+        server = PRServer(dbfile)
+        server.start_tcp_server(host, port)
+        server.serve_forever()
 
-    # Sometimes, the port (i.e. localhost:0) indicated by the user does not match with
-    # the one the server actually is listening, so at least warn the user about it
-    _,rport = server.getinfo()
-    if port != rport:
-        sys.stdout.write("Server is listening at port %s instead of %s\n"
-                         % (rport,port))
+    run_as_daemon(daemon_main, pidfile, os.path.abspath(logfile))
     return 0
 
 def stop_daemon(host, port):
@@ -302,7 +313,7 @@ def is_running(pid):
     return True
 
 def is_local_special(host, port):
-    if host.strip().upper() == 'localhost'.upper() and (not port):
+    if host.strip().lower() == 'localhost' and not port:
         return True
     else:
         return False
@@ -340,20 +351,19 @@ def auto_start(d):
                auto_shutdown()
         if not singleton:
             bb.utils.mkdirhier(cachedir)
-            singleton = PRServSingleton(os.path.abspath(dbfile), os.path.abspath(logfile), ("localhost",0))
+            singleton = PRServSingleton(os.path.abspath(dbfile), os.path.abspath(logfile), "localhost", 0)
             singleton.start()
     if singleton:
-        host, port = singleton.getinfo()
+        host = singleton.host
+        port = singleton.port
     else:
         host = host_params[0]
         port = int(host_params[1])
 
     try:
-        connection = PRServerConnection(host,port)
-        connection.ping()
-        realhost, realport = connection.getinfo()
-        return str(realhost) + ":" + str(realport)
-        
+        ping(host, port)
+        return str(host) + ":" + str(port)
+
     except Exception:
         logger.critical("PRservice %s:%d not available" % (host, port))
         raise PRServiceConfigError
@@ -366,8 +376,17 @@ def auto_shutdown():
         singleton = None
 
 def ping(host, port):
-    conn=PRServerConnection(host, port)
+    conn=PRClient()
+    conn.connect_tcp(host, port)
     return conn.ping()
 
 def connect(host, port):
-    return PRServerConnection(host, port)
+    global singleton
+
+    if host.strip().lower() == 'localhost' and not port:
+        host = 'localhost'
+        port = singleton.port
+
+    conn = PRClient()
+    conn.connect_tcp(host, port)
+    return conn
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] Re-implement prserv on top of asyncrpc
  2021-05-28  8:42 [PATCH 0/4] Re-implement prserv on top of asyncrpc Paul Barker
                   ` (3 preceding siblings ...)
  2021-05-28  8:42 ` [PATCH 4/4] prserv: Replace XML RPC with modern asyncrpc implementation Paul Barker
@ 2021-05-31 11:25 ` Richard Purdie
  2021-05-31 14:45   ` Paul Barker
  4 siblings, 1 reply; 9+ messages in thread
From: Richard Purdie @ 2021-05-31 11:25 UTC (permalink / raw)
  To: Paul Barker, bitbake-devel, Joshua Watt; +Cc: swat

Hi Paul,

On Fri, 2021-05-28 at 09:42 +0100, Paul Barker wrote:
> These changes replace the old XML-based RPC system in prserv with the
> new asyncrpc implementation originally used by hashserv. A couple of
> improvments are required in asyncrpc to support this.
> 
> I finally stumbled across the issue which led to the hanging builds
> seen on the autobuilder when testing the initial RFC series.
> It was a fairly dumb mistake on my behalf and I'm not sure how it
> didn't trigger in my initial testing! The
> `PRServerClient.handle_export()` function was missing a call to
> `self.write_message()` so the client just ended up stuck waiting for a
> response that was never to come. This issue is fixed here.
> 
> I've ran these changes through both `bitbake-selftest` and
> `oe-selftest -a` and all looks good on my end. A couple of failures
> were seen in oe-selftest but these are related to my host system
> configuration (socat not installed, firewall blocking ports, etc) so
> I'm fairly confident they aren't caused by this patch series.

Thanks for these. Unfortunately I think there is still a gremlin somewhere
as this was included in an autobuilder test build that is showing as this:

https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/2203

i.e. all four selftests have not finished and I'd have expected them to 
by now.

I'm trying not to work today so I haven't debugged them or confirmed where 
they are hanging but it seems likely related.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/4] Re-implement prserv on top of asyncrpc
  2021-05-31 11:25 ` [PATCH 0/4] Re-implement prserv on top of asyncrpc Richard Purdie
@ 2021-05-31 14:45   ` Paul Barker
  0 siblings, 0 replies; 9+ messages in thread
From: Paul Barker @ 2021-05-31 14:45 UTC (permalink / raw)
  To: Richard Purdie; +Cc: bitbake-devel, Joshua Watt, swat

On Mon, 31 May 2021 at 12:25, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> Hi Paul,
>
> On Fri, 2021-05-28 at 09:42 +0100, Paul Barker wrote:
> > These changes replace the old XML-based RPC system in prserv with the
> > new asyncrpc implementation originally used by hashserv. A couple of
> > improvments are required in asyncrpc to support this.
> >
> > I finally stumbled across the issue which led to the hanging builds
> > seen on the autobuilder when testing the initial RFC series.
> > It was a fairly dumb mistake on my behalf and I'm not sure how it
> > didn't trigger in my initial testing! The
> > `PRServerClient.handle_export()` function was missing a call to
> > `self.write_message()` so the client just ended up stuck waiting for a
> > response that was never to come. This issue is fixed here.
> >
> > I've ran these changes through both `bitbake-selftest` and
> > `oe-selftest -a` and all looks good on my end. A couple of failures
> > were seen in oe-selftest but these are related to my host system
> > configuration (socat not installed, firewall blocking ports, etc) so
> > I'm fairly confident they aren't caused by this patch series.
>
> Thanks for these. Unfortunately I think there is still a gremlin somewhere
> as this was included in an autobuilder test build that is showing as this:
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/2203
>
> i.e. all four selftests have not finished and I'd have expected them to
> by now.

(╯°□°)╯︵ ┻━┻

>
> I'm trying not to work today so I haven't debugged them or confirmed where
> they are hanging but it seems likely related.

If you're planning to take the day off don't worry about investigating
these. I'll take a look at the patches again on Wednesday. I think the
best approach may be to add some timeouts and maybe more error
handling to the asyncrpc code I extracted from hashserv - if we can
turn these hangs into a proper error then we can reduce the amount of
autobuilder time they take to test and hopefully we'll get a better
insight into what is actually going wrong. My guess is that there's
something in the autobuilder config or just the level of load on the
machines which is aggravating this as the tests finish successfully on
my build machine (with a few expected test failures as noted
previously).

Thanks,

-- 
Paul Barker
Konsulko Group

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] asyncrpc: Avoid duplicate sockets in TCP server
  2021-05-28  8:42 ` [PATCH 2/4] asyncrpc: Avoid duplicate sockets in TCP server Paul Barker
@ 2021-06-02 14:38   ` Joshua Watt
  2021-06-07 13:51     ` Paul Barker
  0 siblings, 1 reply; 9+ messages in thread
From: Joshua Watt @ 2021-06-02 14:38 UTC (permalink / raw)
  To: Paul Barker, bitbake-devel, Richard Purdie


On 5/28/21 3:42 AM, Paul Barker wrote:
> Calling asyncio.start_server with host='localhost' results in two
> sockets being opened with different port numbers (one for IPv4 and one
> for IPv6):
>
>      NOTE: Listening on ('127.0.0.1', 38833)
>      NOTE: Listening on ('::1', 42029, 0, 0)
>
> This is unnecessary and can be avoided by passing host='127.0.0.1'.

Is this causing some sort of bug? I think it would be better to do the 
expected default python behavior than try to work around it.

>
> Signed-off-by: Paul Barker <pbarker@konsulko.com>
> ---
>   lib/bb/asyncrpc/serv.py | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/lib/bb/asyncrpc/serv.py b/lib/bb/asyncrpc/serv.py
> index fd91aa71a..003a118b7 100644
> --- a/lib/bb/asyncrpc/serv.py
> +++ b/lib/bb/asyncrpc/serv.py
> @@ -142,6 +142,9 @@ class AsyncServer(object):
>           self.logger = logger
>   
>       def start_tcp_server(self, host, port):
> +        if host == 'localhost':
> +            host = '127.0.0.1'
> +
>           self.server = self.loop.run_until_complete(
>               asyncio.start_server(self.handle_client, host, port, loop=self.loop)
>           )

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] asyncrpc: Avoid duplicate sockets in TCP server
  2021-06-02 14:38   ` Joshua Watt
@ 2021-06-07 13:51     ` Paul Barker
  0 siblings, 0 replies; 9+ messages in thread
From: Paul Barker @ 2021-06-07 13:51 UTC (permalink / raw)
  To: Joshua Watt; +Cc: bitbake-devel, Richard Purdie

On Wed, 2 Jun 2021 at 15:38, Joshua Watt <jpewhacker@gmail.com> wrote:
>
>
> On 5/28/21 3:42 AM, Paul Barker wrote:
> > Calling asyncio.start_server with host='localhost' results in two
> > sockets being opened with different port numbers (one for IPv4 and one
> > for IPv6):
> >
> >      NOTE: Listening on ('127.0.0.1', 38833)
> >      NOTE: Listening on ('::1', 42029, 0, 0)
> >
> > This is unnecessary and can be avoided by passing host='127.0.0.1'.
>
> Is this causing some sort of bug? I think it would be better to do the
> expected default python behavior than try to work around it.

The default behaviour confuses me but I guess if it doesn't break
builds then this patch can be dropped.

Thanks,


--
Paul Barker
Konsulko Group

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-07 13:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-28  8:42 [PATCH 0/4] Re-implement prserv on top of asyncrpc Paul Barker
2021-05-28  8:42 ` [PATCH 1/4] asyncrpc: Add ping method Paul Barker
2021-05-28  8:42 ` [PATCH 2/4] asyncrpc: Avoid duplicate sockets in TCP server Paul Barker
2021-06-02 14:38   ` Joshua Watt
2021-06-07 13:51     ` Paul Barker
2021-05-28  8:42 ` [PATCH 3/4] asyncrpc: Reduce verbosity Paul Barker
2021-05-28  8:42 ` [PATCH 4/4] prserv: Replace XML RPC with modern asyncrpc implementation Paul Barker
2021-05-31 11:25 ` [PATCH 0/4] Re-implement prserv on top of asyncrpc Richard Purdie
2021-05-31 14:45   ` Paul Barker

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.