| Message ID | 20230929123706.454384-1-karim.benhoucine@landisgyr.com |
|---|---|
| State | New |
| Headers | show |
| Series | hashserv: Fix read-only mode. | expand |
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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] > -=-=-=-=-=-=-=-=-=-=-=- >
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.
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 = {
Signed-off-by: Karim Ben Houcine <karim.benhoucine@landisgyr.com> --- lib/hashserv/server.py | 55 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-)