[PATCH 0/3] PRServer: Fixes daemon issues.

[PATCH 0/3] PRServer: Fixes daemon issues.

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

First two patches fix issues when using hostnames instead of IPs. Third patch
change the journal mode, allowing the DB to be hosted on a remote server and
network filesystems. Detail is given on each patch. Fixes 8258, 8560 and 8215

The following changes since commit 71b0568fa43285f0946fae93fb43cea5f3bbecec:

  rt-tests: drop unnecessary added-missing-dependencies.patch (2015-09-01 11:44:04 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib lsandov1/Remote-PR-server-not-writing-PR-values-to-database
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=lsandov1/Remote-PR-server-not-writing-PR-values-to-database

Leonardo Sandoval (3):
  prserv/serv: Start/Stop daemon using ip instead of host
  prserv/serv.py: Better messaging when starting/stopping the server
    with port=0
  prserv/db: Use DELETE instead of WAL journal mode

 bitbake/lib/prserv/db.py   |  2 +-
 bitbake/lib/prserv/serv.py | 36 +++++++++++++++++++++++++++++-------
 2 files changed, 30 insertions(+), 8 deletions(-)

-- 
1.8.4.5

[PATCH 1/3] prserv/serv: Start/Stop daemon using ip instead of host

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

In cases where hostname is given instead of an IP (i.e. localhost
instead of 127.0.0.1) when stopping the server with bitbake-prserv --stop,
the server shows a misleading message indicating that the daemon was not
found, where it is actually stopped. This patch converts host to IP values
before starting/stopping the daemon, so it will always work on IP, not on
hostnames, avoiding problems like the latter.

[YOCTO #8258]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 bitbake/lib/prserv/serv.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/bitbake/lib/prserv/serv.py b/bitbake/lib/prserv/serv.py
index 0507485..0d96963 100644
--- a/bitbake/lib/prserv/serv.py
+++ b/bitbake/lib/prserv/serv.py
@@ -3,6 +3,7 @@ import signal, time
 from SimpleXMLRPCServer import SimpleXMLRPCServer, SimpleXMLRPCRequestHandler
 import threading
 import Queue
+import socket
 
 try:
     import sqlite3
@@ -37,7 +38,6 @@ singleton = None
 class PRServer(SimpleXMLRPCServer):
     def __init__(self, dbfile, logfile, interface, daemon=True):
         ''' constructor '''
-        import socket
         try:
             SimpleXMLRPCServer.__init__(self, interface,
                                         logRequests=False, allow_none=True)
@@ -261,7 +261,8 @@ class PRServerConnection(object):
         return self.host, self.port
 
 def start_daemon(dbfile, host, port, logfile):
-    pidfile = PIDPREFIX % (host, port)
+    ip = socket.gethostbyname(host)
+    pidfile = PIDPREFIX % (ip, port)
     try:
         pf = file(pidfile,'r')
         pid = int(pf.readline().strip())
@@ -274,12 +275,14 @@ def start_daemon(dbfile, host, port, logfile):
                             % pidfile)
         return 1
 
-    server = PRServer(os.path.abspath(dbfile), os.path.abspath(logfile), (host,port))
+    server = PRServer(os.path.abspath(dbfile), os.path.abspath(logfile), (ip,port))
     server.start()
+
     return 0
 
 def stop_daemon(host, port):
-    pidfile = PIDPREFIX % (host, port)
+    ip = socket.gethostbyname(host)
+    pidfile = PIDPREFIX % (ip, port)
     try:
         pf = file(pidfile,'r')
         pid = int(pf.readline().strip())
@@ -292,7 +295,7 @@ def stop_daemon(host, port):
                         % pidfile)
 
     try:
-        PRServerConnection(host, port).terminate()
+        PRServerConnection(ip, port).terminate()
     except:
         logger.critical("Stop PRService %s:%d failed" % (host,port))
 
-- 
1.8.4.5

[PATCH 2/3] prserv/serv.py: Better messaging when starting/stopping the server with port=0

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

When starting the server using port=0, the server actually starts with a
different port, so print a message with this new value. When stopping the
server with port=0, advise the user which ports the server is listening to,
so next time it tries to close it, user can pick up the correct one.

[YOCTO #8560]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 bitbake/lib/prserv/serv.py | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/bitbake/lib/prserv/serv.py b/bitbake/lib/prserv/serv.py
index 0d96963..8b000e6 100644
--- a/bitbake/lib/prserv/serv.py
+++ b/bitbake/lib/prserv/serv.py
@@ -278,9 +278,16 @@ def start_daemon(dbfile, host, port, logfile):
     server = PRServer(os.path.abspath(dbfile), os.path.abspath(logfile), (ip,port))
     server.start()
 
+    # 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.stderr.write("WARNING: Server is listening at port %s instead of %s\n"
+                         % (rport,port))
     return 0
 
 def stop_daemon(host, port):
+    import glob
     ip = socket.gethostbyname(host)
     pidfile = PIDPREFIX % (ip, port)
     try:
@@ -291,8 +298,20 @@ def stop_daemon(host, port):
         pid = None
 
     if not pid:
-        sys.stderr.write("pidfile %s does not exist. Daemon not running?\n"
-                        % pidfile)
+        # when server starts at port=0 (i.e. localhost:0), server actually takes another port,
+        # so at least advise the user which ports the corresponding server is listening
+        ports = []
+        portstr = ""
+        for pf in glob.glob(PIDPREFIX % (ip,'*')):
+            bn = os.path.basename(pf)
+            root, _ = os.path.splitext(bn)
+            ports.append(root.split('_')[-1])
+        if len(ports):
+            portstr = "Wrong port? Other ports listening at %s: %s" % (host, ' '.join(ports))
+
+        sys.stderr.write("pidfile %s does not exist. Daemon not running? %s\n"
+                         % (pidfile,portstr))
+        return 1
 
     try:
         PRServerConnection(ip, port).terminate()
-- 
1.8.4.5

Re: [PATCH 2/3] prserv/serv.py: Better messaging when starting/stopping the server with port=0

[Burton, Ross]

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

On 15 September 2015 at 15:59, <leonardo.sandoval.gonzalez@linux.intel.com>
wrote:

> +        sys.stderr.write("WARNING: Server is listening at port %s instead
> of %s\n"
> +                         % (rport,port))
>

I wouldn't call this a warning: port=0 means "pick your own", so it's not a
warning that a different port was changed, it's useful information.

Ross

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

Re: [PATCH 2/3] prserv/serv.py: Better messaging when starting/stopping the server with port=0

[Leonardo Sandoval]

On 09/17/2015 03:01 PM, Burton, Ross wrote:
> On 15 September 2015 at 15:59, <leonardo.sandoval.gonzalez@linux.intel.com>
> wrote:
>
>> +        sys.stderr.write("WARNING: Server is listening at port %s instead
>> of %s\n"
>> +                         % (rport,port))
>>
>
> I wouldn't call this a warning: port=0 means "pick your own", so it's not a
> warning that a different port was changed, it's useful information.

I add the WARNING string so user notices the new port taken. This info 
is important when trying to stop the server, but you are right, is not a 
warning, it should be just an info. Sending V2 right now.

>
> Ross
>

[PATCH 3/3] prserv/db: Use DELETE instead of WAL journal mode

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Disadvantages of WAL journal mode are explained on [1], but the
one affecting our case is "All processes using a database must be
on the same host computer; WAL does not work over a network filesystem".
Changing the Journal mode into DELETE which is the normal behavior for
rollback journal.

[YOCTO #8215]

[1] https://www.sqlite.org/wal.html

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 bitbake/lib/prserv/db.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bitbake/lib/prserv/db.py b/bitbake/lib/prserv/db.py
index 4379580..b3e3a4f 100644
--- a/bitbake/lib/prserv/db.py
+++ b/bitbake/lib/prserv/db.py
@@ -245,7 +245,7 @@ class PRData(object):
         self.connection=sqlite3.connect(self.filename, 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 = WAL;")
+        self.connection.execute("PRAGMA journal_mode = DELETE;")
         self._tables={}
 
     def __del__(self):
-- 
1.8.4.5

Re: [PATCH 3/3] prserv/db: Use DELETE instead of WAL journal mode

[Burton, Ross]

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

On 15 September 2015 at 15:59, <leonardo.sandoval.gonzalez@linux.intel.com>
wrote:

> -        self.connection.execute("PRAGMA journal_mode = WAL;")
> +        self.connection.execute("PRAGMA journal_mode = DELETE;")
>

Richard probably has a better memory than me but I seem to recall that WAL
was a pretty serious speed improvement for the local host case.  Did you
benchmark the impact this change has?

Ross

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

Re: [PATCH 3/3] prserv/db: Use DELETE instead of WAL journal mode

[Leonardo Sandoval]

On 09/17/2015 03:01 PM, Burton, Ross wrote:
> On 15 September 2015 at 15:59, <leonardo.sandoval.gonzalez@linux.intel.com>
> wrote:
>
>> -        self.connection.execute("PRAGMA journal_mode = WAL;")
>> +        self.connection.execute("PRAGMA journal_mode = DELETE;")
>>
>
> Richard probably has a better memory than me but I seem to recall that WAL
> was a pretty serious speed improvement for the local host case.  Did you
> benchmark the impact this change has?

Unfortunately, I didn't do any benchmark.

The problem with WAL is the following "All processes using a database 
must be on the same host computer; WAL does not work over a network 
filesystem." Using WAL, all PR values get lost after a PR server reboot, 
so we need a rollback journal. According to the documentation, the 
fastest of the these is "MEMORY" but it has its pros/cons:

"The MEMORY journaling mode stores the rollback journal in volatile RAM. 
This saves disk I/O but at the expense of database safety and integrity. 
If the application using SQLite crashes in the middle of a transaction 
when the MEMORY journaling mode is set, then the database file will very 
likely go corrupt."

>
> Ross
>

Re: [PATCH 3/3] prserv/db: Use DELETE instead of WAL journal mode

[Leonardo Sandoval]

On 09/18/2015 11:42 AM, Leonardo Sandoval wrote:
>
>
> On 09/17/2015 03:01 PM, Burton, Ross wrote:
>> On 15 September 2015 at 15:59,
>> <leonardo.sandoval.gonzalez@linux.intel.com>
>> wrote:
>>
>>> -        self.connection.execute("PRAGMA journal_mode = WAL;")
>>> +        self.connection.execute("PRAGMA journal_mode = DELETE;")
>>>
>>
>> Richard probably has a better memory than me but I seem to recall that
>> WAL
>> was a pretty serious speed improvement for the local host case.  Did you
>> benchmark the impact this change has?
>
> Unfortunately, I didn't do any benchmark.
>
> The problem with WAL is the following "All processes using a database
> must be on the same host computer; WAL does not work over a network
> filesystem." Using WAL, all PR values get lost after a PR server reboot,
> so we need a rollback journal. According to the documentation, the
> fastest of the these is "MEMORY" but it has its pros/cons:
>
> "The MEMORY journaling mode stores the rollback journal in volatile RAM.
> This saves disk I/O but at the expense of database safety and integrity.
> If the application using SQLite crashes in the middle of a transaction
> when the MEMORY journaling mode is set, then the database file will very
> likely go corrupt."
>

Ross: ignore my comment. The limitation "all process using a database 
must be on the same host" is not a limitation for us, because the 
PRserver is the only one talking to the database and this daemon/process 
is at the same place as the DB. So the root reason why WAL is not 
working is still unknown. Sending a V2 soon.

>>
>> Ross
>>