All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hashserv: Fix read-only mode.
@ 2023-09-29 12:37 Karim Ben Houcine
  2023-09-29 13:14 ` Karim Ben Houcine
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Karim Ben Houcine @ 2023-09-29 12:37 UTC (permalink / raw)
  To: bitbake-devel; +Cc: Karim Ben Houcine

Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com>
---
 lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
index d40a2ab8..56d76f1f 100644
--- a/lib/hashserv/server.py
+++ b/lib/hashserv/server.py
@@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
             'get-stats': self.handle_get_stats,
         })
 
-        if not read_only:
+        if read_only:
+            self.handlers.update({    
+                'report': self.handle_readonly_report,
+            })
+        else:
             self.handlers.update({
                 'report': self.handle_report,
                 'report-equiv': self.handle_equivreport,
@@ -453,6 +457,55 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
 
         self.write_message(d)
 
+
+    async def handle_readonly_report(self, data):
+        with closing(self.db.cursor()) as cursor:
+            outhash_data = {
+                'method': data['method'],
+                'outhash': data['outhash'],
+                'taskhash': data['taskhash'],
+                'created': datetime.now()
+            }
+
+            for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):
+                if k in data:
+                    outhash_data[k] = data[k] 
+
+            # Check if outhash is known
+            cursor.execute(
+                '''
+                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2
+                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash
+                -- Select any matching output hash
+                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash
+                -- Pick the oldest hash
+                ORDER BY outhashes_v2.created ASC
+                LIMIT 1
+                ''',
+                {
+                    'method': data['method'],
+                    'outhash': data['outhash'],
+                    'taskhash': data['taskhash'],
+                }
+            )
+            row = cursor.fetchone()
+            if row is not None: 
+                # outhash is known => corrects unihash
+                unihash = row['unihash'] 
+            else:
+                # outhash is unknown => nothing to do
+                unihash = outhash_data['taskhash'] 
+            
+
+            d = {
+                'taskhash': data['taskhash'],
+                'method': data['method'],
+                'unihash': unihash,
+            }
+
+        self.write_message(d)
+        
+
     async def handle_equivreport(self, data):
         with closing(self.db.cursor()) as cursor:
             insert_data = {
-- 
2.25.1



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

* [PATCH] hashserv: Fix read-only mode.
  2023-09-29 12:37 [PATCH] hashserv: Fix read-only mode Karim Ben Houcine
@ 2023-09-29 13:14 ` Karim Ben Houcine
  2023-09-30 10:47   ` [bitbake-devel] " Alexandre Belloni
  2023-10-02 20:49 ` Joshua Watt
  2023-10-03 15:56 ` [bitbake-devel] [PATCH] " Joshua Watt
  2 siblings, 1 reply; 11+ messages in thread
From: Karim Ben Houcine @ 2023-09-29 13:14 UTC (permalink / raw)
  To: bitbake-devel; +Cc: Karim Ben Houcine

TCP read-only hash equivalence server is not working: the connection is prematurely closed without even notifying the value of the unihash.
Expected behaviour is:
    1. the client sends a ‘report’ message indicating the 'taskhash', 'method', 'outhash' and a proposed value of 'unihash'.
    2.the server sends back the 'taskhash', 'method' and actual value of 'unihash' to use.
The problem is that in read-only mode, the server rejects 'report' messages (connexion is closed).

The proposed fix consists in enabling ‘report’ messages in read-only mode using a specific handler that doesn’t modify the hash equivalence database.

Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com>
---
 lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
index d40a2ab8..56d76f1f 100644
--- a/lib/hashserv/server.py
+++ b/lib/hashserv/server.py
@@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
             'get-stats': self.handle_get_stats,
         })
 
-        if not read_only:
+        if read_only:
+            self.handlers.update({    
+                'report': self.handle_readonly_report,
+            })
+        else:
             self.handlers.update({
                 'report': self.handle_report,
                 'report-equiv': self.handle_equivreport,
@@ -453,6 +457,55 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
 
         self.write_message(d)
 
+
+    async def handle_readonly_report(self, data):
+        with closing(self.db.cursor()) as cursor:
+            outhash_data = {
+                'method': data['method'],
+                'outhash': data['outhash'],
+                'taskhash': data['taskhash'],
+                'created': datetime.now()
+            }
+
+            for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):
+                if k in data:
+                    outhash_data[k] = data[k] 
+
+            # Check if outhash is known
+            cursor.execute(
+                '''
+                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2
+                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash
+                -- Select any matching output hash
+                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash
+                -- Pick the oldest hash
+                ORDER BY outhashes_v2.created ASC
+                LIMIT 1
+                ''',
+                {
+                    'method': data['method'],
+                    'outhash': data['outhash'],
+                    'taskhash': data['taskhash'],
+                }
+            )
+            row = cursor.fetchone()
+            if row is not None: 
+                # outhash is known => corrects unihash
+                unihash = row['unihash'] 
+            else:
+                # outhash is unknown => nothing to do
+                unihash = outhash_data['taskhash'] 
+            
+
+            d = {
+                'taskhash': data['taskhash'],
+                'method': data['method'],
+                'unihash': unihash,
+            }
+
+        self.write_message(d)
+        
+
     async def handle_equivreport(self, data):
         with closing(self.db.cursor()) as cursor:
             insert_data = {
-- 
2.25.1



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

* Re: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.
  2023-09-29 13:14 ` Karim Ben Houcine
@ 2023-09-30 10:47   ` Alexandre Belloni
  2023-10-02 11:45     ` Ben Houcine, Karim
  0 siblings, 1 reply; 11+ messages in thread
From: Alexandre Belloni @ 2023-09-30 10:47 UTC (permalink / raw)
  To: Ben Houcine, Karim; +Cc: bitbake-devel

Hello,

This causes bitbake selftest failures:

https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/5830/steps/12/logs/stdio

======================================================================
FAIL: test_ro_server (hashserv.tests.TestHashEquivalenceTCPServer.test_ro_server)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/bitbake/lib/hashserv/tests.py", line 320, in test_ro_server
    with self.assertRaises(ConnectionError):
AssertionError: ConnectionError not raised

======================================================================
FAIL: test_ro_server (hashserv.tests.TestHashEquivalenceUnixServer.test_ro_server)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/bitbake/lib/hashserv/tests.py", line 320, in test_ro_server
    with self.assertRaises(ConnectionError):
AssertionError: ConnectionError not raised

On 29/09/2023 15:14:47+0200, Ben Houcine, Karim wrote:
> TCP read-only hash equivalence server is not working: the connection is prematurely closed without even notifying the value of the unihash.
> Expected behaviour is:
>     1. the client sends a ‘report’ message indicating the 'taskhash', 'method', 'outhash' and a proposed value of 'unihash'.
>     2.the server sends back the 'taskhash', 'method' and actual value of 'unihash' to use.
> The problem is that in read-only mode, the server rejects 'report' messages (connexion is closed).
> 
> The proposed fix consists in enabling ‘report’ messages in read-only mode using a specific handler that doesn’t modify the hash equivalence database.
> 
> Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com>
> ---
>  lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
> index d40a2ab8..56d76f1f 100644
> --- a/lib/hashserv/server.py
> +++ b/lib/hashserv/server.py
> @@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>              'get-stats': self.handle_get_stats,
>          })
>  
> -        if not read_only:
> +        if read_only:
> +            self.handlers.update({    
> +                'report': self.handle_readonly_report,
> +            })
> +        else:
>              self.handlers.update({
>                  'report': self.handle_report,
>                  'report-equiv': self.handle_equivreport,
> @@ -453,6 +457,55 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>  
>          self.write_message(d)
>  
> +
> +    async def handle_readonly_report(self, data):
> +        with closing(self.db.cursor()) as cursor:
> +            outhash_data = {
> +                'method': data['method'],
> +                'outhash': data['outhash'],
> +                'taskhash': data['taskhash'],
> +                'created': datetime.now()
> +            }
> +
> +            for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):
> +                if k in data:
> +                    outhash_data[k] = data[k] 
> +
> +            # Check if outhash is known
> +            cursor.execute(
> +                '''
> +                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2
> +                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash
> +                -- Select any matching output hash
> +                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash
> +                -- Pick the oldest hash
> +                ORDER BY outhashes_v2.created ASC
> +                LIMIT 1
> +                ''',
> +                {
> +                    'method': data['method'],
> +                    'outhash': data['outhash'],
> +                    'taskhash': data['taskhash'],
> +                }
> +            )
> +            row = cursor.fetchone()
> +            if row is not None: 
> +                # outhash is known => corrects unihash
> +                unihash = row['unihash'] 
> +            else:
> +                # outhash is unknown => nothing to do
> +                unihash = outhash_data['taskhash'] 
> +            
> +
> +            d = {
> +                'taskhash': data['taskhash'],
> +                'method': data['method'],
> +                'unihash': unihash,
> +            }
> +
> +        self.write_message(d)
> +        
> +
>      async def handle_equivreport(self, data):
>          with closing(self.db.cursor()) as cursor:
>              insert_data = {
> -- 
> 2.25.1
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#15150): https://lists.openembedded.org/g/bitbake-devel/message/15150
> Mute This Topic: https://lists.openembedded.org/mt/101656393/3617179
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


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

* RE: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.
  2023-09-30 10:47   ` [bitbake-devel] " Alexandre Belloni
@ 2023-10-02 11:45     ` Ben Houcine, Karim
  2023-10-02 18:11       ` Alexandre Belloni
  0 siblings, 1 reply; 11+ messages in thread
From: Ben Houcine, Karim @ 2023-10-02 11:45 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: bitbake-devel@lists.openembedded.org, OTHACEHE, Mathieu (EXT)

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

Hello Alexandre,

In fact, the proposed fix is expected to fail the test.
So the test should be modified if the proposed patch is approved.

When using the readonly server, the client must be notified of the actual unihash, but if the server  rejects the report message it can’t occur.
So we propose to allow report messages on the readonly server, but to prohibit the modification of the database.
Meaning that this report message is used to read the unihash but ont to write it, precisely what is expected of a readonly feature.


I’ll propose a fix of the patch that corrects the test.

Best regards,

Karim Ben Houcine

De : Alexandre Belloni <alexandre.belloni@bootlin.com>
Envoyé : samedi 30 septembre 2023 12:48
À : Ben Houcine, Karim <Karim.benhoucine@landisgyr.com>
Cc : bitbake-devel@lists.openembedded.org
Objet : Re: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.

Hello, This causes bitbake selftest failures: https: //urldefense. com/v3/__https: //autobuilder. yoctoproject. org/typhoon/*/builders/79/builds/5830/steps/12/logs/stdio__;Iw!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbVPQFPeA$
ZjQcmQRYFpfptBannerStart
This Message Is From an Untrusted Sender
You have not previously corresponded with this sender.
ZjQcmQRYFpfptBannerEnd

Hello,



This causes bitbake selftest failures:



https://urldefense.com/v3/__https://autobuilder.yoctoproject.org/typhoon/*/builders/79/builds/5830/steps/12/logs/stdio__;Iw!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbVPQFPeA$<https://urldefense.com/v3/__https:/autobuilder.yoctoproject.org/typhoon/*/builders/79/builds/5830/steps/12/logs/stdio__;Iw!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbVPQFPeA$>



======================================================================

FAIL: test_ro_server (hashserv.tests.TestHashEquivalenceTCPServer.test_ro_server)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/bitbake/lib/hashserv/tests.py", line 320, in test_ro_server

    with self.assertRaises(ConnectionError):

AssertionError: ConnectionError not raised



======================================================================

FAIL: test_ro_server (hashserv.tests.TestHashEquivalenceUnixServer.test_ro_server)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/bitbake/lib/hashserv/tests.py", line 320, in test_ro_server

    with self.assertRaises(ConnectionError):

AssertionError: ConnectionError not raised



On 29/09/2023 15:14:47+0200, Ben Houcine, Karim wrote:

> TCP read-only hash equivalence server is not working: the connection is prematurely closed without even notifying the value of the unihash.

> Expected behaviour is:

>     1. the client sends a ‘report’ message indicating the 'taskhash', 'method', 'outhash' and a proposed value of 'unihash'.

>     2.the server sends back the 'taskhash', 'method' and actual value of 'unihash' to use.

> The problem is that in read-only mode, the server rejects 'report' messages (connexion is closed).

>

> The proposed fix consists in enabling ‘report’ messages in read-only mode using a specific handler that doesn’t modify the hash equivalence database.

>

> Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com<mailto:karim.benhoucine@landisgyr.com>>

> ---

>  lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++-

>  1 file changed, 54 insertions(+), 1 deletion(-)

>

> diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py

> index d40a2ab8..56d76f1f 100644

> --- a/lib/hashserv/server.py

> +++ b/lib/hashserv/server.py

> @@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):

>              'get-stats': self.handle_get_stats,

>          })

>

> -        if not read_only:

> +        if read_only:

> +            self.handlers.update({

> +                'report': self.handle_readonly_report,

> +            })

> +        else:

>              self.handlers.update({

>                  'report': self.handle_report,

>                  'report-equiv': self.handle_equivreport,

> @@ -453,6 +457,55 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):

>

>          self.write_message(d)

>

> +

> +    async def handle_readonly_report(self, data):

> +        with closing(self.db.cursor()) as cursor:

> +            outhash_data = {

> +                'method': data['method'],

> +                'outhash': data['outhash'],

> +                'taskhash': data['taskhash'],

> +                'created': datetime.now()

> +            }

> +

> +            for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):

> +                if k in data:

> +                    outhash_data[k] = data[k]

> +

> +            # Check if outhash is known

> +            cursor.execute(

> +                '''

> +                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2

> +                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash

> +                -- Select any matching output hash

> +                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash

> +                -- Pick the oldest hash

> +                ORDER BY outhashes_v2.created ASC

> +                LIMIT 1

> +                ''',

> +                {

> +                    'method': data['method'],

> +                    'outhash': data['outhash'],

> +                    'taskhash': data['taskhash'],

> +                }

> +            )

> +            row = cursor.fetchone()

> +            if row is not None:

> +                # outhash is known => corrects unihash

> +                unihash = row['unihash']

> +            else:

> +                # outhash is unknown => nothing to do

> +                unihash = outhash_data['taskhash']

> +

> +

> +            d = {

> +                'taskhash': data['taskhash'],

> +                'method': data['method'],

> +                'unihash': unihash,

> +            }

> +

> +        self.write_message(d)

> +

> +

>      async def handle_equivreport(self, data):

>          with closing(self.db.cursor()) as cursor:

>              insert_data = {

> --

> 2.25.1

>



>

> -=-=-=-=-=-=-=-=-=-=-=-

> Links: You receive all messages sent to this group.

> View/Reply Online (#15150): https://urldefense.com/v3/__https://lists.openembedded.org/g/bitbake-devel/message/15150__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbQn24puX$<https://urldefense.com/v3/__https:/lists.openembedded.org/g/bitbake-devel/message/15150__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbQn24puX$>

> Mute This Topic: https://urldefense.com/v3/__https://lists.openembedded.org/mt/101656393/3617179__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbZgslJQi$<https://urldefense.com/v3/__https:/lists.openembedded.org/mt/101656393/3617179__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbZgslJQi$>

> Group Owner: bitbake-devel+owner@lists.openembedded.org<mailto:bitbake-devel+owner@lists.openembedded.org>

> Unsubscribe: https://urldefense.com/v3/__https://lists.openembedded.org/g/bitbake-devel/unsub__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbcYsSLHx$<https://urldefense.com/v3/__https:/lists.openembedded.org/g/bitbake-devel/unsub__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbcYsSLHx$> [alexandre.belloni@bootlin.com]

> -=-=-=-=-=-=-=-=-=-=-=-

>





--

Alexandre Belloni, co-owner and COO, Bootlin

Embedded Linux and Kernel engineering

https://urldefense.com/v3/__https://bootlin.com__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0Lpthbahu1TC1$<https://urldefense.com/v3/__https:/bootlin.com__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0Lpthbahu1TC1$>


P PLEASE CONSIDER OUR ENVIRONMENT BEFORE PRINTING THIS EMAIL.

This e-mail (including any attachments) is confidential and may be legally privileged. If you are not an intended recipient or an authorized representative of an intended recipient, you are prohibited from using, copying or distributing the information in this e-mail or its attachments. If you have received this e-mail in error, please notify the sender immediately by return e-mail and delete all copies of this message and any attachments. Thank you.

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

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

* Re: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.
  2023-10-02 11:45     ` Ben Houcine, Karim
@ 2023-10-02 18:11       ` Alexandre Belloni
  0 siblings, 0 replies; 11+ messages in thread
From: Alexandre Belloni @ 2023-10-02 18:11 UTC (permalink / raw)
  To: Ben Houcine, Karim
  Cc: bitbake-devel@lists.openembedded.org, OTHACEHE, Mathieu (EXT)

Hi,

On 02/10/2023 11:45:22+0000, Ben Houcine, Karim wrote:
> Hello Alexandre,
> 
> In fact, the proposed fix is expected to fail the test.
> So the test should be modified if the proposed patch is approved.
> 
> When using the readonly server, the client must be notified of the actual unihash, but if the server  rejects the report message it can’t occur.
> So we propose to allow report messages on the readonly server, but to prohibit the modification of the database.
> Meaning that this report message is used to read the unihash but ont to write it, precisely what is expected of a readonly feature.
> 
> 
> I’ll propose a fix of the patch that corrects the test.

Yes, tests need to pass after applying your patch.

> 
> Best regards,
> 
> Karim Ben Houcine
> 
> De : Alexandre Belloni <alexandre.belloni@bootlin.com>
> Envoyé : samedi 30 septembre 2023 12:48
> À : Ben Houcine, Karim <Karim.benhoucine@landisgyr.com>
> Cc : bitbake-devel@lists.openembedded.org
> Objet : Re: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.
> 
> Hello, This causes bitbake selftest failures: https: //urldefense. com/v3/__https: //autobuilder. yoctoproject. org/typhoon/*/builders/79/builds/5830/steps/12/logs/stdio__;Iw!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbVPQFPeA$
> ZjQcmQRYFpfptBannerStart
> This Message Is From an Untrusted Sender
> You have not previously corresponded with this sender.
> ZjQcmQRYFpfptBannerEnd
> 
> Hello,
> 
> 
> 
> This causes bitbake selftest failures:
> 
> 
> 
> https://urldefense.com/v3/__https://autobuilder.yoctoproject.org/typhoon/*/builders/79/builds/5830/steps/12/logs/stdio__;Iw!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbVPQFPeA$<https://urldefense.com/v3/__https:/autobuilder.yoctoproject.org/typhoon/*/builders/79/builds/5830/steps/12/logs/stdio__;Iw!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0LpthbVPQFPeA$>
> 
> 
> 
> ======================================================================
> 
> FAIL: test_ro_server (hashserv.tests.TestHashEquivalenceTCPServer.test_ro_server)
> 
> ----------------------------------------------------------------------
> 
> Traceback (most recent call last):
> 
>   File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/bitbake/lib/hashserv/tests.py", line 320, in test_ro_server
> 
>     with self.assertRaises(ConnectionError):
> 
> AssertionError: ConnectionError not raised
> 
> 
> 
> ======================================================================
> 
> FAIL: test_ro_server (hashserv.tests.TestHashEquivalenceUnixServer.test_ro_server)
> 
> ----------------------------------------------------------------------
> 
> Traceback (most recent call last):
> 
>   File "/home/pokybuild/yocto-worker/oe-selftest-centos/build/bitbake/lib/hashserv/tests.py", line 320, in test_ro_server
> 
>     with self.assertRaises(ConnectionError):
> 
> AssertionError: ConnectionError not raised
> 
> 
> 
> On 29/09/2023 15:14:47+0200, Ben Houcine, Karim wrote:
> 
> > TCP read-only hash equivalence server is not working: the connection is prematurely closed without even notifying the value of the unihash.
> 
> > Expected behaviour is:
> 
> >     1. the client sends a ‘report’ message indicating the 'taskhash', 'method', 'outhash' and a proposed value of 'unihash'.
> 
> >     2.the server sends back the 'taskhash', 'method' and actual value of 'unihash' to use.
> 
> > The problem is that in read-only mode, the server rejects 'report' messages (connexion is closed).
> 
> >
> 
> > The proposed fix consists in enabling ‘report’ messages in read-only mode using a specific handler that doesn’t modify the hash equivalence database.
> 
> >
> 
> > Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com<mailto:karim.benhoucine@landisgyr.com>>
> 
> > ---
> 
> >  lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++-
> 
> >  1 file changed, 54 insertions(+), 1 deletion(-)
> 
> >
> 
> > diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
> 
> > index d40a2ab8..56d76f1f 100644
> 
> > --- a/lib/hashserv/server.py
> 
> > +++ b/lib/hashserv/server.py
> 
> > @@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
> 
> >              'get-stats': self.handle_get_stats,
> 
> >          })
> 
> >
> 
> > -        if not read_only:
> 
> > +        if read_only:
> 
> > +            self.handlers.update({
> 
> > +                'report': self.handle_readonly_report,
> 
> > +            })
> 
> > +        else:
> 
> >              self.handlers.update({
> 
> >                  'report': self.handle_report,
> 
> >                  'report-equiv': self.handle_equivreport,
> 
> > @@ -453,6 +457,55 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
> 
> >
> 
> >          self.write_message(d)
> 
> >
> 
> > +
> 
> > +    async def handle_readonly_report(self, data):
> 
> > +        with closing(self.db.cursor()) as cursor:
> 
> > +            outhash_data = {
> 
> > +                'method': data['method'],
> 
> > +                'outhash': data['outhash'],
> 
> > +                'taskhash': data['taskhash'],
> 
> > +                'created': datetime.now()
> 
> > +            }
> 
> > +
> 
> > +            for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):
> 
> > +                if k in data:
> 
> > +                    outhash_data[k] = data[k]
> 
> > +
> 
> > +            # Check if outhash is known
> 
> > +            cursor.execute(
> 
> > +                '''
> 
> > +                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2
> 
> > +                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash
> 
> > +                -- Select any matching output hash
> 
> > +                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash
> 
> > +                -- Pick the oldest hash
> 
> > +                ORDER BY outhashes_v2.created ASC
> 
> > +                LIMIT 1
> 
> > +                ''',
> 
> > +                {
> 
> > +                    'method': data['method'],
> 
> > +                    'outhash': data['outhash'],
> 
> > +                    'taskhash': data['taskhash'],
> 
> > +                }
> 
> > +            )
> 
> > +            row = cursor.fetchone()
> 
> > +            if row is not None:
> 
> > +                # outhash is known => corrects unihash
> 
> > +                unihash = row['unihash']
> 
> > +            else:
> 
> > +                # outhash is unknown => nothing to do
> 
> > +                unihash = outhash_data['taskhash']
> 
> > +
> 
> > +
> 
> > +            d = {
> 
> > +                'taskhash': data['taskhash'],
> 
> > +                'method': data['method'],
> 
> > +                'unihash': unihash,
> 
> > +            }
> 
> > +
> 
> > +        self.write_message(d)
> 
> > +
> 
> > +
> 
> >      async def handle_equivreport(self, data):
> 
> >          with closing(self.db.cursor()) as cursor:
> 
> >              insert_data = {
> 
> > --
> 
> > 2.25.1
> 
> >
> 
> 
> 
> >
> 
> > 
> 
> >
> 
> 
> 
> 
> 
> --
> 
> Alexandre Belloni, co-owner and COO, Bootlin
> 
> Embedded Linux and Kernel engineering
> 
> https://urldefense.com/v3/__https://bootlin.com__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0Lpthbahu1TC1$<https://urldefense.com/v3/__https:/bootlin.com__;!!EEzGOCEfvrF80k8sMoAmtYTD!IUFhiS5Htu4x_xUcJbW2daiPP2vntMXgX5HseX1OyW2wauGwM2hV_LKRlYwRjcJbtp1zrTNhBeADPZSEo5Zp98D0Lpthbahu1TC1$>
> 
> 
> P PLEASE CONSIDER OUR ENVIRONMENT BEFORE PRINTING THIS EMAIL.
> 
> This e-mail (including any attachments) is confidential and may be legally privileged. If you are not an intended recipient or an authorized representative of an intended recipient, you are prohibited from using, copying or distributing the information in this e-mail or its attachments. If you have received this e-mail in error, please notify the sender immediately by return e-mail and delete all copies of this message and any attachments. Thank you.

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#15154): https://lists.openembedded.org/g/bitbake-devel/message/15154
> Mute This Topic: https://lists.openembedded.org/mt/101656393/3617179
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


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

* Re: [bitbake-devel] [PATCH] hashserv: Fix read-only mode
  2023-10-02 11:56 ` Karim Ben Houcine
@ 2023-10-02 18:46   ` Alexandre Belloni
  0 siblings, 0 replies; 11+ messages in thread
From: Alexandre Belloni @ 2023-10-02 18:46 UTC (permalink / raw)
  To: Ben Houcine, Karim; +Cc: bitbake-devel

Hello,

This doesn't apply on bitbake master, can you rebase and fix?

On 02/10/2023 13:56:41+0200, Ben Houcine, Karim wrote:
> TCP read-only hash equivalence server is not working: the connection is prematurely closed without even notifying the value of the unihash.
> Expected behaviour is:
>     1. the client sends a ‘report’ message indicating the 'taskhash', 'method', 'outhash' and a proposed value of 'unihash'.
>     2.the server sends back the 'taskhash', 'method' and actual value of 'unihash' to use.
> The problem is that in read-only mode, the server rejects 'report' messages (connexion is closed).
> hashserv.tests.TestHashEquivalenceUnixServer.test_ro_server test modified accordingly
> 
> Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com>
> ---
>  bitbake/lib/hashserv/server.py | 14 +++++++-------
>  bitbake/lib/hashserv/tests.py  |  4 +---
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/bitbake/lib/hashserv/server.py b/bitbake/lib/hashserv/server.py
> index 56d76f1f81..a12937e25f 100644
> --- a/bitbake/lib/hashserv/server.py
> +++ b/bitbake/lib/hashserv/server.py
> @@ -181,7 +181,7 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>          })
>  
>          if read_only:
> -            self.handlers.update({    
> +            self.handlers.update({
>                  'report': self.handle_readonly_report,
>              })
>          else:
> @@ -469,7 +469,7 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>  
>              for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):
>                  if k in data:
> -                    outhash_data[k] = data[k] 
> +                    outhash_data[k] = data[k]
>  
>              # Check if outhash is known
>              cursor.execute(
> @@ -489,13 +489,13 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>                  }
>              )
>              row = cursor.fetchone()
> -            if row is not None: 
> +            if row is not None:
>                  # outhash is known => corrects unihash
> -                unihash = row['unihash'] 
> +                unihash = row['unihash']
>              else:
>                  # outhash is unknown => nothing to do
> -                unihash = outhash_data['taskhash'] 
> -            
> +                unihash = outhash_data['taskhash']
> +
>  
>              d = {
>                  'taskhash': data['taskhash'],
> @@ -504,7 +504,7 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>              }
>  
>          self.write_message(d)
> -        
> +
>  
>      async def handle_equivreport(self, data):
>          with closing(self.db.cursor()) as cursor:
> diff --git a/bitbake/lib/hashserv/tests.py b/bitbake/lib/hashserv/tests.py
> index 8d997c1ae4..5bf8809214 100644
> --- a/bitbake/lib/hashserv/tests.py
> +++ b/bitbake/lib/hashserv/tests.py
> @@ -312,13 +312,11 @@ class HashEquivalenceCommonTests(object):
>          # Check the hash via the read-only server
>          self.assertClientGetHash(ro_client, taskhash, unihash)
>  
> -        # Karim removed : # Ensure that reporting via the read-only server fails
> -        # Karim added : # Ensure that reporting via the read-only server doesn't modify the database
> +        # Ensure that reporting via the read-only server doesn't modify the database
>          taskhash2 = 'c665584ee6817aa99edfc77a44dd853828279370'
>          outhash2 = '3c979c3db45c569f51ab7626a4651074be3a9d11a84b1db076f5b14f7d39db44'
>          unihash2 = '90e9bc1d1f094c51824adca7f8ea79a048d68824'
>  
> -        # Karim removed : with self.assertRaises(ConnectionError):
>          ro_client.report_unihash(taskhash2, self.METHOD, outhash2, unihash2)
>  
>          # Ensure that the database was not modified
> -- 
> 2.25.1
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#15155): https://lists.openembedded.org/g/bitbake-devel/message/15155
> Mute This Topic: https://lists.openembedded.org/mt/101709922/3617179
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


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

* Re: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.
  2023-09-29 12:37 [PATCH] hashserv: Fix read-only mode Karim Ben Houcine
  2023-09-29 13:14 ` Karim Ben Houcine
@ 2023-10-02 20:49 ` Joshua Watt
  2023-10-03  6:56   ` [PATCH v2] " Karim Ben Houcine
  2023-10-03 15:56 ` [bitbake-devel] [PATCH] " Joshua Watt
  2 siblings, 1 reply; 11+ messages in thread
From: Joshua Watt @ 2023-10-02 20:49 UTC (permalink / raw)
  To: Ben Houcine, Karim; +Cc: bitbake-devel

Seems fine to me other than the comment below and the tests being fixed.

On Fri, Sep 29, 2023 at 6:37 AM Ben Houcine, Karim
<Karim.benhoucine@landisgyr.com> wrote:
>
> Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com>
> ---
>  lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
> index d40a2ab8..56d76f1f 100644
> --- a/lib/hashserv/server.py
> +++ b/lib/hashserv/server.py
> @@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>              'get-stats': self.handle_get_stats,
>          })
>
> -        if not read_only:
> +        if read_only:
> +            self.handlers.update({
> +                'report': self.handle_readonly_report,
> +            })
> +        else:
>              self.handlers.update({
>                  'report': self.handle_report,
>                  'report-equiv': self.handle_equivreport,
> @@ -453,6 +457,55 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>
>          self.write_message(d)
>
> +
> +    async def handle_readonly_report(self, data):
> +        with closing(self.db.cursor()) as cursor:
> +            outhash_data = {
> +                'method': data['method'],
> +                'outhash': data['outhash'],
> +                'taskhash': data['taskhash'],
> +                'created': datetime.now()
> +            }
> +
> +            for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):
> +                if k in data:
> +                    outhash_data[k] = data[k]
> +

All the above lines to with .. cursor are unnecessary. They are
constructing the insertion data, which is not being used here.

> +            # Check if outhash is known
> +            cursor.execute(
> +                '''
> +                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2
> +                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash
> +                -- Select any matching output hash
> +                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash
> +                -- Pick the oldest hash
> +                ORDER BY outhashes_v2.created ASC
> +                LIMIT 1
> +                ''',
> +                {
> +                    'method': data['method'],
> +                    'outhash': data['outhash'],
> +                    'taskhash': data['taskhash'],
> +                }
> +            )
> +            row = cursor.fetchone()
> +            if row is not None:
> +                # outhash is known => corrects unihash
> +                unihash = row['unihash']
> +            else:
> +                # outhash is unknown => nothing to do
> +                unihash = outhash_data['taskhash']
> +
> +
> +            d = {
> +                'taskhash': data['taskhash'],
> +                'method': data['method'],
> +                'unihash': unihash,
> +            }
> +
> +        self.write_message(d)
> +
> +
>      async def handle_equivreport(self, data):
>          with closing(self.db.cursor()) as cursor:
>              insert_data = {
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#15149): https://lists.openembedded.org/g/bitbake-devel/message/15149
> Mute This Topic: https://lists.openembedded.org/mt/101656393/3616693
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [JPEWhacker@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>


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

* [PATCH v2] hashserv: Fix read-only mode
  2023-10-02 20:49 ` Joshua Watt
@ 2023-10-03  6:56   ` Karim Ben Houcine
  0 siblings, 0 replies; 11+ messages in thread
From: Karim Ben Houcine @ 2023-10-03  6:56 UTC (permalink / raw)
  To: bitbake-devel; +Cc: Karim Ben Houcine

TCP read-only hash equivalence server is not working: the connection is prematurely closed without even notifying the value of the unihash.
Expected behaviour is:
    1. the client sends a ‘report’ message indicating the 'taskhash', 'method', 'outhash' and a proposed value of 'unihash'.
    2.the server sends back the 'taskhash', 'method' and actual value of 'unihash' to use.
The problem is that in read-only mode, the server rejects 'report' messages (connexion is closed).
hashserv.tests.TestHashEquivalenceUnixServer.test_ro_server test modified accordingly

Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com>
---
 lib/hashserv/server.py | 45 +++++++++++++++++++++++++++++++++++++++++-
 lib/hashserv/tests.py  |  5 ++---
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
index d40a2ab8..1f198af4 100644
--- a/lib/hashserv/server.py
+++ b/lib/hashserv/server.py
@@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
             'get-stats': self.handle_get_stats,
         })
 
-        if not read_only:
+        if read_only:
+            self.handlers.update({
+                'report': self.handle_readonly_report,
+            })
+        else:
             self.handlers.update({
                 'report': self.handle_report,
                 'report-equiv': self.handle_equivreport,
@@ -453,6 +457,45 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
 
         self.write_message(d)
 
+
+    async def handle_readonly_report(self, data):
+        with closing(self.db.cursor()) as cursor:
+
+            # Check if outhash is known
+            cursor.execute(
+                '''
+                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2
+                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash
+                -- Select any matching output hash
+                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash
+                -- Pick the oldest hash
+                ORDER BY outhashes_v2.created ASC
+                LIMIT 1
+                ''',
+                {
+                    'method': data['method'],
+                    'outhash': data['outhash'],
+                    'taskhash': data['taskhash'],
+                }
+            )
+            row = cursor.fetchone()
+            if row is not None:
+                # outhash is known => corrects unihash
+                unihash = row['unihash']
+            else:
+                # outhash is unknown => nothing to do
+                unihash = data['taskhash']
+
+
+            d = {
+                'taskhash': data['taskhash'],
+                'method': data['method'],
+                'unihash': unihash,
+            }
+
+        self.write_message(d)
+
+
     async def handle_equivreport(self, data):
         with closing(self.db.cursor()) as cursor:
             insert_data = {
diff --git a/lib/hashserv/tests.py b/lib/hashserv/tests.py
index f6b85aed..5bf88092 100644
--- a/lib/hashserv/tests.py
+++ b/lib/hashserv/tests.py
@@ -312,13 +312,12 @@ class HashEquivalenceCommonTests(object):
         # Check the hash via the read-only server
         self.assertClientGetHash(ro_client, taskhash, unihash)
 
-        # Ensure that reporting via the read-only server fails
+        # Ensure that reporting via the read-only server doesn't modify the database
         taskhash2 = 'c665584ee6817aa99edfc77a44dd853828279370'
         outhash2 = '3c979c3db45c569f51ab7626a4651074be3a9d11a84b1db076f5b14f7d39db44'
         unihash2 = '90e9bc1d1f094c51824adca7f8ea79a048d68824'
 
-        with self.assertRaises(ConnectionError):
-            ro_client.report_unihash(taskhash2, self.METHOD, outhash2, unihash2)
+        ro_client.report_unihash(taskhash2, self.METHOD, outhash2, unihash2)
 
         # Ensure that the database was not modified
         self.assertClientGetHash(self.client, taskhash2, None)
-- 
2.25.1



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

* Re: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.
  2023-09-29 12:37 [PATCH] hashserv: Fix read-only mode Karim Ben Houcine
  2023-09-29 13:14 ` Karim Ben Houcine
  2023-10-02 20:49 ` Joshua Watt
@ 2023-10-03 15:56 ` Joshua Watt
  2023-10-04  7:31   ` [PATCH v3] " Karim Ben Houcine
  2023-10-05  5:30   ` [bitbake-devel] [PATCH] " Ben Houcine, Karim
  2 siblings, 2 replies; 11+ messages in thread
From: Joshua Watt @ 2023-10-03 15:56 UTC (permalink / raw)
  To: Ben Houcine, Karim; +Cc: bitbake-devel

On Fri, Sep 29, 2023 at 6:37 AM Ben Houcine, Karim
<Karim.benhoucine@landisgyr.com> wrote:
>
> Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com>
> ---
>  lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
> index d40a2ab8..56d76f1f 100644
> --- a/lib/hashserv/server.py
> +++ b/lib/hashserv/server.py
> @@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>              'get-stats': self.handle_get_stats,
>          })
>
> -        if not read_only:
> +        if read_only:
> +            self.handlers.update({
> +                'report': self.handle_readonly_report,
> +            })
> +        else:
>              self.handlers.update({
>                  'report': self.handle_report,
>                  'report-equiv': self.handle_equivreport,
> @@ -453,6 +457,55 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
>
>          self.write_message(d)
>
> +
> +    async def handle_readonly_report(self, data):
> +        with closing(self.db.cursor()) as cursor:
> +            outhash_data = {
> +                'method': data['method'],
> +                'outhash': data['outhash'],
> +                'taskhash': data['taskhash'],
> +                'created': datetime.now()
> +            }
> +
> +            for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):
> +                if k in data:
> +                    outhash_data[k] = data[k]
> +
> +            # Check if outhash is known
> +            cursor.execute(
> +                '''
> +                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2
> +                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash
> +                -- Select any matching output hash
> +                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash
> +                -- Pick the oldest hash
> +                ORDER BY outhashes_v2.created ASC
> +                LIMIT 1
> +                ''',
> +                {
> +                    'method': data['method'],
> +                    'outhash': data['outhash'],
> +                    'taskhash': data['taskhash'],
> +                }
> +            )
> +            row = cursor.fetchone()
> +            if row is not None:
> +                # outhash is known => corrects unihash
> +                unihash = row['unihash']
> +            else:
> +                # outhash is unknown => nothing to do
> +                unihash = outhash_data['taskhash']
> +
> +
> +            d = {
> +                'taskhash': data['taskhash'],
> +                'method': data['method'],
> +                'unihash': unihash,
> +            }

Actually, I looked into this more and the above query is wrong. What
we actually want is to call self.get_unihash() because that's all that
the function would do anyway if the outhash wasn't inserted.
Basically, take the last few lines of handle_report to a new function
that can be called in the "report" RPC call when read-only

> +
> +        self.write_message(d)
> +
> +
>      async def handle_equivreport(self, data):
>          with closing(self.db.cursor()) as cursor:
>              insert_data = {
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#15149): https://lists.openembedded.org/g/bitbake-devel/message/15149
> Mute This Topic: https://lists.openembedded.org/mt/101656393/3616693
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [JPEWhacker@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>


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

* [PATCH v3] hashserv: Fix read-only mode
  2023-10-03 15:56 ` [bitbake-devel] [PATCH] " Joshua Watt
@ 2023-10-04  7:31   ` Karim Ben Houcine
  2023-10-05  5:30   ` [bitbake-devel] [PATCH] " Ben Houcine, Karim
  1 sibling, 0 replies; 11+ messages in thread
From: Karim Ben Houcine @ 2023-10-04  7:31 UTC (permalink / raw)
  To: bitbake-devel; +Cc: Karim Ben Houcine

TCP read-only hash equivalence server is not working: the connection is prematurely closed without even notifying the value of the unihash.
Expected behaviour is:
    1. the client sends a ‘report’ message indicating the 'taskhash', 'method', 'outhash' and a proposed value of 'unihash'.
    2.the server sends back the 'taskhash', 'method' and actual value of 'unihash' to use.
The problem is that in read-only mode, the server rejects 'report' messages (connexion is closed).
hashserv.tests.TestHashEquivalenceUnixServer.test_ro_server test modified accordingly

Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com>

Indeed, using selt.get_unihash() is better, thanks.
---
 lib/hashserv/server.py | 27 ++++++++++++++++++++++++++-
 lib/hashserv/tests.py  |  5 ++---
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py
index d40a2ab8..8d78ba7f 100644
--- a/lib/hashserv/server.py
+++ b/lib/hashserv/server.py
@@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
             'get-stats': self.handle_get_stats,
         })
 
-        if not read_only:
+        if read_only:
+            self.handlers.update({
+                'report': self.handle_readonly_report,
+            })
+        else:
             self.handlers.update({
                 'report': self.handle_report,
                 'report-equiv': self.handle_equivreport,
@@ -453,6 +457,27 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):
 
         self.write_message(d)
 
+
+    async def handle_readonly_report(self, data):
+        with closing(self.db.cursor()) as cursor:
+
+            # Check if outhash is known
+            unihash_data = await self.get_unihash(cursor, data['method'], data['taskhash'])
+            if unihash_data is not None:
+                # outhash is known => retrieve unihash
+                unihash = unihash_data['unihash']
+            else:
+                # outhash is unknown => nothing to do
+                unihash = data['taskhash']
+
+            d = {
+                'taskhash': data['taskhash'],
+                'method': data['method'],
+                'unihash': unihash,
+            }
+
+        self.write_message(d)
+
     async def handle_equivreport(self, data):
         with closing(self.db.cursor()) as cursor:
             insert_data = {
diff --git a/lib/hashserv/tests.py b/lib/hashserv/tests.py
index f6b85aed..5bf88092 100644
--- a/lib/hashserv/tests.py
+++ b/lib/hashserv/tests.py
@@ -312,13 +312,12 @@ class HashEquivalenceCommonTests(object):
         # Check the hash via the read-only server
         self.assertClientGetHash(ro_client, taskhash, unihash)
 
-        # Ensure that reporting via the read-only server fails
+        # Ensure that reporting via the read-only server doesn't modify the database
         taskhash2 = 'c665584ee6817aa99edfc77a44dd853828279370'
         outhash2 = '3c979c3db45c569f51ab7626a4651074be3a9d11a84b1db076f5b14f7d39db44'
         unihash2 = '90e9bc1d1f094c51824adca7f8ea79a048d68824'
 
-        with self.assertRaises(ConnectionError):
-            ro_client.report_unihash(taskhash2, self.METHOD, outhash2, unihash2)
+        ro_client.report_unihash(taskhash2, self.METHOD, outhash2, unihash2)
 
         # Ensure that the database was not modified
         self.assertClientGetHash(self.client, taskhash2, None)
-- 
2.25.1



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

* RE: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.
  2023-10-03 15:56 ` [bitbake-devel] [PATCH] " Joshua Watt
  2023-10-04  7:31   ` [PATCH v3] " Karim Ben Houcine
@ 2023-10-05  5:30   ` Ben Houcine, Karim
  1 sibling, 0 replies; 11+ messages in thread
From: Ben Houcine, Karim @ 2023-10-05  5:30 UTC (permalink / raw)
  To: Joshua Watt; +Cc: bitbake-devel@lists.openembedded.org

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

Hi Joshua,

I’m fixing the test_ro_server test to verify the expected read_only server behaviour.
We want this server to send back the actual unihash corresponding to a given outhash.
The client will then use this unihash to avoid rebuilding packets whose result will not change.
It appears that it is not the case when using self.get_unihash().
Indeed returns the unihash corresponding to a given tashash while we want the unihash for a given outhash.

Maybe I’m misunderstanding the expected behaviour, but using the readonly server as it is implemented today doesn’t seem to prevent useless builds since the client doesn’t know the expected unihash corresponding to the outhash it computed.


Mofified test:


    def test_ro_server(self):
        (ro_client, ro_server) = self.start_server(dbpath=self.server.dbpath, read_only=True)

        # Report a hash via the read-write server
        rw_taskhash = '35788efcb8dfb0a02659d81cf2bfd695fb30faf9'
        outhash = '2765d4a5884be49b28601445c2760c5f21e7e5c0ee2b7e3fce98fd7e5970796f'
        rw_unihash = 'f46d3fbb439bd9b921095da657a4de906510d2cd'

        result = self.client.report_unihash(rw_taskhash, self.METHOD, outhash, rw_unihash)
        self.assertEqual(result['unihash'], rw_unihash, 'Server returned bad unihash')

        # Check the hash via the read-only server
        self.assertClientGetHash(ro_client, rw_taskhash, rw_unihash)

        # Ensure that reporting via the read-only server returns actual unihash without modifying the database
        ro_taskhash = 'c665584ee6817aa99edfc77a44dd853828279370'
        ro_unihash = '90e9bc1d1f094c51824adca7f8ea79a048d68824'

        #Ensure that reporting via the read-only server returns actual unihash
        result = ro_client.report_unihash(ro_taskhash, self.METHOD, outhash, ro_unihash)
        self.assertEqual(result['unihash'], rw_unihash)

        # Ensure that the database was not modified
        self.assertClientGetHash(self.client, ro_taskhash, None)


Best regards,
Karim

De : Joshua Watt <jpewhacker@gmail.com>
Envoyé : mardi 3 octobre 2023 17:57
À : Ben Houcine, Karim <Karim.benhoucine@landisgyr.com>
Cc : bitbake-devel@lists.openembedded.org
Objet : Re: [bitbake-devel] [PATCH] hashserv: Fix read-only mode.

On Fri, Sep 29, 2023 at 6: 37 AM Ben Houcine, Karim <Karim. benhoucine@ landisgyr. com> wrote: > > Signed-off-by: Karim Ben Houcine <karim. benhoucine@ landisgyr. com> > --- > lib/hashserv/server. py | 55 +++++++++++++++++++++++++++++++++++++++++-
ZjQcmQRYFpfptBannerStart
This Message Is From an Untrusted Sender
You have not previously corresponded with this sender.
ZjQcmQRYFpfptBannerEnd

On Fri, Sep 29, 2023 at 6:37 AM Ben Houcine, Karim

<Karim.benhoucine@landisgyr.com<mailto:Karim.benhoucine@landisgyr.com>> wrote:

>

> Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com<mailto:karim.benhoucine@landisgyr.com>>

> ---

>  lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++-

>  1 file changed, 54 insertions(+), 1 deletion(-)

>

> diff --git a/lib/hashserv/server.py b/lib/hashserv/server.py

> index d40a2ab8..56d76f1f 100644

> --- a/lib/hashserv/server.py

> +++ b/lib/hashserv/server.py

> @@ -180,7 +180,11 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):

>              'get-stats': self.handle_get_stats,

>          })

>

> -        if not read_only:

> +        if read_only:

> +            self.handlers.update({

> +                'report': self.handle_readonly_report,

> +            })

> +        else:

>              self.handlers.update({

>                  'report': self.handle_report,

>                  'report-equiv': self.handle_equivreport,

> @@ -453,6 +457,55 @@ class ServerClient(bb.asyncrpc.AsyncServerConnection):

>

>          self.write_message(d)

>

> +

> +    async def handle_readonly_report(self, data):

> +        with closing(self.db.cursor()) as cursor:

> +            outhash_data = {

> +                'method': data['method'],

> +                'outhash': data['outhash'],

> +                'taskhash': data['taskhash'],

> +                'created': datetime.now()

> +            }

> +

> +            for k in ('owner', 'PN', 'PV', 'PR', 'task', 'outhash_siginfo'):

> +                if k in data:

> +                    outhash_data[k] = data[k]

> +

> +            # Check if outhash is known

> +            cursor.execute(

> +                '''

> +                SELECT outhashes_v2.taskhash AS taskhash, unihashes_v2.unihash AS unihash FROM outhashes_v2

> +                INNER JOIN unihashes_v2 ON unihashes_v2.method=outhashes_v2.method AND unihashes_v2.taskhash=outhashes_v2.taskhash

> +                -- Select any matching output hash

> +                WHERE outhashes_v2.method=:method AND outhashes_v2.outhash=:outhash

> +                -- Pick the oldest hash

> +                ORDER BY outhashes_v2.created ASC

> +                LIMIT 1

> +                ''',

> +                {

> +                    'method': data['method'],

> +                    'outhash': data['outhash'],

> +                    'taskhash': data['taskhash'],

> +                }

> +            )

> +            row = cursor.fetchone()

> +            if row is not None:

> +                # outhash is known => corrects unihash

> +                unihash = row['unihash']

> +            else:

> +                # outhash is unknown => nothing to do

> +                unihash = outhash_data['taskhash']

> +

> +

> +            d = {

> +                'taskhash': data['taskhash'],

> +                'method': data['method'],

> +                'unihash': unihash,

> +            }



Actually, I looked into this more and the above query is wrong. What

we actually want is to call self.get_unihash() because that's all that

the function would do anyway if the outhash wasn't inserted.

Basically, take the last few lines of handle_report to a new function

that can be called in the "report" RPC call when read-only



> +

> +        self.write_message(d)

> +

> +

>      async def handle_equivreport(self, data):

>          with closing(self.db.cursor()) as cursor:

>              insert_data = {

> --

> 2.25.1

>

>

> -=-=-=-=-=-=-=-=-=-=-=-

> Links: You receive all messages sent to this group.

> View/Reply Online (#15149): https://urldefense.com/v3/__https://lists.openembedded.org/g/bitbake-devel/message/15149__;!!EEzGOCEfvrF80k8sMoAmtYTD!PmdqyUUAEXVFGrAJuaj5yWSodmOoB3JcX6ObRK4JFWpEW9AS5Z3d1ZHtvvagyaFIcmrz4Hdpgo0WNWpyndE2ckWUkQ4$<https://urldefense.com/v3/__https:/lists.openembedded.org/g/bitbake-devel/message/15149__;!!EEzGOCEfvrF80k8sMoAmtYTD!PmdqyUUAEXVFGrAJuaj5yWSodmOoB3JcX6ObRK4JFWpEW9AS5Z3d1ZHtvvagyaFIcmrz4Hdpgo0WNWpyndE2ckWUkQ4$>

> Mute This Topic: https://urldefense.com/v3/__https://lists.openembedded.org/mt/101656393/3616693__;!!EEzGOCEfvrF80k8sMoAmtYTD!PmdqyUUAEXVFGrAJuaj5yWSodmOoB3JcX6ObRK4JFWpEW9AS5Z3d1ZHtvvagyaFIcmrz4Hdpgo0WNWpyndE2TKc5of4$<https://urldefense.com/v3/__https:/lists.openembedded.org/mt/101656393/3616693__;!!EEzGOCEfvrF80k8sMoAmtYTD!PmdqyUUAEXVFGrAJuaj5yWSodmOoB3JcX6ObRK4JFWpEW9AS5Z3d1ZHtvvagyaFIcmrz4Hdpgo0WNWpyndE2TKc5of4$>

> Group Owner: bitbake-devel+owner@lists.openembedded.org<mailto:bitbake-devel+owner@lists.openembedded.org>

> Unsubscribe: https://urldefense.com/v3/__https://lists.openembedded.org/g/bitbake-devel/unsub__;!!EEzGOCEfvrF80k8sMoAmtYTD!PmdqyUUAEXVFGrAJuaj5yWSodmOoB3JcX6ObRK4JFWpEW9AS5Z3d1ZHtvvagyaFIcmrz4Hdpgo0WNWpyndE2ZErqKS4$<https://urldefense.com/v3/__https:/lists.openembedded.org/g/bitbake-devel/unsub__;!!EEzGOCEfvrF80k8sMoAmtYTD!PmdqyUUAEXVFGrAJuaj5yWSodmOoB3JcX6ObRK4JFWpEW9AS5Z3d1ZHtvvagyaFIcmrz4Hdpgo0WNWpyndE2ZErqKS4$> [JPEWhacker@gmail.com]

> -=-=-=-=-=-=-=-=-=-=-=-

>


P PLEASE CONSIDER OUR ENVIRONMENT BEFORE PRINTING THIS EMAIL.

This e-mail (including any attachments) is confidential and may be legally privileged. If you are not an intended recipient or an authorized representative of an intended recipient, you are prohibited from using, copying or distributing the information in this e-mail or its attachments. If you have received this e-mail in error, please notify the sender immediately by return e-mail and delete all copies of this message and any attachments. Thank you.

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

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

end of thread, other threads:[~2023-10-05  5:31 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-29 12:37 [PATCH] hashserv: Fix read-only mode Karim Ben Houcine
2023-09-29 13:14 ` Karim Ben Houcine
2023-09-30 10:47   ` [bitbake-devel] " Alexandre Belloni
2023-10-02 11:45     ` Ben Houcine, Karim
2023-10-02 18:11       ` Alexandre Belloni
2023-10-02 20:49 ` Joshua Watt
2023-10-03  6:56   ` [PATCH v2] " Karim Ben Houcine
2023-10-03 15:56 ` [bitbake-devel] [PATCH] " Joshua Watt
2023-10-04  7:31   ` [PATCH v3] " Karim Ben Houcine
2023-10-05  5:30   ` [bitbake-devel] [PATCH] " Ben Houcine, Karim
     [not found] <178A47C70519CAC1.9230@lists.openembedded.org>
2023-10-02 11:56 ` Karim Ben Houcine
2023-10-02 18:46   ` [bitbake-devel] " Alexandre Belloni

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.