From 076ba6f0ae31e5e044bcf992e35c02db7747a459 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 7 Jul 2022 12:06:06 -0700 Subject: [PATCH 1/3] Dramatically reduce cache duration for reading statuses I originally set this for a very long timeout because this value should be invalidated when it needs to be by the models, and if that worked perfectly, this would reduce queries dramatically for books that show up in ones feed frequently, but don't change status (for example, a book you read and your friend is currently posting about). In practice, of course, there are errors in invalidating this cache which leave this value appearing extremely broken and it's next to impossible to fix. This change makes each of the timeouts related to reading an hour, which will still give performance benefit when browsing the site (especially for loading the same book multiple times on a page), but resolve naturally if the cache gets into a bad state. --- bookwyrm/templatetags/shelf_tags.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bookwyrm/templatetags/shelf_tags.py b/bookwyrm/templatetags/shelf_tags.py index 0eb1d8118..51e3db8ab 100644 --- a/bookwyrm/templatetags/shelf_tags.py +++ b/bookwyrm/templatetags/shelf_tags.py @@ -17,7 +17,7 @@ def get_is_book_on_shelf(book, shelf): lambda b, s: s.books.filter(id=b.id).exists(), book, shelf, - timeout=15552000, + timeout=60*60, # just cache this for an hour ) @@ -68,7 +68,7 @@ def active_shelf(context, book): ), user, book, - timeout=15552000, + timeout=60*60, ) or {"book": book} @@ -85,5 +85,5 @@ def latest_read_through(book, user): ), user, book, - timeout=15552000, + timeout=60*60, ) From 76c466ee45f481adde7c0e9b284c70f937829459 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 7 Jul 2022 12:21:24 -0700 Subject: [PATCH 2/3] Fixes cache invalidation for editions Your reading status is shown across all editions of a work, so when you change your status in relation to the edition you're currently reading, it needs to invalidate the cached values for all editions of that work. --- bookwyrm/models/shelf.py | 11 +++++++++-- bookwyrm/views/reading.py | 10 +++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py index 3291d5653..b1b99fac1 100644 --- a/bookwyrm/models/shelf.py +++ b/bookwyrm/models/shelf.py @@ -103,12 +103,19 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel): if not self.user: self.user = self.shelf.user if self.id and self.user.local: - cache.delete(f"book-on-shelf-{self.book.id}-{self.shelf.id}") + # remove all caches related to all editions of this book + cache.delete_many([ + f"book-on-shelf-{book.id}-{self.shelf.id}" + for book in self.book.parent_work.editions.all() + ]) super().save(*args, **kwargs) def delete(self, *args, **kwargs): if self.id and self.user.local: - cache.delete(f"book-on-shelf-{self.book.id}-{self.shelf.id}") + cache.delete_many([ + f"book-on-shelf-{book}-{self.shelf.id}" + for book in self.book.parent_work.editions.values_list("id", flat=True) + ]) super().delete(*args, **kwargs) class Meta: diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index c1e6e5955..e168021d2 100644 --- a/bookwyrm/views/reading.py +++ b/bookwyrm/views/reading.py @@ -52,9 +52,6 @@ class ReadingStatus(View): logger.exception("Invalid reading status type: %s", status) return HttpResponseBadRequest() - # invalidate related caches - cache.delete(f"active_shelf-{request.user.id}-{book_id}") - desired_shelf = get_object_or_404( models.Shelf, identifier=identifier, user=request.user ) @@ -65,6 +62,13 @@ class ReadingStatus(View): .get(id=book_id) ) + # invalidate related caches + cache.delete_many([ + f"active_shelf-{request.user.id}-{ed}" + for ed in book.parent_work.editions.values_list("id", flat=True) + ]) + + # gets the first shelf that indicates a reading status, or None shelves = [ s From 742d97b17783e66e2b3d40b8ef0d332ccbc7bec4 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 7 Jul 2022 12:23:10 -0700 Subject: [PATCH 3/3] Python formatting --- bookwyrm/models/shelf.py | 22 ++++++++++++++-------- bookwyrm/templatetags/shelf_tags.py | 6 +++--- bookwyrm/views/reading.py | 11 ++++++----- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py index b1b99fac1..d955e8d07 100644 --- a/bookwyrm/models/shelf.py +++ b/bookwyrm/models/shelf.py @@ -104,18 +104,24 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel): self.user = self.shelf.user if self.id and self.user.local: # remove all caches related to all editions of this book - cache.delete_many([ - f"book-on-shelf-{book.id}-{self.shelf.id}" - for book in self.book.parent_work.editions.all() - ]) + cache.delete_many( + [ + f"book-on-shelf-{book.id}-{self.shelf.id}" + for book in self.book.parent_work.editions.all() + ] + ) super().save(*args, **kwargs) def delete(self, *args, **kwargs): if self.id and self.user.local: - cache.delete_many([ - f"book-on-shelf-{book}-{self.shelf.id}" - for book in self.book.parent_work.editions.values_list("id", flat=True) - ]) + cache.delete_many( + [ + f"book-on-shelf-{book}-{self.shelf.id}" + for book in self.book.parent_work.editions.values_list( + "id", flat=True + ) + ] + ) super().delete(*args, **kwargs) class Meta: diff --git a/bookwyrm/templatetags/shelf_tags.py b/bookwyrm/templatetags/shelf_tags.py index 51e3db8ab..1fb799883 100644 --- a/bookwyrm/templatetags/shelf_tags.py +++ b/bookwyrm/templatetags/shelf_tags.py @@ -17,7 +17,7 @@ def get_is_book_on_shelf(book, shelf): lambda b, s: s.books.filter(id=b.id).exists(), book, shelf, - timeout=60*60, # just cache this for an hour + timeout=60 * 60, # just cache this for an hour ) @@ -68,7 +68,7 @@ def active_shelf(context, book): ), user, book, - timeout=60*60, + timeout=60 * 60, ) or {"book": book} @@ -85,5 +85,5 @@ def latest_read_through(book, user): ), user, book, - timeout=60*60, + timeout=60 * 60, ) diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index e168021d2..eb43e4ea4 100644 --- a/bookwyrm/views/reading.py +++ b/bookwyrm/views/reading.py @@ -63,11 +63,12 @@ class ReadingStatus(View): ) # invalidate related caches - cache.delete_many([ - f"active_shelf-{request.user.id}-{ed}" - for ed in book.parent_work.editions.values_list("id", flat=True) - ]) - + cache.delete_many( + [ + f"active_shelf-{request.user.id}-{ed}" + for ed in book.parent_work.editions.values_list("id", flat=True) + ] + ) # gets the first shelf that indicates a reading status, or None shelves = [