removing tombstones

Author: shawkins

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java

@@ -220,6 +220,11 @@ public Optional<R> get(ResourceID resourceID) {
                     ? controllerConfiguration.getConfigurationService().getResourceCloner().clone(r)
                     : r);
   }
+  
+  public Optional<String> getLastSyncResourceVersion(Optional<String> namespace) {
+    return getSource(namespace.orElse(WATCH_ALL_NAMESPACES))
+        .map(source -> source.getLastSyncResourceVersion());
+  }
 
   @Override
   public Stream<ResourceID> keys() {

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java

@@ -155,6 +155,10 @@ public void stop() throws OperatorException {
   public Optional<T> get(ResourceID resourceID) {
     return Optional.ofNullable(cache.getByKey(getKey(resourceID)));
   }
+  
+  public String getLastSyncResourceVersion() {
+    return this.informer.lastSyncResourceVersion();
+  }
 
   private String getKey(ResourceID resourceID) {
     return Cache.namespaceKeyFunc(resourceID.getNamespace().orElse(null), resourceID.getName());

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java

@@ -147,7 +147,11 @@ public Optional<R> get(ResourceID resourceID) {
       return res;
     }
   }
-
+  
+  public Optional<String> getLastSyncResourceVersion(Optional<String> namespace) {
+    return cache.getLastSyncResourceVersion(namespace);
+  }
+  
   @SuppressWarnings("unused")
   public Optional<R> getCachedValue(ResourceID resourceID) {
     return get(resourceID);

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java

@@ -15,7 +15,6 @@
  */
 package io.javaoperatorsdk.operator.processing.event.source.informer;
 
-import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
@@ -46,53 +45,11 @@
  */
 public class TemporaryResourceCache<T extends HasMetadata> {
 
-  static class ExpirationCache<K> {
-    private final LinkedHashMap<K, Long> cache;
-    private final int ttlMs;
-
-    public ExpirationCache(int maxEntries, int ttlMs) {
-      this.ttlMs = ttlMs;
-      this.cache =
-          new LinkedHashMap<>() {
-            @Override
-            protected boolean removeEldestEntry(Map.Entry<K, Long> eldest) {
-              return size() > maxEntries;
-            }
-          };
-    }
-
-    public void add(K key) {
-      clean();
-      cache.putIfAbsent(key, System.currentTimeMillis());
-    }
-
-    public boolean contains(K key) {
-      clean();
-      return cache.get(key) != null;
-    }
-
-    void clean() {
-      if (!cache.isEmpty()) {
-        long currentTimeMillis = System.currentTimeMillis();
-        var iter = cache.entrySet().iterator();
-        // the order will already be from oldest to newest, clean a fixed number of entries to
-        // amortize the cost amongst multiple calls
-        for (int i = 0; i < 10 && iter.hasNext(); i++) {
-          var entry = iter.next();
-          if (currentTimeMillis - entry.getValue() > ttlMs) {
-            iter.remove();
-          }
-        }
-      }
-    }
-  }
-
   private static final Logger log = LoggerFactory.getLogger(TemporaryResourceCache.class);
 
   private final Map<ResourceID, T> cache = new ConcurrentHashMap<>();
 
   // keep up to the last million deletions for up to 10 minutes
-  private final ExpirationCache<String> tombstones = new ExpirationCache<>(1000000, 1200000);
   private final ManagedInformerEventSource<T, ?, ?> managedInformerEventSource;
   private final boolean parseResourceVersions;
 
@@ -104,7 +61,6 @@ public TemporaryResourceCache(
   }
 
   public synchronized void onDeleteEvent(T resource, boolean unknownState) {
-    tombstones.add(resource.getMetadata().getUid());
     onEvent(resource, unknownState);
   }
 
@@ -131,17 +87,21 @@ public synchronized void putAddedResource(T newResource) {
    */
   public synchronized void putResource(T newResource, String previousResourceVersion) {
     var resourceId = ResourceID.fromResource(newResource);
+
     var cachedResource = managedInformerEventSource.get(resourceId).orElse(null);
 
     boolean moveAhead = false;
+    String latest = managedInformerEventSource.getLastSyncResourceVersion(resourceId.getNamespace()).orElse(null);
+    if (latest != null && latest > newResource.getMetadata().getResourceVersion()) {
+      log.debug(
+          "Won't move backwards for {}: resourceVersion {}, latest {}",
+          resourceId,
+          newResource.getMetadata().getResourceVersion(),
+          latest);
+      return;
+    }
+      
     if (previousResourceVersion == null && cachedResource == null) {
-      if (tombstones.contains(newResource.getMetadata().getUid())) {
-        log.debug(
-            "Won't resurrect uid {} for resource id: {}",
-            newResource.getMetadata().getUid(),
-            resourceId);
-        return;
-      }
       // we can skip further checks as this is a simple add and there's no previous entry to
       // consider
       moveAhead = true;