[v1] testing: Fix invalidation of prefetch cache upon set_property

[Patchew-devel] [PATCH] testing: Fix invalidation of prefetch cache upon set_property

Posted by Fam Zheng 5 years ago

"testing.ready" didn't work due to the stale cache of
Message._properties, since Message._properties cannot reflect the
.set_property() updates.

Fix it with a harder invalidating code path.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 api/models.py         | 14 ++++++++++----
 tests/test_testing.py |  3 +++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/api/models.py b/api/models.py
index de01a61..a672bd4 100644
--- a/api/models.py
+++ b/api/models.py
@@ -408,9 +408,15 @@ class Message(models.Model):
 
     def get_properties(self):
         if hasattr(self, '_properties'):
-            return self._properties
+            if self._properties is not None:
+                return self._properties
+            else:
+                # The prefetch cache is invalidated, query again
+                all_props = MessageProperty.objects.filter(message=self)
+        else:
+            all_props = self.properties.all()
         r = {}
-        for m in self.properties.all():
+        for m in all_props:
             if m.blob:
                 r[m.name] = load_blob_json(m.value)
             else:
@@ -434,8 +440,8 @@ class Message(models.Model):
         mp.value = value
         mp.blob = blob
         mp.save()
-        if hasattr(self, '_properties'):
-            del(self._properties)
+        # Invalidate cache
+        self._properties = None
 
     def set_property(self, prop, value):
         old_val = self.get_property(prop)
diff --git a/tests/test_testing.py b/tests/test_testing.py
index 0897bbc..dcb3dbc 100755
--- a/tests/test_testing.py
+++ b/tests/test_testing.py
@@ -43,6 +43,9 @@ class TestingTest(PatchewTestCase):
         self.msg.set_property("git.tag", "dummy tag")
         self.msg.set_property("git.base", "dummy base")
 
+    def test_testing_ready(self):
+        self.assertTrue(self.msg.get_property("testing.ready", True))
+
     def msg_testing_done(self, log=None, **report):
         if not 'passed' in report:
             report['passed'] = True
-- 
2.14.3

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel

Re: [Patchew-devel] [PATCH] testing: Fix invalidation of prefetch cache upon set_property

Posted by Paolo Bonzini 5 years ago

On 21/03/2018 15:07, Fam Zheng wrote:
> "testing.ready" didn't work due to the stale cache of
> Message._properties, since Message._properties cannot reflect the
> .set_property() updates.
> 
> Fix it with a harder invalidating code path.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

I don't really understand the code, but new tests are always good.  Go
ahead. :)

Paolo

> ---
>  api/models.py         | 14 ++++++++++----
>  tests/test_testing.py |  3 +++
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/api/models.py b/api/models.py
> index de01a61..a672bd4 100644
> --- a/api/models.py
> +++ b/api/models.py
> @@ -408,9 +408,15 @@ class Message(models.Model):
>  
>      def get_properties(self):
>          if hasattr(self, '_properties'):
> -            return self._properties
> +            if self._properties is not None:
> +                return self._properties
> +            else:
> +                # The prefetch cache is invalidated, query again
> +                all_props = MessageProperty.objects.filter(message=self)
> +        else:
> +            all_props = self.properties.all()
>          r = {}
> -        for m in self.properties.all():
> +        for m in all_props:
>              if m.blob:
>                  r[m.name] = load_blob_json(m.value)
>              else:
> @@ -434,8 +440,8 @@ class Message(models.Model):
>          mp.value = value
>          mp.blob = blob
>          mp.save()
> -        if hasattr(self, '_properties'):
> -            del(self._properties)
> +        # Invalidate cache
> +        self._properties = None
>  
>      def set_property(self, prop, value):
>          old_val = self.get_property(prop)
> diff --git a/tests/test_testing.py b/tests/test_testing.py
> index 0897bbc..dcb3dbc 100755
> --- a/tests/test_testing.py
> +++ b/tests/test_testing.py
> @@ -43,6 +43,9 @@ class TestingTest(PatchewTestCase):
>          self.msg.set_property("git.tag", "dummy tag")
>          self.msg.set_property("git.base", "dummy base")
>  
> +    def test_testing_ready(self):
> +        self.assertTrue(self.msg.get_property("testing.ready", True))
> +
>      def msg_testing_done(self, log=None, **report):
>          if not 'passed' in report:
>              report['passed'] = True
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel

Re: [Patchew-devel] [PATCH] testing: Fix invalidation of prefetch cache upon set_property

Posted by Fam Zheng 5 years ago

On Wed, 03/21 16:24, Paolo Bonzini wrote:
> On 21/03/2018 15:07, Fam Zheng wrote:
> > "testing.ready" didn't work due to the stale cache of
> > Message._properties, since Message._properties cannot reflect the
> > .set_property() updates.
> > 
> > Fix it with a harder invalidating code path.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> I don't really understand the code, but new tests are always good.  Go
> ahead. :)

The added test is not good. I'll send v2.

Fam

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel