greenwave

#86 Active cache invalidation
Merged by ralph. Opened by ralph.
cache-invalidation  into  master

Download 86.patch

First, this fixes caching to be testable. The old way didn't work because our cache object was global. It would get configured by the first test and then never be reconfigurable for other tests. This gets fixed here by hanging the cache region on flask.current_app so that it gets reinstantiated and reconfigured for each test run (and each WSGI process).

~~Second, this introduces a CacheInvalidatorExtraordinaire consumer that does the active cache invalidation stuff from #77. So far, only for resultsdb.~~ The cache invalidation now happens as part of the first step in the resultsdb handler, to ensure that cache invalidation is complete before any decision changes are evaluated or published.

2 new commits added

  • A cache invalidator, and tests.
  • Use a file-based cache for the tests... and they work!

Let's proceed without caching waiverdb responses for now.

It's going to be much more complicated.

A natural way to cache the waiverdb responses it to cache the response for a given list of result_ids. This is how greenwave queries today. It has (say) 100 result_ids, and it asks waiverdb for waivers on any of those in a single go. However, when we receive an event from waiverdb about a new waiver - that is in terms of a single result_id. We don't know how to look up all the possible cached values that could include that result_id. Without some super complicated data structure to facilitate this lookup (too complicated), we'll need to reorganize the way we query for waivers (maybe too inefficient?). Let's proceed without it and just cache results for now (and see how it plays).
Edited by mjia

See also https://src.fedoraproject.org/container/greenwave/c/e29f931fb6153af4efdeda2dff74bfd96661d689?branch=master

Let's proceed without caching waiverdb responses for now.
It's going to be much more complicated.
A natural way to cache the waiverdb responses it to cache the response for a given list of result_ids. This is how greenwave queries today. It has (say) 100 result_ids, and it asks waiverdb for waivers on any of those in a single go. However, when we receive an event from waiverdb about a new waiver - that is in terms of a single result_id. We don't know how to look up all the possible cached values that could include that result_id. Without some super complicated data structure to facilitate this lookup (too complicated), we'll need to reorganize the way we query for waivers (maybe too inefficient?). Let's proceed without it and just cache results for now (and see how it plays).

Maybe we could change it to iterate the results and get the waiver for each result? In this case, we can at least cache the waivers. So if a subsequent request is asking the same waiver, we can just return the cached one.

Edited by mjia

I would recommend using https://docs.python.org/2/library/functools.html#functools.wraps

If the item has never been cached before, is it still okay to raise a KeyError here?

In fedmsg, is there any way to config the execution order of the consumers? My concern is if the resultsdb consumer runs first, it will use the cached results and no message would publish if the new result causes any decision change.
Edited by mjia

LGTM except for some questions.

Agreed. Fixed in 4533552.

Unfortunately no, and this is a very good catch.

To fix, I will remove CacheInvalidatorExtraordinaire and moves its functionality into the resultsdb handler so we can ensure that invalidation is done first before decision-change publication.

Hm, no. I was raising the KeyError to catch errors in my key generation logic. My bad.

Fixed in 0403dc9.

2 new commits added

  • Ensure that invalidating an unknown value is sane.
  • Use functools.wraps.

Fixed in cfed4027402f1e91fa93f74c89f5cd78486ecc23.

1 new commit added

  • Move cache invalidation inside the resultsdb handler.

Maybe we could change it to iterate the results and get the waiver for each result? In this case, we can at least cache the waivers. So if a subsequent request is asking the same waiver, we can just return the cached one.

Yeah, maybe. Can we take this on in a separate issue/PR?

rebased onto 53e64758f42893232796abcfaca69bf7b35bcf9f

FYI, this is ready for review again.

Maybe we could change it to iterate the results and get the waiver for each result? In this case, we can at least cache the waivers. So if a subsequent request is asking the same waiver, we can just return the cached one.

Yeah, maybe. Can we take this on in a separate issue/PR?

Absolutely yes.

Aha, I'm a big fan of stone cold steve austin.
https://www.youtube.com/watch?v=gxbAHV6SYmA

:thumbsup:

\ó/

Pull-Request has been merged by ralph
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.