twalthr commented on a change in pull request #16150:
URL: https://github.com/apache/flink/pull/16150#discussion_r650716393



##########
File path: 
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -759,31 +791,49 @@ public boolean contains(ConfigOption<?> configOption) {
         }
 
         synchronized (this.confData) {
+            if (canBePrefixMap) {
+                removePrefixMap(this.confData, key);
+            }

Review comment:
       No this is not symmetric, because we cannot control `map.k1 = 1` as we 
have no information about the ConfigOption type. Nevertheless, I would like to 
avoid confusion by having both `map.k1` `map` in the key space at the same time 
if we have the possibility to clean up. The method is consistent with 
`removeConfig`.

##########
File path: 
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -759,31 +791,49 @@ public boolean contains(ConfigOption<?> configOption) {
         }
 
         synchronized (this.confData) {
+            if (canBePrefixMap) {
+                removePrefixMap(this.confData, key);
+            }

Review comment:
       No this is not symmetric, because we cannot control `map.k1 = 1` as we 
have no information about the ConfigOption type. Nevertheless, I would like to 
avoid confusion by having both `map.k1` `map` in the key space at the same time 
if we have the possibility to clean up. The method is consistent with 
`removeConfig` which definitely needs to clean up.




-- 
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.

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


Reply via email to