You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by davidyan74 <gi...@git.apache.org> on 2016/04/05 01:32:04 UTC

[GitHub] incubator-apex-core pull request: APEXCORE-411 support restarting ...

GitHub user davidyan74 opened a pull request:

    https://github.com/apache/incubator-apex-core/pull/294

    APEXCORE-411 support restarting app without app package or jar files

    @tweise please review

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

    $ git pull https://github.com/davidyan74/incubator-apex-core APEXCORE-411

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

    https://github.com/apache/incubator-apex-core/pull/294.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 #294
    
----
commit 204c2b3b09fc42d3d34c967d7f8e7057c0061f61
Author: David Yan <da...@datatorrent.com>
Date:   2016-04-04T22:56:53Z

    APEXCORE-411 support restarting app without app package or jar files

----


---
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-apex-core pull request: APEXCORE-411 support restarting ...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/294#issuecomment-207630632
  
    @tweise explanation is added in the comment.


---
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-apex-core pull request: APEXCORE-411 support restarting ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-apex-core/pull/294


---
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-apex-core pull request: APEXCORE-411 support restarting ...

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

    https://github.com/apache/incubator-apex-core/pull/294#discussion_r59096681
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/client/StramAppLauncher.java ---
    @@ -210,14 +239,67 @@ public StramAppLauncher(FileSystem fs, Path path, Configuration conf) throws Exc
       public StramAppLauncher(String name, Configuration conf) throws Exception
       {
         this.propertiesBuilder = new LogicalPlanConfiguration(conf);
    +    this.conf = conf;
         init(name);
       }
     
    +  public StramAppLauncher(FileSystem fs, Configuration conf) throws Exception
    +  {
    +    this.propertiesBuilder = new LogicalPlanConfiguration(conf);
    +    this.fs = fs;
    +    this.conf = conf;
    +    init();
    +  }
    +
       public String getMvnBuildClasspathOutput()
       {
         return mvnBuildClasspathOutput.toString();
       }
     
    +  /**
    +   * This is for recovering an app without specifying apa or appjar file
    +   * @throws Exception
    +   */
    +  private void init() throws Exception
    +  {
    +    String originalAppId = propertiesBuilder.conf.get(ORIGINAL_APP_ID);
    +    if (originalAppId == null) {
    +      throw new AssertionError("Need original app id if launching without apa or appjar");
    +    }
    +    Path appsBasePath = new Path(StramClientUtils.getDTDFSRootDir(fs, conf), StramClientUtils.SUBDIR_APPS);
    +    Path origAppPath = new Path(appsBasePath, originalAppId);
    +    StringWriter writer = new StringWriter();
    +    try (FSDataInputStream in = fs.open(new Path(origAppPath, "meta.json"))) {
    +      IOUtils.copy(in, writer);
    +    }
    +    JSONObject metaJson = new JSONObject(writer.toString());
    +    String originalLibJars = null;
    +    try {
    +      JSONObject attributes = metaJson.getJSONObject("attributes");
    +      originalLibJars = attributes.getString(LogicalPlan.LIBRARY_JARS.getSimpleName());
    +      recoveryAppName = attributes.getString(Context.DAGContext.APPLICATION_NAME.getSimpleName());
    +    } catch (JSONException ex) {
    +      // ignore
    +    }
    +
    +    LinkedHashSet<URL> clUrls = new LinkedHashSet<>();
    --- End diff --
    
    Should just add a comment that this is necessary to construct the class loader because during launch it needs to update the serialized state with new app id. This may become unnecessary if we don't relay on object serialization for this in the future. 


---
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-apex-core pull request: APEXCORE-411 support restarting ...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 closed the pull request at:

    https://github.com/apache/incubator-apex-core/pull/294


---
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-apex-core pull request: APEXCORE-411 support restarting ...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 closed the pull request at:

    https://github.com/apache/incubator-apex-core/pull/294


---
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-apex-core pull request: APEXCORE-411 support restarting ...

Posted by davidyan74 <gi...@git.apache.org>.
GitHub user davidyan74 reopened a pull request:

    https://github.com/apache/incubator-apex-core/pull/294

    APEXCORE-411 support restarting app without app package or jar files

    @tweise please review

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

    $ git pull https://github.com/davidyan74/incubator-apex-core APEXCORE-411

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

    https://github.com/apache/incubator-apex-core/pull/294.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 #294
    
----
commit 77f1458a3d1a9299897f1bd2ae01dce15b0a7985
Author: David Yan <da...@datatorrent.com>
Date:   2016-04-04T22:56:53Z

    APEXCORE-411 support restarting app without app package or jar files

----


---
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-apex-core pull request: APEXCORE-411 support restarting ...

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

    https://github.com/apache/incubator-apex-core/pull/294#discussion_r59095579
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/client/StramAppLauncher.java ---
    @@ -203,21 +231,75 @@ public StramAppLauncher(FileSystem fs, Path path, Configuration conf) throws Exc
         this.fs = fs;
         fs.copyToLocalFile(path, new Path(localJarFile.getAbsolutePath()));
         this.jarFile = localJarFile;
    +    this.conf = conf;
         this.propertiesBuilder = new LogicalPlanConfiguration(conf);
         init(this.jarFile.getName());
       }
     
       public StramAppLauncher(String name, Configuration conf) throws Exception
       {
         this.propertiesBuilder = new LogicalPlanConfiguration(conf);
    +    this.conf = conf;
         init(name);
       }
     
    +  public StramAppLauncher(FileSystem fs, Configuration conf) throws Exception
    +  {
    +    this.propertiesBuilder = new LogicalPlanConfiguration(conf);
    +    this.fs = fs;
    +    this.conf = conf;
    +    init();
    +  }
    +
       public String getMvnBuildClasspathOutput()
       {
         return mvnBuildClasspathOutput.toString();
       }
     
    +  /**
    +   * This is for recovering an app without specifying apa or appjar file
    --- End diff --
    
    Might be better to move the comment on the constructor?


---
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-apex-core pull request: APEXCORE-411 support restarting ...

Posted by davidyan74 <gi...@git.apache.org>.
GitHub user davidyan74 reopened a pull request:

    https://github.com/apache/incubator-apex-core/pull/294

    APEXCORE-411 support restarting app without app package or jar files

    @tweise please review

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

    $ git pull https://github.com/davidyan74/incubator-apex-core APEXCORE-411

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

    https://github.com/apache/incubator-apex-core/pull/294.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 #294
    
----
commit 77f1458a3d1a9299897f1bd2ae01dce15b0a7985
Author: David Yan <da...@datatorrent.com>
Date:   2016-04-04T22:56:53Z

    APEXCORE-411 support restarting app without app package or jar files

----


---
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-apex-core pull request: APEXCORE-411 support restarting ...

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

    https://github.com/apache/incubator-apex-core/pull/294#discussion_r58595340
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/client/StramAppLauncher.java ---
    @@ -210,14 +239,67 @@ public StramAppLauncher(FileSystem fs, Path path, Configuration conf) throws Exc
       public StramAppLauncher(String name, Configuration conf) throws Exception
       {
         this.propertiesBuilder = new LogicalPlanConfiguration(conf);
    +    this.conf = conf;
         init(name);
       }
     
    +  public StramAppLauncher(FileSystem fs, Configuration conf) throws Exception
    +  {
    +    this.propertiesBuilder = new LogicalPlanConfiguration(conf);
    +    this.fs = fs;
    +    this.conf = conf;
    +    init();
    +  }
    +
       public String getMvnBuildClasspathOutput()
       {
         return mvnBuildClasspathOutput.toString();
       }
     
    +  /**
    +   * This is for recovering an app without specifying apa or appjar file
    +   * @throws Exception
    +   */
    +  private void init() throws Exception
    +  {
    +    String originalAppId = propertiesBuilder.conf.get(ORIGINAL_APP_ID);
    +    if (originalAppId == null) {
    +      throw new AssertionError("Need original app id if launching without apa or appjar");
    +    }
    +    Path appsBasePath = new Path(StramClientUtils.getDTDFSRootDir(fs, conf), StramClientUtils.SUBDIR_APPS);
    +    Path origAppPath = new Path(appsBasePath, originalAppId);
    +    StringWriter writer = new StringWriter();
    +    try (FSDataInputStream in = fs.open(new Path(origAppPath, "meta.json"))) {
    +      IOUtils.copy(in, writer);
    +    }
    +    JSONObject metaJson = new JSONObject(writer.toString());
    +    String originalLibJars = null;
    +    try {
    +      JSONObject attributes = metaJson.getJSONObject("attributes");
    +      originalLibJars = attributes.getString(LogicalPlan.LIBRARY_JARS.getSimpleName());
    +      recoveryAppName = attributes.getString(Context.DAGContext.APPLICATION_NAME.getSimpleName());
    +    } catch (JSONException ex) {
    +      // ignore
    +    }
    +
    +    LinkedHashSet<URL> clUrls = new LinkedHashSet<>();
    --- End diff --
    
    That was what I thought too. But StramClient.copyInitialState itself needs all the operator classes to be loaded before we can even launch. Here's the exception stack trace if you remove all that code:
    {code}
    com.esotericsoftware.kryo.KryoException: Unable to find class: com.datatorrent.lib.testbench.RandomEventGenerator
    	at com.esotericsoftware.kryo.util.DefaultClassResolver.readName(DefaultClassResolver.java:138)
    	at com.esotericsoftware.kryo.util.DefaultClassResolver.readClass(DefaultClassResolver.java:115)
    	at com.esotericsoftware.kryo.Kryo.readClass(Kryo.java:641)
    	at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:752)
    	at com.datatorrent.common.util.FSStorageAgent.retrieve(FSStorageAgent.java:193)
    	at com.datatorrent.stram.plan.logical.LogicalPlan$OperatorMeta.readObject(LogicalPlan.java:857)
    ...
    ...
    	at java.io.ObjectInputStream.readObject(ObjectInputStream.java:370)
    	at com.datatorrent.stram.FSRecoveryHandler.restore(FSRecoveryHandler.java:247)
    	at com.datatorrent.stram.StramClient.copyInitialState(StramClient.java:275)
    	at com.datatorrent.stram.StramClient.startApplication(StramClient.java:513)
    	at com.datatorrent.stram.client.StramAppLauncher.launchApp(StramAppLauncher.java:616)
    	at com.datatorrent.stram.cli.DTCli$LaunchCommand.execute(DTCli.java:2067)
    	at com.datatorrent.stram.cli.DTCli$3.run(DTCli.java:1454)
    {code}


---
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-apex-core pull request: APEXCORE-411 support restarting ...

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

    https://github.com/apache/incubator-apex-core/pull/294#discussion_r58482768
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/cli/DTCli.java ---
    @@ -1861,49 +1862,60 @@ public void execute(String[] args, ConsoleReader reader) throws Exception
             } catch (Exception ex) {
               throw new CliException("Error opening the config XML file: " + configFile, ex);
             }
    -        String fileName = expandFileName(commandLineInfo.args[0], true);
             StramAppLauncher submitApp;
             AppFactory appFactory = null;
    -        String matchString = commandLineInfo.args.length >= 2 ? commandLineInfo.args[1] : null;
    -        if (fileName.endsWith(".json")) {
    -          File file = new File(fileName);
    -          submitApp = new StramAppLauncher(file.getName(), config);
    -          appFactory = new StramAppLauncher.JsonFileAppFactory(file);
    -          if (matchString != null) {
    -            LOG.warn("Match string \"{}\" is ignored for launching applications specified in JSON", matchString);
    -          }
    -        } else if (fileName.endsWith(".properties")) {
    -          File file = new File(fileName);
    -          submitApp = new StramAppLauncher(file.getName(), config);
    -          appFactory = new StramAppLauncher.PropertyFileAppFactory(file);
    -          if (matchString != null) {
    -            LOG.warn("Match string \"{}\" is ignored for launching applications specified in properties file", matchString);
    +        String matchString = null;
    +        if (commandLineInfo.args.length == 0) {
    +          if (commandLineInfo.origAppId == null) {
    +            throw new CliException("APA or JAR file is required for launching an application when not recovering from a terminated application");
    --- End diff --
    
    "when not resuming terminated application" ?


---
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-apex-core pull request: APEXCORE-411 support restarting ...

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

    https://github.com/apache/incubator-apex-core/pull/294#discussion_r58482984
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/client/StramAppLauncher.java ---
    @@ -210,14 +239,67 @@ public StramAppLauncher(FileSystem fs, Path path, Configuration conf) throws Exc
       public StramAppLauncher(String name, Configuration conf) throws Exception
       {
         this.propertiesBuilder = new LogicalPlanConfiguration(conf);
    +    this.conf = conf;
         init(name);
       }
     
    +  public StramAppLauncher(FileSystem fs, Configuration conf) throws Exception
    +  {
    +    this.propertiesBuilder = new LogicalPlanConfiguration(conf);
    +    this.fs = fs;
    +    this.conf = conf;
    +    init();
    +  }
    +
       public String getMvnBuildClasspathOutput()
       {
         return mvnBuildClasspathOutput.toString();
       }
     
    +  /**
    +   * This is for recovering an app without specifying apa or appjar file
    +   * @throws Exception
    +   */
    +  private void init() throws Exception
    +  {
    +    String originalAppId = propertiesBuilder.conf.get(ORIGINAL_APP_ID);
    +    if (originalAppId == null) {
    +      throw new AssertionError("Need original app id if launching without apa or appjar");
    +    }
    +    Path appsBasePath = new Path(StramClientUtils.getDTDFSRootDir(fs, conf), StramClientUtils.SUBDIR_APPS);
    +    Path origAppPath = new Path(appsBasePath, originalAppId);
    +    StringWriter writer = new StringWriter();
    +    try (FSDataInputStream in = fs.open(new Path(origAppPath, "meta.json"))) {
    +      IOUtils.copy(in, writer);
    +    }
    +    JSONObject metaJson = new JSONObject(writer.toString());
    +    String originalLibJars = null;
    +    try {
    +      JSONObject attributes = metaJson.getJSONObject("attributes");
    +      originalLibJars = attributes.getString(LogicalPlan.LIBRARY_JARS.getSimpleName());
    +      recoveryAppName = attributes.getString(Context.DAGContext.APPLICATION_NAME.getSimpleName());
    +    } catch (JSONException ex) {
    +      // ignore
    +    }
    +
    +    LinkedHashSet<URL> clUrls = new LinkedHashSet<>();
    --- End diff --
    
    What's happening here? Weren't all the dependencies already copied to HDFS?


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