From 4ed9b4de4e3773ad7aa89be4af75bea72e807151 Mon Sep 17 00:00:00 2001 From: "Keith R. Gustafson" Date: Thu, 20 Apr 2023 13:53:30 -0700 Subject: [PATCH] Fix deficiencies in the LruSqlEntity relation There were problems that arose when running in a multi-instance environment. Fixes: - Override additional methods so distribution listeners are notified. - Have reset(Class type) check to ensure the type matches. - Invalidate LRU cache in add/addAll as explained by the inline comment. --- .../cache/LruSqlEntityRelation.java | 97 ++++++++++++++++++- 1 file changed, 93 insertions(+), 4 deletions(-) diff --git a/gemini/src/main/java/com/techempower/cache/LruSqlEntityRelation.java b/gemini/src/main/java/com/techempower/cache/LruSqlEntityRelation.java index 38c82bd..ccf451c 100644 --- a/gemini/src/main/java/com/techempower/cache/LruSqlEntityRelation.java +++ b/gemini/src/main/java/com/techempower/cache/LruSqlEntityRelation.java @@ -80,6 +80,15 @@ public Set leftIDs(long rightID) { } } + /** + * Override to call our version that updates listeners as well. + */ + @Override + public boolean remove(long leftID, long rightID) + { + return remove(leftID, rightID, true, true, true); + } + @Override public boolean remove(long leftID, long rightID, boolean updateDatabase, boolean notifyListeners, boolean notifyDistributionListeners) { @@ -132,6 +141,15 @@ public boolean removeLeftValue(long leftID, boolean updateDatabase, boolean noti return toReturn; } + /** + * Override to call our version that updates listeners as well. + */ + @Override + public boolean removeRightValue(long rightID) + { + return removeRightValue(rightID, true, true, true); + } + @Override public boolean removeRightValue(long rightID, boolean updateDatabase, boolean notifyListeners, boolean notifyDistributionListeners) { @@ -162,6 +180,15 @@ public boolean removeRightValue(long rightID, boolean updateDatabase, boolean no return toReturn; } + /** + * Override to call our version that updates listeners as well. + */ + @Override + public boolean replaceAll(LongRelation relationToReplace) + { + return replaceAll(relationToReplace, true, true, true); + } + @Override public boolean replaceAll(LongRelation relationToReplace, boolean updateDatabase, boolean notifyListeners, boolean notifyDistributionListeners) { @@ -199,12 +226,19 @@ public void reset(boolean notifyListeners, boolean notifyDistributionListeners) @Override public void reset(Class type, boolean notifyListeners, boolean notifyDistributionListeners) { - reset(notifyListeners, notifyDistributionListeners); + if (type.equals(this.leftType()) + || type.equals(this.rightType())) + { + reset(notifyListeners, notifyDistributionListeners); + } } + /** + * Override to call our version that updates listeners as well. + */ @Override public void reset(Class type) { - reset(true, true); + reset(type, true, true); } @Override @@ -227,6 +261,15 @@ public void setId(long identity) { this.id = identity; } + /** + * Override to call our version that updates listeners as well. + */ + @Override + public boolean removeAll(LongRelation relationToRemove) + { + return removeAll(relationToRemove, true, true, true); + } + @Override public boolean removeAll(LongRelation relationToRemove, boolean updateDatabase, boolean notifyListeners, boolean notifyDistributionListeners) { @@ -247,6 +290,15 @@ public boolean removeAll(LongRelation relationToRemove, boolean updateDatabase, return toReturn; } + /** + * Override to call our version that updates listeners as well. + */ + @Override + public void clear() + { + clear(true, true, true); + } + @Override public void clear(boolean updateDatabase, boolean notifyListeners, boolean notifyDistributionListeners) { @@ -265,13 +317,30 @@ public void clear(boolean updateDatabase, boolean notifyListeners, } } + /** + * Override to call our version that updates listeners as well. + */ + @Override + public boolean addAll(LongRelation relationToAdd) + { + return addAll(relationToAdd, true, true, true); + } + @Override public boolean addAll(LongRelation relationToAdd, boolean updateDatabase, boolean notifyListeners, boolean notifyDistributionListeners) { if (relationToAdd == null) { return false; } - // Do not update our LRU cache. Wait until a relationship is requested before caching it. + // Do not add these new relations to our LRU cache because we cannot know if they are complete + // without checking the database. We need to avoid a situation where our LRU cache contains an + // incomplete mapping (e.g., only caches the mapping [A,B] when the database also contains + // [A,C]) because then our LRU cache would confidently return wrong answers. Rather than the + // complexity and performance hit of checking the database and loading the LRU cache with + // mappings that may or may not be used, we'll simply invalidate the LRU cache. + this.leftMap.invalidateAll(); + this.rightMap.invalidateAll(); + boolean toReturn = false; // Not important for this to be strictly accurate. if (updateDatabase) { toReturn = super.addAll(relationToAdd); @@ -286,10 +355,30 @@ public boolean addAll(LongRelation relationToAdd, boolean updateDatabase, boolea return toReturn; } + /** + * Override to call our version that updates listeners as well. + */ + @Override + public boolean add(long leftID, long rightID) + { + return add(leftID, rightID, true, true, true); + } + @Override public boolean add(long leftID, long rightID, boolean updateDatabase, boolean notifyListeners, boolean notifyDistributionListeners) { - // Do not update our LRU cache. Wait until a relationship is requested before caching it. + // Do not add these new relations to our LRU cache because we cannot know if they are complete + // without checking the database. We need to avoid a situation where our LRU cache contains an + // incomplete mapping (e.g., only caches the mapping [A,B] when the database also contains + // [A,C]) because then our LRU cache would confidently return wrong answers. Rather than the + // complexity and performance hit of checking the database and loading the LRU cache with + // mappings that may or may not be used, we'll simply invalidate the LRU cache. + + // This is potentially invalidating more than required (in the case of many-to-many relations) + // but it's not worth the complexity to be exact about these invalidations. + this.leftMap.invalidate(leftID); + this.rightMap.invalidate(rightID); + boolean toReturn = false; // Not important for this to be strictly accurate. if (updateDatabase) { toReturn = super.add(leftID, rightID);