[GitHub] [flink] xintongsong commented on a change in pull request #11176: [FLINK-15948][yarn] Enrich the warning log for YARN minimum allocation memory

[GitBox <gi...@apache.org>]

xintongsong commented on a change in pull request #11176: [FLINK-15948][yarn] Enrich the warning log for YARN minimum allocation memory
URL: https://github.com/apache/flink/pull/11176#discussion_r383064789
 
 

 ##########
 File path: flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java
 ##########
 @@ -521,11 +521,18 @@ private ClusterSpecification validateClusterResources(
 		int jobManagerMemoryMb = clusterSpecification.getMasterMemoryMB();
 		final int taskManagerMemoryMb = clusterSpecification.getTaskManagerMemoryMB();
 
-		if (jobManagerMemoryMb < yarnMinAllocationMB || taskManagerMemoryMb < yarnMinAllocationMB) {
-			LOG.warn("The JobManager or TaskManager memory is below the smallest possible YARN Container size. "
-					+ "The value of 'yarn.scheduler.minimum-allocation-mb' is '" + yarnMinAllocationMB + "'. Please increase the memory size." +
-					"YARN will allocate the smaller containers but the scheduler will account for the minimum-allocation-mb, maybe not all instances " +
-					"you requested will start.");
+		final List<String> noteMem = new ArrayList<>();
+		if (jobManagerMemoryMb % yarnMinAllocationMB != 0 || jobManagerMemoryMb == 0) {
+			noteMem.add("JobManager memory(" + jobManagerMemoryMb + ")");
+		}
+		if (taskManagerMemoryMb % yarnMinAllocationMB != 0 || taskManagerMemoryMb == 0) {
+			noteMem.add("TaskManager memory(" + taskManagerMemoryMb + ")");
+		}
+		if (noteMem.size() > 0) {
+			LOG.warn("The {} is not a multiple of YARN minimum allocation memory({}), so some extra memory will be wasted. "
+				+ "Because YARN will always normalize the resource request by insuring that the requested memory is a multiple "
+				+ "of minimum allocation. The value of 'yarn.scheduler.minimum-allocation-mb' is '" + yarnMinAllocationMB + "'. "
+				+ "Please increase the specified memory size.", noteMem.toString(), yarnMinAllocationMB);
 
 Review comment:
   I would suggest to have a more concise log message. The current one mentions "multiple of YARN min allocation" and the configured value of yarnMinAllocationMB twice each, which is not necessary.
   
   I would suggest the following:
   `The configured {} memory is {} MB. YARN will allocate {} MB to make up an integer multiple of its minimum allocation memory ({} MB, configured via 'yarn.scheduler.minimum-allocation-mb'). The extra {} MB may not be used by Flink.`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services