flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dawidwys <...@git.apache.org>
Subject [GitHub] flink pull request #6297: [FLINK-9777] YARN: JM and TM Memory must be specif...
Date Thu, 12 Jul 2018 15:47:52 GMT
Github user dawidwys commented on a diff in the pull request:

    https://github.com/apache/flink/pull/6297#discussion_r202085772
  
    --- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java
---
    @@ -386,10 +386,10 @@ private ClusterSpecification createClusterSpecification(Configuration
configurat
     		}
     
     		// JobManager Memory
    -		final int jobManagerMemoryMB = MemorySize.parse(configuration.getString(JobManagerOptions.JOB_MANAGER_HEAP_MEMORY)).getMebiBytes();
    +		final int jobManagerMemoryMB = MemorySize.parse(configuration.getString(JobManagerOptions.JOB_MANAGER_HEAP_MEMORY),
MemorySize.MemoryUnit.MEGA_BYTES).getMebiBytes();
    --- End diff --
    
    I think we cannot do this as users might not use the `config.sh`. At the same time we
think we really should keep backwards compatibility, as otherwise there might be complaints.
 We've discussed this with @GJL offline and we think the way to go right now would be to check
everywhere we use the new option to check for the old one as well if the new one was not set.
    
    That is:
    * `org.apache.flink.client.deployment.ClusterSpecification#fromConfiguration`
    * `org.apache.flink.yarn.YarnResourceManager#YarnResourceManager`
    * `org.apache.flink.yarn.cli.FlinkYarnSessionCli#createClusterSpecification`
    
    Probably we should wrap it in some utility method.
    
    Also for the yarn command line we should keep the old behaviour. This means we should
add "m" suffix in `org.apache.flink.yarn.cli.FlinkYarnSessionCli#applyCommandLineOptionsToConfiguration`


---

Mime
View raw message