All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] Re-implement prserv on top of asyncrpc
@ 2021-08-19 16:46 Scott Murray
  2021-08-19 16:46 ` [PATCH v6 1/4] bitbake: asyncrpc: Defer all asyncio to child process Scott Murray
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Scott Murray @ 2021-08-19 16:46 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt, Paul Barker

These changes replace the old XML-based RPC system in prserv with the
new asyncrpc implementation originally used by hashserv, and add a
read-only mode to match the hash equivalency server's support.

Note that an additional change to the PR export support code in
oe-core is being sent to its mailing list.  That change is also
required for any serious testing, as it resolves another hang
issue in the PR export selftest.

Changes from v5:
  * Added new patch from Joshua Watt to shift asyncio loop creation
    into the new server processes.  This fixes one of the hangs seen
    on the autobuilder.

  * New patch to always create and set the asyncio loop as default
    in both the asyncrpc client and server initialization.  In my
    testing this resolves a couple of different hang issues seen in
    the selftests.

  * Task cleanup on shutdown and exception handler patches from Paul
    dropped from stack, as testing showed the hang issues were not
    helped by the changes.

  * Patch to rebase PR server on asyncrpc code tweaked to split the
    new client code into a separate source file as is done in hashserv.

Changes from v4:
  * Patch 2 updated to conditionalize all_tasks/current_task usage
    from asyncio vs asyncio.Task based on Python version.  This fixes
    running against Python 3.9 where they were finally removed from
    asyncio.Task.

Changes from v3:
  * Scott Murray taking over upstreaming effort from Paul Barker.

  * Dropped patches which are currently applied to master-next, this
    series should be applied on top of the current master-next branch.

  * Patches 2-4 updated by Scott Murray to rebase on top of 3983643
    ("bitbake: asyncrpc: Catch early SIGTERM").

  * Read-only PR server support patch added to stack to get it into
    the review process.

Joshua Watt (1):
  bitbake: asyncrpc: Defer all asyncio to child process

Paul Barker (2):
  prserv: Replace XML RPC with modern asyncrpc implementation
  prserv: Add read-only mode

Scott Murray (1):
  bitbake: asyncrpc: always create new asyncio loops

 bin/bitbake-prserv        |   4 +-
 lib/bb/asyncrpc/client.py |  10 ++
 lib/bb/asyncrpc/serv.py   | 134 +++++++++++++-------
 lib/hashserv/server.py    |   4 +-
 lib/prserv/client.py      |  48 ++++++++
 lib/prserv/db.py          |  65 +++++++---
 lib/prserv/serv.py        | 248 ++++++++++++++++++--------------------
 7 files changed, 319 insertions(+), 194 deletions(-)
 create mode 100644 lib/prserv/client.py

-- 
2.31.1


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

* [PATCH v6 1/4] bitbake: asyncrpc: Defer all asyncio to child process
  2021-08-19 16:46 [PATCH v6 0/4] Re-implement prserv on top of asyncrpc Scott Murray
@ 2021-08-19 16:46 ` Scott Murray
  2021-08-19 16:46 ` [PATCH v6 2/4] bitbake: asyncrpc: always create new asyncio loops Scott Murray
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Scott Murray @ 2021-08-19 16:46 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt, Paul Barker

From: Joshua Watt <JPEWhacker@gmail.com>

Reworks the async I/O API so that the async loop is only created in the
child process. This requires deferring the creation of the server until
the child process and a queue to transfer the bound address back to the
parent process

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
[small loop -> self.loop fix in serv.py]
Signed-off-by: Scott Murray <scott.murray@konsulko.com>
---
 lib/bb/asyncrpc/serv.py | 118 ++++++++++++++++++++++++----------------
 lib/hashserv/server.py  |   4 +-
 2 files changed, 74 insertions(+), 48 deletions(-)

diff --git a/lib/bb/asyncrpc/serv.py b/lib/bb/asyncrpc/serv.py
index 4084f300..45628698 100644
--- a/lib/bb/asyncrpc/serv.py
+++ b/lib/bb/asyncrpc/serv.py
@@ -131,53 +131,58 @@ class AsyncServerConnection(object):
 
 
 class AsyncServer(object):
-    def __init__(self, logger, loop=None):
-        if loop is None:
-            self.loop = asyncio.new_event_loop()
-            self.close_loop = True
-        else:
-            self.loop = loop
-            self.close_loop = False
-
+    def __init__(self, logger):
         self._cleanup_socket = None
         self.logger = logger
+        self.start = None
+        self.address = None
+
+    @property
+    def loop(self):
+        return asyncio.get_event_loop()
 
     def start_tcp_server(self, host, port):
-        self.server = self.loop.run_until_complete(
-            asyncio.start_server(self.handle_client, host, port, loop=self.loop)
-        )
-
-        for s in self.server.sockets:
-            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)
-            s.setsockopt(socket.SOL_TCP, socket.TCP_QUICKACK, 1)
-
-        name = self.server.sockets[0].getsockname()
-        if self.server.sockets[0].family == socket.AF_INET6:
-            self.address = "[%s]:%d" % (name[0], name[1])
-        else:
-            self.address = "%s:%d" % (name[0], name[1])
+        def start_tcp():
+            self.server = self.loop.run_until_complete(
+                asyncio.start_server(self.handle_client, host, port)
+            )
+
+            for s in self.server.sockets:
+                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)
+                s.setsockopt(socket.SOL_TCP, socket.TCP_QUICKACK, 1)
+
+            name = self.server.sockets[0].getsockname()
+            if self.server.sockets[0].family == socket.AF_INET6:
+                self.address = "[%s]:%d" % (name[0], name[1])
+            else:
+                self.address = "%s:%d" % (name[0], name[1])
+
+        self.start = start_tcp
 
     def start_unix_server(self, path):
         def cleanup():
             os.unlink(path)
 
-        cwd = os.getcwd()
-        try:
-            # Work around path length limits in AF_UNIX
-            os.chdir(os.path.dirname(path))
-            self.server = self.loop.run_until_complete(
-                asyncio.start_unix_server(self.handle_client, os.path.basename(path), loop=self.loop)
-            )
-        finally:
-            os.chdir(cwd)
+        def start_unix():
+            cwd = os.getcwd()
+            try:
+                # Work around path length limits in AF_UNIX
+                os.chdir(os.path.dirname(path))
+                self.server = self.loop.run_until_complete(
+                    asyncio.start_unix_server(self.handle_client, os.path.basename(path))
+                )
+            finally:
+                os.chdir(cwd)
 
-        self.logger.debug('Listening on %r' % path)
+            self.logger.debug('Listening on %r' % path)
 
-        self._cleanup_socket = cleanup
-        self.address = "unix://%s" % os.path.abspath(path)
+            self._cleanup_socket = cleanup
+            self.address = "unix://%s" % os.path.abspath(path)
+
+        self.start = start_unix
 
     @abc.abstractmethod
     def accept_client(self, reader, writer):
@@ -205,8 +210,7 @@ class AsyncServer(object):
         self.logger.debug("Got exit signal")
         self.loop.stop()
 
-    def serve_forever(self):
-        asyncio.set_event_loop(self.loop)
+    def _serve_forever(self):
         try:
             self.loop.add_signal_handler(signal.SIGTERM, self.signal_handler)
             signal.pthread_sigmask(signal.SIG_UNBLOCK, [signal.SIGTERM])
@@ -217,28 +221,50 @@ class AsyncServer(object):
             self.loop.run_until_complete(self.server.wait_closed())
             self.logger.debug('Server shutting down')
         finally:
-            if self.close_loop:
-                if sys.version_info >= (3, 6):
-                    self.loop.run_until_complete(self.loop.shutdown_asyncgens())
-                self.loop.close()
-
             if self._cleanup_socket is not None:
                 self._cleanup_socket()
 
+    def serve_forever(self):
+        """
+        Serve requests in the current process
+        """
+        self.start()
+        self._serve_forever()
+
     def serve_as_process(self, *, prefunc=None, args=()):
-        def run():
+        """
+        Serve requests in a child process
+        """
+        def run(queue):
+            try:
+                self.start()
+            finally:
+                queue.put(self.address)
+                queue.close()
+
             if prefunc is not None:
                 prefunc(self, *args)
-            self.serve_forever()
+
+            self._serve_forever()
+
+            if sys.version_info >= (3, 6):
+                self.loop.run_until_complete(self.loop.shutdown_asyncgens())
+            self.loop.close()
+
+        queue = multiprocessing.Queue()
 
         # Temporarily block SIGTERM. The server process will inherit this
         # block which will ensure it doesn't receive the SIGTERM until the
         # handler is ready for it
         mask = signal.pthread_sigmask(signal.SIG_BLOCK, [signal.SIGTERM])
         try:
-            self.process = multiprocessing.Process(target=run)
+            self.process = multiprocessing.Process(target=run, args=(queue,))
             self.process.start()
 
+            self.address = queue.get()
+            queue.close()
+            queue.join_thread()
+
             return self.process
         finally:
             signal.pthread_sigmask(signal.SIG_SETMASK, mask)
diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
index 8e849897..a059e521 100644
--- a/lib/hashserv/server.py
+++ b/lib/hashserv/server.py
@@ -410,11 +410,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
 
 
 class Server(bb.asyncrpc.AsyncServer):
-    def __init__(self, db, loop=None, upstream=None, read_only=False):
+    def __init__(self, db, upstream=None, read_only=False):
         if upstream and read_only:
             raise bb.asyncrpc.ServerError("Read-only hashserv cannot pull from an upstream server")
 
-        super().__init__(logger, loop)
+        super().__init__(logger)
 
         self.request_stats = Stats()
         self.db = db
-- 
2.31.1


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

* [PATCH v6 2/4] bitbake: asyncrpc: always create new asyncio loops
  2021-08-19 16:46 [PATCH v6 0/4] Re-implement prserv on top of asyncrpc Scott Murray
  2021-08-19 16:46 ` [PATCH v6 1/4] bitbake: asyncrpc: Defer all asyncio to child process Scott Murray
@ 2021-08-19 16:46 ` Scott Murray
  2021-08-19 19:58   ` Joshua Watt
  2021-08-19 16:46 ` [PATCH v6 3/4] prserv: Replace XML RPC with modern asyncrpc implementation Scott Murray
  2021-08-19 16:46 ` [PATCH v6 4/4] prserv: Add read-only mode Scott Murray
  3 siblings, 1 reply; 8+ messages in thread
From: Scott Murray @ 2021-08-19 16:46 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt, Paul Barker

asyncio in older Python 3.x (seen with 3.7) can seemingly hang if
new_event_loop is called repeatedly in the same process.  The
reuse of processes in the Bitbake thread pool during parsing seems
to be able to trigger this with the PR server export selftest.
It appears that calling set_event_loop with the new loop avoids the
issue, so that is now done in the asyncrpc Client initializer (with
an explanatory comment).  This should be revisited when the day
arrives that Python 3.9 becomes the minimum required for BitBake.

Additionally, it was discovered that using get_event_loop in the
asyncrpc server initialization can trigger hangs in the hashserv
unittests when the second test is run.  To avoid this, switch to
calling new_event_loop + set_event_loop in the initialization code
there as well.

Signed-off-by: Scott Murray <scott.murray@konsulko.com>
---
 lib/bb/asyncrpc/client.py | 10 ++++++++++
 lib/bb/asyncrpc/serv.py   | 24 ++++++++++++++++++++----
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py
index 3eb4fdde..50e60d5c 100644
--- a/lib/bb/asyncrpc/client.py
+++ b/lib/bb/asyncrpc/client.py
@@ -119,6 +119,16 @@ class Client(object):
         self.client = self._get_async_client()
         self.loop = asyncio.new_event_loop()
 
+        # Override any pre-existing loop.
+        # Without this, the PR server export selftest triggers a hang
+        # when running with Python 3.7.  The drawback is that there is
+        # potential for issues if the PR and hash equiv (or some new)
+        # clients need to both be instantiated in the same process.
+        # This should be revisited if/when Python 3.9 becomes the
+        # minimum required version for BitBake, as it seems not
+        # required (but harmless) with it.
+        asyncio.set_event_loop(self.loop)
+
         self._add_methods('connect_tcp', 'close', 'ping')
 
     @abc.abstractmethod
diff --git a/lib/bb/asyncrpc/serv.py b/lib/bb/asyncrpc/serv.py
index 45628698..b4cffff2 100644
--- a/lib/bb/asyncrpc/serv.py
+++ b/lib/bb/asyncrpc/serv.py
@@ -136,10 +136,7 @@ class AsyncServer(object):
         self.logger = logger
         self.start = None
         self.address = None
-
-    @property
-    def loop(self):
-        return asyncio.get_event_loop()
+        self.loop = None
 
     def start_tcp_server(self, host, port):
         def start_tcp():
@@ -228,6 +225,12 @@ class AsyncServer(object):
         """
         Serve requests in the current process
         """
+        # Create loop and override any loop that may have existed in
+        # a parent process.  It is possible that the usecases of
+        # serve_forever might be constrained enough to allow using
+        # get_event_loop here, but better safe than sorry for now.
+        self.loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(self.loop)
         self.start()
         self._serve_forever()
 
@@ -236,6 +239,19 @@ class AsyncServer(object):
         Serve requests in a child process
         """
         def run(queue):
+            # Create loop and override any loop that may have existed
+            # in a parent process.  Without doing this and instead
+            # using get_event_loop, at the very minimum the hashserv
+            # unit tests will hang when running the second test.
+            # This happens since get_event_loop in the spawned server
+            # process for the second testcase ends up with the loop
+            # from the hashserv client created in the unit test process
+            # when running the first testcase.  The problem is somewhat
+            # more general, though, as any potential use of asyncio in
+            # Cooker could create a loop that needs to replaced in this
+            # new process.
+            self.loop = asyncio.new_event_loop()
+            asyncio.set_event_loop(self.loop)
             try:
                 self.start()
             finally:
-- 
2.31.1


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

* [PATCH v6 3/4] prserv: Replace XML RPC with modern asyncrpc implementation
  2021-08-19 16:46 [PATCH v6 0/4] Re-implement prserv on top of asyncrpc Scott Murray
  2021-08-19 16:46 ` [PATCH v6 1/4] bitbake: asyncrpc: Defer all asyncio to child process Scott Murray
  2021-08-19 16:46 ` [PATCH v6 2/4] bitbake: asyncrpc: always create new asyncio loops Scott Murray
@ 2021-08-19 16:46 ` Scott Murray
  2021-08-26 16:02   ` [bitbake-devel] " Martin Jansa
  2021-08-19 16:46 ` [PATCH v6 4/4] prserv: Add read-only mode Scott Murray
  3 siblings, 1 reply; 8+ messages in thread
From: Scott Murray @ 2021-08-19 16:46 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt, Paul Barker

From: Paul Barker <pbarker@konsulko.com>

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>
[updated for asyncrpc changes, client split to separate file]
Signed-off-by: Scott Murray <scott.murray@konsulko.com>
---
 lib/prserv/client.py |  41 ++++++++
 lib/prserv/serv.py   | 234 ++++++++++++++++++++-----------------------
 2 files changed, 147 insertions(+), 128 deletions(-)
 create mode 100644 lib/prserv/client.py

diff --git a/lib/prserv/client.py b/lib/prserv/client.py
new file mode 100644
index 00000000..285dce72
--- /dev/null
+++ b/lib/prserv/client.py
@@ -0,0 +1,41 @@
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+
+import logging
+import bb.asyncrpc
+
+logger = logging.getLogger("BitBake.PRserv")
+
+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()
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 5e322bf8..1fa4e176 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -4,157 +4,125 @@
 
 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()
+    def validate_proto_version(self):
+        return (self.proto_version == (1, 0))
 
-        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 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()
-        self.table.sync_if_dirty()
+            raise
 
-    def serve_forever(self, poll_interval=0.5):
-        signal.signal(signal.SIGINT, self.sigint_handler)
-        signal.signal(signal.SIGTERM, self.sigterm_handler)
+        self.table.sync_if_dirty()
 
-        self.db = prserv.db.PRData(self.dbfile)
-        self.table = self.db["PRMAIN"]
-        return super().serve_forever(poll_interval)
+    async def handle_get_pr(self, request):
+        version = request['version']
+        pkgarch = request['pkgarch']
+        checksum = request['checksum']
 
-    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)
 
-    def getinfo(self):
-        return (self.host, self.port)
+    async def handle_export(self, request):
+        version = request['version']
+        pkgarch = request['pkgarch']
+        checksum = request['checksum']
+        colinfo = request['colinfo']
 
-    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):
+        super().__init__(logger)
         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 = self.prserv.serve_as_process()
 
-    def getinfo(self):
-        return self.host, self.port
+        if not self.port:
+            self.port = int(self.prserv.address.rsplit(':', 1)[1])
 
 def run_as_daemon(func, pidfile, logfile):
     """
@@ -240,15 +208,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 +268,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 +306,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 +331,21 @@ def auto_shutdown():
         singleton = None
 
 def ping(host, port):
-    conn=PRServerConnection(host, port)
+    from . import client
+
+    conn = client.PRClient()
+    conn.connect_tcp(host, port)
     return conn.ping()
 
 def connect(host, port):
-    return PRServerConnection(host, port)
+    from . import client
+
+    global singleton
+
+    if host.strip().lower() == 'localhost' and not port:
+        host = 'localhost'
+        port = singleton.port
+
+    conn = client.PRClient()
+    conn.connect_tcp(host, port)
+    return conn
-- 
2.31.1


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

* [PATCH v6 4/4] prserv: Add read-only mode
  2021-08-19 16:46 [PATCH v6 0/4] Re-implement prserv on top of asyncrpc Scott Murray
                   ` (2 preceding siblings ...)
  2021-08-19 16:46 ` [PATCH v6 3/4] prserv: Replace XML RPC with modern asyncrpc implementation Scott Murray
@ 2021-08-19 16:46 ` Scott Murray
  3 siblings, 0 replies; 8+ messages in thread
From: Scott Murray @ 2021-08-19 16:46 UTC (permalink / raw)
  To: bitbake-devel, Richard Purdie, Joshua Watt, Paul Barker

From: Paul Barker <pbarker@konsulko.com>

[YOCTO #13659]

Signed-off-by: Paul Barker <pbarker@konsulko.com>
[updated for asyncrpc changes]
Signed-off-by: Scott Murray <scott.murray@konsulko.com>
---
 bin/bitbake-prserv   |  4 ++-
 lib/prserv/client.py |  9 +++++-
 lib/prserv/db.py     | 65 ++++++++++++++++++++++++++++++++++----------
 lib/prserv/serv.py   | 40 ++++++++++++++++-----------
 4 files changed, 86 insertions(+), 32 deletions(-)

diff --git a/bin/bitbake-prserv b/bin/bitbake-prserv
index 1e9b6cbc..bef5ef68 100755
--- a/bin/bitbake-prserv
+++ b/bin/bitbake-prserv
@@ -36,12 +36,14 @@ def main():
                       dest="host", type="string", default=PRHOST_DEFAULT)
     parser.add_option("--port", help="port number(default: 8585)", action="store",
                       dest="port", type="int", default=PRPORT_DEFAULT)
+    parser.add_option("-r", "--read-only", help="open database in read-only mode",
+                      action="store_true")
 
     options, args = parser.parse_args(sys.argv)
     prserv.init_logger(os.path.abspath(options.logfile),options.loglevel)
 
     if options.start:
-        ret=prserv.serv.start_daemon(options.dbfile, options.host, options.port,os.path.abspath(options.logfile))
+        ret=prserv.serv.start_daemon(options.dbfile, options.host, options.port,os.path.abspath(options.logfile), options.read_only)
     elif options.stop:
         ret=prserv.serv.stop_daemon(options.host, options.port)
     else:
diff --git a/lib/prserv/client.py b/lib/prserv/client.py
index 285dce72..a3f19dda 100644
--- a/lib/prserv/client.py
+++ b/lib/prserv/client.py
@@ -32,10 +32,17 @@ class PRAsyncClient(bb.asyncrpc.AsyncClient):
         if response:
             return (response['metainfo'], response['datainfo'])
 
+    async def is_readonly(self):
+        response = await self.send_message(
+            {'is-readonly': {}}
+        )
+        if response:
+            return response['readonly']
+
 class PRClient(bb.asyncrpc.Client):
     def __init__(self):
         super().__init__()
-        self._add_methods('getPR', 'importone', 'export')
+        self._add_methods('getPR', 'importone', 'export', 'is_readonly')
 
     def _get_async_client(self):
         return PRAsyncClient()
diff --git a/lib/prserv/db.py b/lib/prserv/db.py
index cb2a2461..2710d4a2 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -30,21 +30,29 @@ if sqlversion[0] < 3 or (sqlversion[0] == 3 and sqlversion[1] < 3):
 #
 
 class PRTable(object):
-    def __init__(self, conn, table, nohist):
+    def __init__(self, conn, table, nohist, read_only):
         self.conn = conn
         self.nohist = nohist
+        self.read_only = read_only
         self.dirty = False
         if nohist:
             self.table = "%s_nohist" % table 
         else:
             self.table = "%s_hist" % table 
 
-        self._execute("CREATE TABLE IF NOT EXISTS %s \
-                    (version TEXT NOT NULL, \
-                    pkgarch TEXT NOT NULL,  \
-                    checksum TEXT NOT NULL, \
-                    value INTEGER, \
-                    PRIMARY KEY (version, pkgarch, checksum));" % self.table)
+        if self.read_only:
+            table_exists = self._execute(
+                        "SELECT count(*) FROM sqlite_master \
+                        WHERE type='table' AND name='%s'" % (self.table))
+            if not table_exists:
+                raise prserv.NotFoundError
+        else:
+            self._execute("CREATE TABLE IF NOT EXISTS %s \
+                        (version TEXT NOT NULL, \
+                        pkgarch TEXT NOT NULL,  \
+                        checksum TEXT NOT NULL, \
+                        value INTEGER, \
+                        PRIMARY KEY (version, pkgarch, checksum));" % self.table)
 
     def _execute(self, *query):
         """Execute a query, waiting to acquire a lock if necessary"""
@@ -59,8 +67,9 @@ class PRTable(object):
                 raise exc
 
     def sync(self):
-        self.conn.commit()
-        self._execute("BEGIN EXCLUSIVE TRANSACTION")
+        if not self.read_only:
+            self.conn.commit()
+            self._execute("BEGIN EXCLUSIVE TRANSACTION")
 
     def sync_if_dirty(self):
         if self.dirty:
@@ -75,6 +84,15 @@ class PRTable(object):
             return row[0]
         else:
             #no value found, try to insert
+            if self.read_only:
+                data = self._execute("SELECT ifnull(max(value)+1,0) FROM %s where version=? AND pkgarch=?;" % (self.table),
+                                   (version, pkgarch))
+                row = data.fetchone()
+                if row is not None:
+                    return row[0]
+                else:
+                    return 0
+
             try:
                 self._execute("INSERT INTO %s VALUES (?, ?, ?, (select ifnull(max(value)+1,0) from %s where version=? AND pkgarch=?));"
                            % (self.table,self.table),
@@ -103,6 +121,15 @@ class PRTable(object):
             return row[0]
         else:
             #no value found, try to insert
+            if self.read_only:
+                data = self._execute("SELECT ifnull(max(value)+1,0) FROM %s where version=? AND pkgarch=?;" % (self.table),
+                                   (version, pkgarch))
+                row = data.fetchone()
+                if row is not None:
+                    return row[0]
+                else:
+                    return 0
+
             try:
                 self._execute("INSERT OR REPLACE INTO %s VALUES (?, ?, ?, (select ifnull(max(value)+1,0) from %s where version=? AND pkgarch=?));"
                                % (self.table,self.table),
@@ -128,6 +155,9 @@ class PRTable(object):
             return self._getValueHist(version, pkgarch, checksum)
 
     def _importHist(self, version, pkgarch, checksum, value):
+        if self.read_only:
+            return None
+
         val = None 
         data = self._execute("SELECT value FROM %s WHERE version=? AND pkgarch=? AND checksum=?;" % self.table,
                            (version, pkgarch, checksum))
@@ -152,6 +182,9 @@ class PRTable(object):
         return val
 
     def _importNohist(self, version, pkgarch, checksum, value):
+        if self.read_only:
+            return None
+
         try:
             #try to insert
             self._execute("INSERT INTO %s VALUES (?, ?, ?, ?);"  % (self.table),
@@ -245,19 +278,23 @@ class PRTable(object):
 
 class PRData(object):
     """Object representing the PR database"""
-    def __init__(self, filename, nohist=True):
+    def __init__(self, filename, nohist=True, read_only=False):
         self.filename=os.path.abspath(filename)
         self.nohist=nohist
+        self.read_only = read_only
         #build directory hierarchy
         try:
             os.makedirs(os.path.dirname(self.filename))
         except OSError as e:
             if e.errno != errno.EEXIST:
                 raise e
-        self.connection=sqlite3.connect(self.filename, isolation_level="EXCLUSIVE", check_same_thread = False)
+        uri = "file:%s%s" % (self.filename, "?mode=ro" if self.read_only else "")
+        logger.debug("Opening PRServ database '%s'" % (uri))
+        self.connection=sqlite3.connect(uri, uri=True, isolation_level="EXCLUSIVE", check_same_thread = False)
         self.connection.row_factory=sqlite3.Row
-        self.connection.execute("pragma synchronous = off;")
-        self.connection.execute("PRAGMA journal_mode = MEMORY;")
+        if not self.read_only:
+            self.connection.execute("pragma synchronous = off;")
+            self.connection.execute("PRAGMA journal_mode = MEMORY;")
         self._tables={}
 
     def disconnect(self):
@@ -270,7 +307,7 @@ class PRData(object):
         if tblname in self._tables:
             return self._tables[tblname]
         else:
-            tableobj = self._tables[tblname] = PRTable(self.connection, tblname, self.nohist)
+            tableobj = self._tables[tblname] = PRTable(self.connection, tblname, self.nohist, self.read_only)
             return tableobj
 
     def __delitem__(self, tblname):
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 1fa4e176..17ae4096 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -18,14 +18,16 @@ PIDPREFIX = "/tmp/PRServer_%s_%s.pid"
 singleton = None
 
 class PRServerClient(bb.asyncrpc.AsyncServerConnection):
-    def __init__(self, reader, writer, table):
+    def __init__(self, reader, writer, table, read_only):
         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,
+            'is-readonly': self.handle_is_readonly,
         })
         self.table = table
+        self.read_only = read_only
 
     def validate_proto_version(self):
         return (self.proto_version == (1, 0))
@@ -56,16 +58,17 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         self.write_message(response)
 
     async def handle_import_one(self, request):
-        version = request['version']
-        pkgarch = request['pkgarch']
-        checksum = request['checksum']
-        value = request['value']
+        response = None
+        if not self.read_only:
+            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}
 
-        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):
@@ -83,20 +86,25 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         response = {'metainfo': metainfo, 'datainfo': datainfo}
         self.write_message(response)
 
+    async def handle_is_readonly(self, request):
+        response = {'readonly': self.read_only}
+        self.write_message(response)
+
 class PRServer(bb.asyncrpc.AsyncServer):
-    def __init__(self, dbfile):
+    def __init__(self, dbfile, read_only=False):
         super().__init__(logger)
         self.dbfile = dbfile
         self.table = None
+        self.read_only = read_only
 
     def accept_client(self, reader, writer):
-        return PRServerClient(reader, writer, self.table)
+        return PRServerClient(reader, writer, self.table, self.read_only)
 
     def _serve_forever(self):
-        self.db = prserv.db.PRData(self.dbfile)
+        self.db = prserv.db.PRData(self.dbfile, read_only=self.read_only)
         self.table = self.db["PRMAIN"]
 
-        logger.debug("Started PRServer with DBfile: %s, Address: %s, PID: %s" %
+        logger.info("Started PRServer with DBfile: %s, Address: %s, PID: %s" %
                      (self.dbfile, self.address, str(os.getpid())))
 
         super()._serve_forever()
@@ -194,7 +202,7 @@ def run_as_daemon(func, pidfile, logfile):
     os.remove(pidfile)
     os._exit(0)
 
-def start_daemon(dbfile, host, port, logfile):
+def start_daemon(dbfile, host, port, logfile, read_only=False):
     ip = socket.gethostbyname(host)
     pidfile = PIDPREFIX % (ip, port)
     try:
@@ -210,7 +218,7 @@ def start_daemon(dbfile, host, port, logfile):
 
     dbfile = os.path.abspath(dbfile)
     def daemon_main():
-        server = PRServer(dbfile)
+        server = PRServer(dbfile, read_only=read_only)
         server.start_tcp_server(host, port)
         server.serve_forever()
 
-- 
2.31.1


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

* Re: [PATCH v6 2/4] bitbake: asyncrpc: always create new asyncio loops
  2021-08-19 16:46 ` [PATCH v6 2/4] bitbake: asyncrpc: always create new asyncio loops Scott Murray
@ 2021-08-19 19:58   ` Joshua Watt
  0 siblings, 0 replies; 8+ messages in thread
From: Joshua Watt @ 2021-08-19 19:58 UTC (permalink / raw)
  To: Scott Murray, bitbake-devel, Richard Purdie, Paul Barker


On 8/19/21 11:46 AM, Scott Murray wrote:
> asyncio in older Python 3.x (seen with 3.7) can seemingly hang if
> new_event_loop is called repeatedly in the same process.  The
> reuse of processes in the Bitbake thread pool during parsing seems
> to be able to trigger this with the PR server export selftest.
> It appears that calling set_event_loop with the new loop avoids the
> issue, so that is now done in the asyncrpc Client initializer (with
> an explanatory comment).  This should be revisited when the day
> arrives that Python 3.9 becomes the minimum required for BitBake.
>
> Additionally, it was discovered that using get_event_loop in the
> asyncrpc server initialization can trigger hangs in the hashserv
> unittests when the second test is run.  To avoid this, switch to
> calling new_event_loop + set_event_loop in the initialization code
> there as well.
>
> Signed-off-by: Scott Murray <scott.murray@konsulko.com>

Looks good to me

Reviewed-by: Joshua Watt <JPEWhacker@gmail.com>


> ---
>   lib/bb/asyncrpc/client.py | 10 ++++++++++
>   lib/bb/asyncrpc/serv.py   | 24 ++++++++++++++++++++----
>   2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py
> index 3eb4fdde..50e60d5c 100644
> --- a/lib/bb/asyncrpc/client.py
> +++ b/lib/bb/asyncrpc/client.py
> @@ -119,6 +119,16 @@ class Client(object):
>           self.client = self._get_async_client()
>           self.loop = asyncio.new_event_loop()
>   
> +        # Override any pre-existing loop.
> +        # Without this, the PR server export selftest triggers a hang
> +        # when running with Python 3.7.  The drawback is that there is
> +        # potential for issues if the PR and hash equiv (or some new)
> +        # clients need to both be instantiated in the same process.
> +        # This should be revisited if/when Python 3.9 becomes the
> +        # minimum required version for BitBake, as it seems not
> +        # required (but harmless) with it.
> +        asyncio.set_event_loop(self.loop)
> +
>           self._add_methods('connect_tcp', 'close', 'ping')
>   
>       @abc.abstractmethod
> diff --git a/lib/bb/asyncrpc/serv.py b/lib/bb/asyncrpc/serv.py
> index 45628698..b4cffff2 100644
> --- a/lib/bb/asyncrpc/serv.py
> +++ b/lib/bb/asyncrpc/serv.py
> @@ -136,10 +136,7 @@ class AsyncServer(object):
>           self.logger = logger
>           self.start = None
>           self.address = None
> -
> -    @property
> -    def loop(self):
> -        return asyncio.get_event_loop()
> +        self.loop = None
>   
>       def start_tcp_server(self, host, port):
>           def start_tcp():
> @@ -228,6 +225,12 @@ class AsyncServer(object):
>           """
>           Serve requests in the current process
>           """
> +        # Create loop and override any loop that may have existed in
> +        # a parent process.  It is possible that the usecases of
> +        # serve_forever might be constrained enough to allow using
> +        # get_event_loop here, but better safe than sorry for now.
> +        self.loop = asyncio.new_event_loop()
> +        asyncio.set_event_loop(self.loop)
>           self.start()
>           self._serve_forever()
>   
> @@ -236,6 +239,19 @@ class AsyncServer(object):
>           Serve requests in a child process
>           """
>           def run(queue):
> +            # Create loop and override any loop that may have existed
> +            # in a parent process.  Without doing this and instead
> +            # using get_event_loop, at the very minimum the hashserv
> +            # unit tests will hang when running the second test.
> +            # This happens since get_event_loop in the spawned server
> +            # process for the second testcase ends up with the loop
> +            # from the hashserv client created in the unit test process
> +            # when running the first testcase.  The problem is somewhat
> +            # more general, though, as any potential use of asyncio in
> +            # Cooker could create a loop that needs to replaced in this
> +            # new process.
> +            self.loop = asyncio.new_event_loop()
> +            asyncio.set_event_loop(self.loop)
>               try:
>                   self.start()
>               finally:

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

* Re: [bitbake-devel] [PATCH v6 3/4] prserv: Replace XML RPC with modern asyncrpc implementation
  2021-08-19 16:46 ` [PATCH v6 3/4] prserv: Replace XML RPC with modern asyncrpc implementation Scott Murray
@ 2021-08-26 16:02   ` Martin Jansa
  2021-08-26 19:17     ` Scott Murray
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Jansa @ 2021-08-26 16:02 UTC (permalink / raw)
  To: Scott Murray; +Cc: bitbake-devel, Richard Purdie, Joshua Watt, Paul Barker

[-- Attachment #1: Type: text/plain, Size: 4152 bytes --]

On Thu, Aug 19, 2021 at 6:47 PM Scott Murray <scott.murray@konsulko.com>
wrote:

> From: Paul Barker <pbarker@konsulko.com>
>
> 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>
> [updated for asyncrpc changes, client split to separate file]
> Signed-off-by: Scott Murray <scott.murray@konsulko.com>
>
> ....


> -    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 = self.prserv.serve_as_process()
>
> -    def getinfo(self):
> -        return self.host, self.port
> +        if not self.port:
> +            self.port = int(self.prserv.address.rsplit(':', 1)[1])
>
>
Hi,

I've tried this when running inside docker container and it failed with a
bit ugly exception:

bitbake@599696cd20aa:~/nodistro/honister$ bitbake -k pkgconfig-native
Traceback (most recent call last):
  File "/OE/nodistro/honister/bitbake/bin/bitbake", line 35, in <module>
    sys.exit(bitbake_main(BitBakeConfigParameters(sys.argv),
  File "/OE/nodistro/honister/bitbake/lib/bb/main.py", line 385, in
bitbake_main
    return ui_module.main(server_connection.connection,
server_connection.events,
  File "/OE/nodistro/honister/bitbake/lib/bb/ui/knotty.py", line 397, in
main
    params.updateToServer(server, os.environ.copy())
  File "/OE/nodistro/honister/bitbake/lib/bb/cookerdata.py", line 75, in
updateToServer
    raise Exception("Unable to update the server configuration with local
parameters: %s" % error)
Exception: Unable to update the server configuration with local parameters:
Traceback (most recent call last):
  File "/OE/nodistro/honister/bitbake/lib/bb/command.py", line 90, in
runCommand
    result = command_method(self, commandline)
  File "/OE/nodistro/honister/bitbake/lib/bb/command.py", line 286, in
updateConfig
    command.cooker.updateConfigOpts(options, environment, cmdline)
  File "/OE/nodistro/honister/bitbake/lib/bb/cooker.py", line 491, in
updateConfigOpts
    self.reset()
  File "/OE/nodistro/honister/bitbake/lib/bb/cooker.py", line 1717, in reset
    self.handlePRServ()
  File "/OE/nodistro/honister/bitbake/lib/bb/cooker.py", line 383, in
handlePRServ
    self.prhost = prserv.serv.auto_start(self.data)
  File "/OE/nodistro/honister/bitbake/lib/prserv/serv.py", line 318, in
auto_start
    singleton.start()
  File "/OE/nodistro/honister/bitbake/lib/prserv/serv.py", line 133, in
start
    self.port = int(self.prserv.address.rsplit(':', 1)[1])
AttributeError: 'NoneType' object has no attribute 'rsplit'

after removing the rsplit it shows better error message:

bitbake@599696cd20aa:~/nodistro/honister$ bitbake -k pkgconfig-native
WARNING: Error talking to server: Multiple exceptions: [Errno 111] Connect
call failed ('127.0.0.1', 0), [Errno 99] Cannot assign requested address
WARNING: Error talking to server: Multiple exceptions: [Errno 111] Connect
call failed ('127.0.0.1', 0), [Errno 99] Cannot assign requested address
WARNING: Error talking to server: Multiple exceptions: [Errno 111] Connect
call failed ('127.0.0.1', 0), [Errno 99] Cannot assign requested address
WARNING: Error talking to server: Multiple exceptions: [Errno 111] Connect
call failed ('127.0.0.1', 0), [Errno 99] Cannot assign requested address
ERROR: PRservice localhost:0 not available
ERROR: Unable to start PR Server, exitting

It would be nice to improve the error handling a bit.

Regards,

[-- Attachment #2: Type: text/html, Size: 5321 bytes --]

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

* Re: [bitbake-devel] [PATCH v6 3/4] prserv: Replace XML RPC with modern asyncrpc implementation
  2021-08-26 16:02   ` [bitbake-devel] " Martin Jansa
@ 2021-08-26 19:17     ` Scott Murray
  0 siblings, 0 replies; 8+ messages in thread
From: Scott Murray @ 2021-08-26 19:17 UTC (permalink / raw)
  To: Martin Jansa; +Cc: bitbake-devel, Richard Purdie, Joshua Watt, Paul Barker

On Thu, 26 Aug 2021, Martin Jansa wrote:

> On Thu, Aug 19, 2021 at 6:47 PM Scott Murray <scott.murray@konsulko.com>
> wrote:
>
> > From: Paul Barker <pbarker@konsulko.com>
> >
> > 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>
> > [updated for asyncrpc changes, client split to separate file]
> > Signed-off-by: Scott Murray <scott.murray@konsulko.com>
> >
> > ....
>
> > -    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 = self.prserv.serve_as_process()
> >
> > -    def getinfo(self):
> > -        return self.host, self.port
> > +        if not self.port:
> > +            self.port = int(self.prserv.address.rsplit(':', 1)[1])
>
> Hi,
>
> I've tried this when running inside docker container and it failed with a
> bit ugly exception:
>
> bitbake@599696cd20aa:~/nodistro/honister$ bitbake -k pkgconfig-native
> Traceback (most recent call last):
>   File "/OE/nodistro/honister/bitbake/bin/bitbake", line 35, in <module>
>     sys.exit(bitbake_main(BitBakeConfigParameters(sys.argv),
>   File "/OE/nodistro/honister/bitbake/lib/bb/main.py", line 385, in
> bitbake_main
>     return ui_module.main(server_connection.connection,
> server_connection.events,
>   File "/OE/nodistro/honister/bitbake/lib/bb/ui/knotty.py", line 397, in
> main
>     params.updateToServer(server, os.environ.copy())
>   File "/OE/nodistro/honister/bitbake/lib/bb/cookerdata.py", line 75, in
> updateToServer
>     raise Exception("Unable to update the server configuration with local
> parameters: %s" % error)
> Exception: Unable to update the server configuration with local parameters:
> Traceback (most recent call last):
>   File "/OE/nodistro/honister/bitbake/lib/bb/command.py", line 90, in
> runCommand
>     result = command_method(self, commandline)
>   File "/OE/nodistro/honister/bitbake/lib/bb/command.py", line 286, in
> updateConfig
>     command.cooker.updateConfigOpts(options, environment, cmdline)
>   File "/OE/nodistro/honister/bitbake/lib/bb/cooker.py", line 491, in
> updateConfigOpts
>     self.reset()
>   File "/OE/nodistro/honister/bitbake/lib/bb/cooker.py", line 1717, in reset
>     self.handlePRServ()
>   File "/OE/nodistro/honister/bitbake/lib/bb/cooker.py", line 383, in
> handlePRServ
>     self.prhost = prserv.serv.auto_start(self.data)
>   File "/OE/nodistro/honister/bitbake/lib/prserv/serv.py", line 318, in
> auto_start
>     singleton.start()
>   File "/OE/nodistro/honister/bitbake/lib/prserv/serv.py", line 133, in
> start
>     self.port = int(self.prserv.address.rsplit(':', 1)[1])
> AttributeError: 'NoneType' object has no attribute 'rsplit'
>
> after removing the rsplit it shows better error message:
>
> bitbake@599696cd20aa:~/nodistro/honister$ bitbake -k pkgconfig-native
> WARNING: Error talking to server: Multiple exceptions: [Errno 111] Connect
> call failed ('127.0.0.1', 0), [Errno 99] Cannot assign requested address
> WARNING: Error talking to server: Multiple exceptions: [Errno 111] Connect
> call failed ('127.0.0.1', 0), [Errno 99] Cannot assign requested address
> WARNING: Error talking to server: Multiple exceptions: [Errno 111] Connect
> call failed ('127.0.0.1', 0), [Errno 99] Cannot assign requested address
> WARNING: Error talking to server: Multiple exceptions: [Errno 111] Connect
> call failed ('127.0.0.1', 0), [Errno 99] Cannot assign requested address
> ERROR: PRservice localhost:0 not available
> ERROR: Unable to start PR Server, exitting
>
> It would be nice to improve the error handling a bit.

Could you elaborate a bit on your configuration?  Is it just trying to
build plain oe-core / poky in Docker?  I don't really see right off from
the code in bitbake's lib/bb/asyncrpc/serv.py how the port from the
asyncio server instance could end up unset.

Thanks,

Scott


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

end of thread, other threads:[~2021-08-26 19:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 16:46 [PATCH v6 0/4] Re-implement prserv on top of asyncrpc Scott Murray
2021-08-19 16:46 ` [PATCH v6 1/4] bitbake: asyncrpc: Defer all asyncio to child process Scott Murray
2021-08-19 16:46 ` [PATCH v6 2/4] bitbake: asyncrpc: always create new asyncio loops Scott Murray
2021-08-19 19:58   ` Joshua Watt
2021-08-19 16:46 ` [PATCH v6 3/4] prserv: Replace XML RPC with modern asyncrpc implementation Scott Murray
2021-08-26 16:02   ` [bitbake-devel] " Martin Jansa
2021-08-26 19:17     ` Scott Murray
2021-08-19 16:46 ` [PATCH v6 4/4] prserv: Add read-only mode Scott Murray

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.