You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by qmlmoon <gi...@git.apache.org> on 2014/12/02 10:42:46 UTC

[GitHub] incubator-flink pull request: [FLINK-938] Auomatically configure t...

GitHub user qmlmoon opened a pull request:

    https://github.com/apache/incubator-flink/pull/248

    [FLINK-938] Auomatically configure the jobmanager address

    This PR is an alternativ approach to #48 without touching the config file.  Once this is merged, I'll close #48

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/qmlmoon/incubator-flink hostname2

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-flink/pull/248.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #248
    
----
commit 6dffd0855e51ddae3a1851cc8752521ac3d41b27
Author: mingliang <qm...@gmail.com>
Date:   2014-12-02T09:37:05Z

    [FLINK-938] Auomatically configure the jobmanager address

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-938] Auomatically configure t...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/incubator-flink/pull/248#discussion_r21429930
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobmanager/JobManager.java ---
    @@ -786,6 +786,13 @@ public static JobManager initialize(String[] args) throws Exception {
     		
     		// First, try to load global configuration
     		GlobalConfiguration.loadConfiguration(configDir);
    +		// The configuretion does not contain a jobmanager address
    +		if (GlobalConfiguration.getString(ConfigConstants.JOB_MANAGER_IPC_ADDRESS_KEY, null) == null) {
    +			Configuration c = GlobalConfiguration.getConfiguration();
    +			c.setString(ConfigConstants.JOB_MANAGER_IPC_ADDRESS_KEY, InetAddress.getLocalHost().getHostName());
    +			LOG.info("Setting jobmanager rpc address to " + InetAddress.getLocalHost().getHostName());
    +			GlobalConfiguration.includeConfiguration(c);
    --- End diff --
    
    I am still not a big fan of manipulating the global configuration. That is an artifact of the singleton pattern that (which I think we should reduce and get rid of eventually). I would pass the hostname as a proper parameter, not through the config.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-938] Auomatically configure t...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:

    https://github.com/apache/incubator-flink/pull/248#discussion_r21152078
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/TaskManager.java ---
    @@ -1057,12 +1057,17 @@ public static void main(String[] args) throws IOException {
     		Option tempDir = OptionBuilder.withArgName("temporary directory (overwrites configured option)")
     				.hasArg().withDescription(
     				"Specify temporary directory.").create(ARG_CONF_DIR);
    +		Option defaultJobManagerAddressOpt = OptionBuilder.withArgName("default jobmanager rpc address")
    +				.hasArg().withDescription(
    +				"Speicify the default jobmanager rpc address.").create("defaultJobManagerAdd");
    --- End diff --
    
    Typo inf "Speicify"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-938] Auomatically configure t...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/incubator-flink/pull/248#discussion_r21429905
  
    --- Diff: flink-dist/src/main/flink-bin/conf/flink-conf.yaml ---
    @@ -21,7 +21,10 @@
     # Common
     #==============================================================================
     
    -jobmanager.rpc.address: localhost
    +# The jobmanager rpc address will be automatically set to hostname
    --- End diff --
    
    How about "By default, the JobManager addredd will be set to the host from which the start-cluster or the start-local script is called.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-flink pull request: [FLINK-938] Auomatically configure t...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/incubator-flink/pull/248#discussion_r21429934
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/TaskManager.java ---
    @@ -1075,9 +1080,19 @@ public static void main(String[] args) throws IOException {
     
     		String configDir = line.getOptionValue(configDirOpt.getOpt(), null);
     		String tempDirVal = line.getOptionValue(tempDir.getOpt(), null);
    +		String jobmanagerAdd = line.getOptionValue(defaultJobManagerAddressOpt.getOpt(), null);
     
     		// First, try to load global configuration
     		GlobalConfiguration.loadConfiguration(configDir);
    +
    +		// The configuretion does not contain a jobmanager address
    +		if (GlobalConfiguration.getString(ConfigConstants.JOB_MANAGER_IPC_ADDRESS_KEY, null) == null) {
    +			Configuration c = GlobalConfiguration.getConfiguration();
    +			c.setString(ConfigConstants.JOB_MANAGER_IPC_ADDRESS_KEY, jobmanagerAdd);
    +			LOG.info("Setting jobmanager rpc address to " + jobmanagerAdd);
    +			GlobalConfiguration.includeConfiguration(c);
    --- End diff --
    
    same here, see above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---