spuru9 commented on code in PR #1207:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/1207#discussion_r3954981106


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/state/ConfigMapStore.java:
##########
@@ -93,9 +102,19 @@ public void removeInfoFromCache(ResourceID resourceID) {
         cache.remove(resourceID);
     }
 
+    private static boolean isNotFound(Exception e) {
+        return e instanceof KubernetesClientException
+                && ((KubernetesClientException) e).getCode() == 
HttpURLConnection.HTTP_NOT_FOUND;
+    }
+
     private ConfigMapView getConfigMap(KubernetesJobAutoScalerContext 
jobContext) {
-        return cache.computeIfAbsent(
-                jobContext.getJobKey(), (id) -> 
getConfigMapFromKubernetes(jobContext));
+        var ownerUid = jobContext.getResource().getMetadata().getUid();
+        return cache.compute(

Review Comment:
   major: cache.compute re-checks the owner UID on every call, not just once. 
If the old CM hasn't been GC'd yet after a recreate, every getConfigMap() call 
in the same reconcile sees a mismatch and re-fetches from k8s, discarding 
whatever was put() earlier in that same pass (since flush() only runs once at 
the end). E.g. put("x", ...) then put("y", ...) before the recreate's old CM is 
gone → x is silently lost. 
   
   Might need the "is this cache entry stale" decision made once per 
ConfigMapView, not re-derived from the live owner ref on every access.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to