Check the stream position before checking if the cache is empty. (#14639)

An empty cache does not mean the entity has no changed, if
it is earlier than the earliest known stream position return that
the entity *has* changed since the cache cannot accurately
answer that query.
This commit is contained in:
Patrick Cloke 2022-12-08 11:35:49 -05:00 committed by GitHub
parent f3ad68c343
commit da77720752
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 10 additions and 7 deletions

1
changelog.d/14639.bugfix Normal file
View file

@ -0,0 +1 @@
Fix a long-standing bug where the user directory and room/user stats might be out of sync.

View file

@ -213,16 +213,17 @@ class StreamChangeCache:
"""
assert isinstance(stream_pos, int)
if not self._cache:
# If the cache is empty, nothing can have changed.
return False
# _cache is not valid at or before the earliest known stream position, so
# return that an entity has changed.
if stream_pos <= self._earliest_known_stream_pos:
self.metrics.inc_misses()
return True
# If the cache is empty, nothing can have changed.
if not self._cache:
self.metrics.inc_misses()
return False
self.metrics.inc_hits()
return stream_pos < self._cache.peekitem()[0]

View file

@ -144,9 +144,10 @@ class StreamChangeCacheTests(unittest.HomeserverTestCase):
"""
cache = StreamChangeCache("#test", 1)
# With no entities, it returns False for the past, present, and future.
self.assertFalse(cache.has_any_entity_changed(0))
self.assertFalse(cache.has_any_entity_changed(1))
# With no entities, it returns True for the past, present, and False for
# the future.
self.assertTrue(cache.has_any_entity_changed(0))
self.assertTrue(cache.has_any_entity_changed(1))
self.assertFalse(cache.has_any_entity_changed(2))
# We add an entity