davidradl commented on code in PR #26277:
URL: https://github.com/apache/flink/pull/26277#discussion_r1986265859


##########
flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java:
##########
@@ -459,7 +459,10 @@ public static String[] mergeListsToArray(List<String> 
base, List<String> append)
             ConfigOptions.key("parallelism.default")
                     .intType()
                     .defaultValue(1)
-                    .withDescription("Default parallelism for jobs.");
+                    .withDescription("Default parallelism for jobs. There is 
an exception here. When creating a"

Review Comment:
   I am not sure what the new description is saying:
   - Is the description ever a default of 1? If the default is not always 1 - I 
suggest we consider removing the default though this would have migration 
implications we would need to think through. 
   - the phrase "There is an exception here." I would not use the word 
exception; as this is an overloaded word implying there might be an error of 
some sort.
   - I do not think the sentence "the return value of Runtime. getRuntime. 
availableProcessors()" should be included, it refers to java code, that would 
not be very understandable for someone truing to configure this.  
   
   Please can you verify what cases 1 is the default and when the parallelism 
should be the number of availableProcessors? It would seem to me that using the 
available processors for parallelism would make a lot of sense for all cases - 
if a value has not been supplied. Is this the case?   
   



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