[GitHub] bornej commented on issue #2692: Making emit, ack, and fail thread safe

[GitBox]

bornej commented on issue #2692: Making emit, ack, and fail thread safe
URL: https://github.com/apache/incubator-heron/pull/2692#issuecomment-390423959
 
 
   @jerrypeng Thank you so much!
   
   I will do experiments with heron multithreading in the next weeks. I hope to 
have some results to share soon.



This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] bornej commented on issue #2692: Making emit, ack, and fail thread safe

[GitBox]

bornej commented on issue #2692: Making emit, ack, and fail thread safe
URL: https://github.com/apache/incubator-heron/pull/2692#issuecomment-390423959
 
 
   @jerrypeng Thank you so much!
   
   I will do experiments with heron multithreading in the next weeks. I hope to 
have some performance comparisons to provide soon. 



This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jerrypeng commented on issue #2692: Making emit, ack, and fail thread safe

[GitBox]

jerrypeng commented on issue #2692: Making emit, ack, and fail thread safe
URL: https://github.com/apache/incubator-heron/pull/2692#issuecomment-390422873
 
 
   @nlu90 just to be clear, with out this patch its not even possible to 
guarantee at-most once.  An NPE will very likely to be thrown if you emit, ack, 
or fail from different threads or even if you emit, ack, or fail from the same 
thread but not the bolt/spout instance thread you will run into trouble


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jerrypeng commented on issue #2692: Making emit, ack, and fail thread safe

[GitBox]

jerrypeng commented on issue #2692: Making emit, ack, and fail thread safe
URL: https://github.com/apache/incubator-heron/pull/2692#issuecomment-390422873
 
 
   @nlu90 just to be clear, with out this patch its not even possible to 
guarantee at-most once.  An NPE will very likely to be thrown if you emit, ack, 
or fail from different threads 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] bornej commented on issue #2692: Making emit, ack, and fail thread safe

[GitBox]

bornej commented on issue #2692: Making emit, ack, and fail thread safe
URL: https://github.com/apache/incubator-heron/pull/2692#issuecomment-390401048
 
 
   Thank you very much,
   Is there any example of multi-threaded heron bolt/spout/topology out there?
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] bornej commented on issue #2692: Making emit, ack, and fail thread safe

[GitBox]

bornej commented on issue #2692: Making emit, ack, and fail thread safe
URL: https://github.com/apache/incubator-heron/pull/2692#issuecomment-390401048
 
 
   Thanks you very much,
   Is there any example of multi-threaded heron bolt/spout/topology out there?
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on issue #2794: update container count to 2 in the example ExclamationTopology

[GitBox]

nwangtw commented on issue #2794: update container count to 2 in the example 
ExclamationTopology
URL: https://github.com/apache/incubator-heron/pull/2794#issuecomment-390318771
 
 
   lgtm. but I am curious about the reason to have this PR.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on issue #2807: remove three other roundings in cpu calculation

[GitBox]

nwangtw commented on issue #2807: remove three other roundings in cpu 
calculation
URL: https://github.com/apache/incubator-heron/pull/2807#issuecomment-390299897
 
 
   It is more about "why there was a Math.rounding()".
   
   The rounding logic doesn't seem necessary if we assume cpu # can be less 
than fractional. Also the logic have bugs that in some cases it can be floored 
to 0.
   
   However when I was working on this PR, I found out that some parking 
algorithms rely on the rounding so this is not a quick fix. This PR needs to be 
revisited when I get more time.
   
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #1093: heron package manager tool

[GitBox]

nlu90 commented on issue #1093: heron package manager tool
URL: https://github.com/apache/incubator-heron/pull/1093#issuecomment-390290315
 
 
   Will close this PR for it's opened too long ago and now we have the 
downloaders spi.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #1093: heron package manager tool

[GitBox]

nlu90 closed pull request #1093: heron package manager tool
URL: https://github.com/apache/incubator-heron/pull/1093
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/heron/packer/__init__.py b/heron/packer/__init__.py
new file mode 100644
index 00..e69de29bb2
diff --git a/heron/packer/src/__init__.py b/heron/packer/src/__init__.py
new file mode 100644
index 00..e69de29bb2
diff --git a/heron/packer/src/python/BUILD b/heron/packer/src/python/BUILD
new file mode 100644
index 00..2e13b33ebd
--- /dev/null
+++ b/heron/packer/src/python/BUILD
@@ -0,0 +1,29 @@
+package(default_visibility = ["//visibility:public"])
+
+load("/tools/rules/pex_rules", "pex_library", "pex_binary")
+
+pex_library(
+name = "heron-packer-lib",
+srcs = [
+  "store/metastore.py",
+  "store/blobstore.py",
+  "common/utils.py",
+  "common/packer.py",
+  "common/constants.py",
+  "common/parser.py",
+],
+deps = [
+  "//heron/common/src/python:common",
+],
+reqs = ["pyyaml==3.10"],
+)
+
+pex_binary(
+name = "heron-packer",
+srcs = [
+"main.py",
+],
+deps = [
+":heron-packer-lib",
+]
+)
diff --git a/heron/packer/src/python/__init__.py 
b/heron/packer/src/python/__init__.py
new file mode 100644
index 00..e69de29bb2
diff --git a/heron/packer/src/python/common/__init__.py 
b/heron/packer/src/python/common/__init__.py
new file mode 100644
index 00..d7e8c54ea0
--- /dev/null
+++ b/heron/packer/src/python/common/__init__.py
@@ -0,0 +1,10 @@
+"""common module"""
+
+from .packer import (
+Packer,
+HeronPacker
+)
+
+from .utils import *
+from .constants import *
+from .parser import *
diff --git a/heron/packer/src/python/common/constants.py 
b/heron/packer/src/python/common/constants.py
new file mode 100644
index 00..75a7ce56f4
--- /dev/null
+++ b/heron/packer/src/python/common/constants.py
@@ -0,0 +1,17 @@
+"""constans used by package management tool"""
+
+CLUSTER = "cluster"
+COUNTER = "counter"
+DELETED = "deleted"
+DESC = "description"
+DEST_PATH = "dest_path"
+ENV = "env"
+EXTRA = "extra_fields"
+LATEST = "latest"
+LIVE = "live"
+PKG = "package"
+PKG_PATH = "pkg_path"
+RAW = "raw"
+ROLE = "role"
+STORE_PATH = "store_location"
+VERSION = "version"
diff --git a/heron/packer/src/python/common/packer.py 
b/heron/packer/src/python/common/packer.py
new file mode 100644
index 00..b3af257696
--- /dev/null
+++ b/heron/packer/src/python/common/packer.py
@@ -0,0 +1,206 @@
+"""Package module defines the interfaces of package management tool needs to 
implement"""
+import sys
+
+from heron.common.src.python.color import Log
+from heron.packer.src.python.common import constants
+from heron.packer.src.python.common import utils
+
+class Packer(object):
+  """Package interface"""
+  def add_version(self, args):
+"""add a package with 'args' information"""
+raise NotImplementedError("add_version is not implemented")
+
+  def download(self, args):
+"""download a package referenced by 'args'"""
+raise NotImplementedError("download is not implemented")
+
+  def delete_version(self, args):
+"""delete a package referenced by 'args'"""
+raise NotImplementedError("delete_version is not implemented")
+
+  def list_packages(self, args):
+"""list all packages referenced by 'args'"""
+raise NotImplementedError("list_packages is not implemented")
+
+  def list_versions(self, args):
+"""list all versions of a package referenced by 'args'"""
+raise NotImplementedError("list_versions is not implemented")
+
+  def set_live(self, args):
+"""set a package's version to 'live'"""
+raise NotImplementedError("set_live is not implemented")
+
+  def unset_live(self, args):
+"""unset the 'live' tag on a package"""
+raise NotImplementedError("unset_live is not implemented")
+
+  def show_version(self, args):
+"""show the basic information for package's version"""
+raise NotImplementedError("show_version is not implemented")
+
+  def show_live(self, args):
+"""show the basic information for package's 'live' version"""
+raise NotImplementedError("show_live is not implemented")
+
+  def show_latest(self, args):
+"""show the basic information for package's 'latest' version"""
+raise NotImplementedError("show_latest is not implemented")
+
+  def clean(self, args):
+"""clean the state"""
+raise NotImplementedError("clean is not implemented")
+
+class HeronPacker(Packer):
+  """A metastore and blobstore based implementation of Package"""
+  def __init__(self, metastore, blobstore):
+self.metastore = metastore
+self.blobstore = blobstore
+
+  def add_version(self, args):
+role = args[constants.ROLE]
+pkg_name = 

[GitHub] nlu90 commented on issue #2794: update container count to 2 in the example ExclamationTopology

[GitBox]

nlu90 commented on issue #2794: update container count to 2 in the example 
ExclamationTopology
URL: https://github.com/apache/incubator-heron/pull/2794#issuecomment-390289767
 
 
   @kramasamy @nwangtw @huijunw Any ideas on this PR?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2807: remove three other roundings in cpu calculation

[GitBox]

nlu90 commented on issue #2807: remove three other roundings in cpu calculation
URL: https://github.com/apache/incubator-heron/pull/2807#issuecomment-390288719
 
 
   What's the background of removing this `Math.rounding()`? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2850: Add SSL support for stream manager connections

[GitBox]

nlu90 commented on issue #2850: Add SSL support for stream manager connections
URL: https://github.com/apache/incubator-heron/pull/2850#issuecomment-390287634
 
 
   @ajorgensen seems several unit tests failed. Could you update it and for a 
further review?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2691: Fix for localhost metrics manager

[GitBox]

nlu90 commented on issue #2691: Fix for localhost metrics manager
URL: https://github.com/apache/incubator-heron/pull/2691#issuecomment-390286652
 
 
   @jrcrawfo Ping for this PR, any updates?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2872: read merge instructions;

[GitBox]

nlu90 closed pull request #2872: read merge instructions; 
URL: https://github.com/apache/incubator-heron/pull/2872
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/.htaccess b/.htaccess
new file mode 100644
index 00..305761dfdf
--- /dev/null
+++ b/.htaccess
@@ -0,0 +1,2 @@
+Redirect   /index.html https://apache.github.io/incubator-heron/
+
diff --git a/index.html b/index.html
new file mode 100644
index 00..69301650ff
--- /dev/null
+++ b/index.html
@@ -0,0 +1,3 @@
+
+
+


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2902: Make MetricsManager.java a target so that it can be customized

[GitBox]

nlu90 closed pull request #2902: Make MetricsManager.java a target so that it 
can be customized
URL: https://github.com/apache/incubator-heron/pull/2902
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/heron/metricsmgr/src/java/BUILD b/heron/metricsmgr/src/java/BUILD
index 36cf3ade31..4707369838 100644
--- a/heron/metricsmgr/src/java/BUILD
+++ b/heron/metricsmgr/src/java/BUILD
@@ -24,6 +24,11 @@ deps = [
 "@org_yaml_snakeyaml//jar",
 ]
 
+filegroup(
+name = "metricsmgr-main",
+srcs = glob(["**/MetricsManager.java"]),
+)
+
 java_library(
 name = "metricsmgr-java",
 srcs = files, 
@@ -32,7 +37,7 @@ java_library(
 
 java_binary(
 name = "metricsmgr-unshaded",
-srcs = glob(["**/MetricsManager.java"]),
+srcs = [":metricsmgr-main"],
 deps = [
 ":metricsmgr-java",
 "//heron/api/src/java:api-java-low-level",


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2692: Making emit, ack, and fail thread safe

[GitBox]

nlu90 commented on issue #2692: Making emit, ack, and fail thread safe
URL: https://github.com/apache/incubator-heron/pull/2692#issuecomment-390284998
 
 
   @bornej It is possible to utilizing multithreads in Heron even without this 
patch. The only requirement is that you need to ack/fail the processed tuple in 
the original main thread for correctness. If your topology is AT-MOST-ONCE 
semantics, then you don't even need to worry about this problem.
   
   @srkukarni After reviewing this long converstaion and the changes made, it 
looks good to me. But I think we need to mention the thread-safe acking to 
users very carefully, ensures them that EFFECTLY-ONCE might be broken if they 
don't implement `preSave` carefully.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] bornej commented on issue #2692: Making emit, ack, and fail thread safe

[GitBox]

bornej commented on issue #2692: Making emit, ack, and fail thread safe
URL: https://github.com/apache/incubator-heron/pull/2692#issuecomment-390241133
 
 
   Hi, 
   while investigating the possibility of using multi-threaded bolt/spouts, I 
stumbled upon this PR.
   I don't know if this is the appropriate place to ask but I'm stuck with some 
questions...
   
   From what I understand, in each Heron instance there is only one slave 
thread doing bolt/spout computation (and a gateway thread used for 
communications with the stream manager). But after reading this discussion I'm 
a bit confused
   
   @jerrypeng 
   > We have some use cases in which users will want to emit and ack or fail in 
different threads.
   
   @srkukarni 
   > Users can(and do) have concurrent threads today. 
   > ... imagine this situation
   > 1. User Bolt has two 'worker' threads that actually do some work and use 
some kind of queue to put 
   > their acks/fails in the main process thread.

   Is it currently possible for a user to create a multi-threaded Bolt/spout? 
(Without altering HeronInstance implementation-> FixedThreadPool size) 
   If so, could someone help me figure out how to achieve that?
   
   I'm very new to Heron so I hope you will forgive me if my question seems 
stupid.
   
   I understand that Heron philosophy is to use "single"-threaded Instances 
(bolt/spouts) and why this design choice was made but I'm wondering if some 
computation tasks could benefit from multi-threading...
   
   Thanks for your help and your work.
   Best Regards.
   Jonathan.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2903: cleaning setup scripts for removed directories

[GitBox]

nlu90 closed pull request #2903: cleaning setup scripts for removed directories
URL: https://github.com/apache/incubator-heron/pull/2903
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/scripts/get_all_heron_paths.sh b/scripts/get_all_heron_paths.sh
index 8df9b5207c..c88921bd91 100755
--- a/scripts/get_all_heron_paths.sh
+++ b/scripts/get_all_heron_paths.sh
@@ -67,7 +67,7 @@ function get_package_of() {
 }
 
 function get_heron_java_paths() {
-  local java_paths=$(find 
{heron,heron/tools,tools,integration_test,storm-compatibility,contrib,eco,eco-storm-examples,eco-heron-examples}
 -name "*.java" | sed "s|/src/java/.*$|/src/java|"| sed 
"s|/java/src/.*$|/java/src|" |  sed "s|/tests/java/.*$|/tests/java|" | sort -u 
| fgrep -v "heron/scheduler/" | fgrep -v "heron/scheduler/" )
+  local java_paths=$(find 
{heron,heron/tools,tools,integration_test,storm-compatibility,eco,eco-storm-examples,eco-heron-examples}
 -name "*.java" | sed "s|/src/java/.*$|/src/java|"| sed 
"s|/java/src/.*$|/java/src|" |  sed "s|/tests/java/.*$|/tests/java|" | sort -u 
| fgrep -v "heron/scheduler/" | fgrep -v "heron/scheduler/" )
   if [ "$(uname -s | tr 'A-Z' 'a-z')" != "darwin" ]; then
 java_paths=$(echo "${java_paths}" | fgrep -v "/objc_tools/")
   fi
@@ -105,8 +105,7 @@ function get_containing_library() {
 
 function collect_generated_binary_deps() {
   local proto_deps=$(find bazel-bin/heron/proto -type f | grep "jar$");
-  local thrift_deps=$(find bazel-bin/heron/metricsmgr/src/thrift -type f | 
grep "jar$");
-  echo "${proto_deps} ${thrift_deps}" | sort | uniq
+  echo "${proto_deps}" | sort | uniq
 }
 
 function collect_generated_paths() {


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2901: Refactor CheckpointManager to move initialization code out of constru…

[GitBox]

nlu90 closed pull request #2901: Refactor CheckpointManager to move 
initialization code out of constru…
URL: https://github.com/apache/incubator-heron/pull/2901
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/heron/ckptmgr/src/java/org/apache/heron/ckptmgr/CheckpointManager.java 
b/heron/ckptmgr/src/java/org/apache/heron/ckptmgr/CheckpointManager.java
index fa9c435f9c..5c590ffca5 100644
--- a/heron/ckptmgr/src/java/org/apache/heron/ckptmgr/CheckpointManager.java
+++ b/heron/ckptmgr/src/java/org/apache/heron/ckptmgr/CheckpointManager.java
@@ -50,8 +50,8 @@
   private static final String CHECKPOINT_MANAGER_HOST = "127.0.0.1";
 
   // The looper drives CheckpointManagerServer
-  private final NIOLooper checkpointManagerServerLoop;
-  private final CheckpointManagerServer checkpointManagerServer;
+  private NIOLooper checkpointManagerServerLoop;
+  private CheckpointManagerServer checkpointManagerServer;
 
   // Print usage options
   private static void usage(Options options) {
@@ -133,14 +133,21 @@ private static Options constructHelpOptions() {
 return options;
   }
 
-  public CheckpointManager(
-  String topologyName, String topologyId, String checkpointMgrId,
-  String serverHost, int serverPort,
-  SystemConfig systemConfig, CheckpointManagerConfig 
checkpointManagerConfig)
-  throws IOException, CheckpointManagerException {
+  public CheckpointManager() {
+  }
 
-this.checkpointManagerServerLoop = new NIOLooper();
+  public void init(
+  String topologyName,
+  String topologyId,
+  String checkpointMgrId,
+  String serverHost,
+  int serverPort,
+  SystemConfig systemConfig,
+  CheckpointManagerConfig checkpointManagerConfig)
+  throws IOException, CheckpointManagerException {
 
+LOG.info("Initializing CheckpointManager");
+checkpointManagerServerLoop = new NIOLooper();
 HeronSocketOptions serverSocketOptions =
 new HeronSocketOptions(
 checkpointManagerConfig.getWriteBatchSize(),
@@ -152,9 +159,29 @@ public CheckpointManager(
 checkpointManagerConfig.getMaximumPacketSize());
 
 // Setup the IStatefulStorage
-// TODO(mfu): This should be done in an executor driven by another thread, 
kind of async
+IStatefulStorage statefulStorage = setupStatefulStorage(topologyName, 
checkpointManagerConfig);
+
+// Start the server
+this.checkpointManagerServer = new CheckpointManagerServer(
+topologyName, topologyId, checkpointMgrId, statefulStorage,
+checkpointManagerServerLoop, serverHost, serverPort, 
serverSocketOptions);
+  }
+
+  public void startAndLoop() {
+// The CheckpointManagerServer would run in the main thread
+// We do it in the final step since it would await the main thread
+LOG.info("Starting CheckpointManager Server");
+checkpointManagerServer.start();
+checkpointManagerServerLoop.loop();
+  }
+
+  private static IStatefulStorage setupStatefulStorage(
+  String topologyName,
+  CheckpointManagerConfig checkpointManagerConfig) throws 
CheckpointManagerException {
+
 IStatefulStorage statefulStorage;
 String classname = checkpointManagerConfig.getStorageClassname();
+
 try {
   statefulStorage = (IStatefulStorage) 
Class.forName(classname).newInstance();
 } catch (InstantiationException e) {
@@ -172,18 +199,7 @@ public CheckpointManager(
   throw new CheckpointManagerException(classname + " init threw 
exception", e);
 }
 
-// Start the server
-this.checkpointManagerServer = new CheckpointManagerServer(
-topologyName, topologyId, checkpointMgrId, statefulStorage,
-checkpointManagerServerLoop, serverHost, serverPort, 
serverSocketOptions);
-  }
-
-  public void startAndLoop() {
-// The CheckpointManagerServer would run in the main thread
-// We do it in the final step since it would await the main thread
-LOG.info("Starting CheckpointManager Server");
-checkpointManagerServer.start();
-checkpointManagerServerLoop.loop();
+return statefulStorage;
   }
 
   public static void main(String[] args) throws IOException,
@@ -241,9 +257,9 @@ public static void main(String[] args) throws IOException,
 
 LOG.info("System Config: " + systemConfig);
 
-CheckpointManager checkpointManager =
-new CheckpointManager(topologyName, topologyId, ckptmgrId,
-CHECKPOINT_MANAGER_HOST, port, systemConfig, ckptmgrConfig);
+CheckpointManager checkpointManager = new CheckpointManager();
+checkpointManager.init(topologyName, topologyId, ckptmgrId,
+CHECKPOINT_MANAGER_HOST, port, systemConfig, ckptmgrConfig);
 checkpointManager.startAndLoop();
 
 LOG.info("Loops terminated. Exiting.");


 

[GitHub] nlu90 opened a new pull request #2903: cleaning removed directory for setup scripts

[GitBox]

nlu90 opened a new pull request #2903: cleaning removed directory for setup 
scripts
URL: https://github.com/apache/incubator-heron/pull/2903
 
 
   Two directories are removed from the project but the paths are not updated 
accordingly in the setup scripts. And causing the following warning:
   
   ```
   find: contrib: No such file or directory
   find: bazel-bin/heron/metricsmgr/src/thrift: No such file or directory
   ```
   
   This PR is to fix the issue. After the cleaning, the building result is:
   
   ```
   INFO: Build completed successfully, 4754 total actions
   Bazel build successful!!
   Path is /Users/nlu/workspace/heron
   Generating IDEA project...
   Done. IDEA module file: heron.iml
   Opening Heron project in IDEA...
   Done.
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2870: Add extra info when failed to create new instance of class

[GitBox]

nlu90 closed pull request #2870: Add extra info when failed to create new 
instance of class
URL: https://github.com/apache/incubator-heron/pull/2870
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/storm-compatibility/src/java/backtype/storm/utils/Utils.java 
b/storm-compatibility/src/java/backtype/storm/utils/Utils.java
index 696956f3f4..a05bed58b0 100644
--- a/storm-compatibility/src/java/backtype/storm/utils/Utils.java
+++ b/storm-compatibility/src/java/backtype/storm/utils/Utils.java
@@ -38,7 +38,7 @@ public static Object newInstance(String klass) {
   Class c = Class.forName(klass);
   return c.newInstance();
 } catch (ClassNotFoundException | InstantiationException | 
IllegalAccessException e) {
-  throw new RuntimeException(e);
+  throw new RuntimeException("Failed to create instance for class: " + 
klass, e);
 }
   }
 
diff --git a/storm-compatibility/src/java/org/apache/storm/utils/Utils.java 
b/storm-compatibility/src/java/org/apache/storm/utils/Utils.java
index d67af5ed55..a30a77d211 100644
--- a/storm-compatibility/src/java/org/apache/storm/utils/Utils.java
+++ b/storm-compatibility/src/java/org/apache/storm/utils/Utils.java
@@ -38,7 +38,7 @@ public static Object newInstance(String klass) {
   Class c = Class.forName(klass);
   return c.newInstance();
 } catch (ClassNotFoundException | InstantiationException | 
IllegalAccessException e) {
-  throw new RuntimeException(e);
+  throw new RuntimeException("Failed to create instance for class: " + 
klass, e);
 }
   }
 


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kramasamy commented on issue #2900: Fix Potential Double Delete Bug

[GitBox]

kramasamy commented on issue #2900: Fix Potential Double Delete Bug
URL: https://github.com/apache/incubator-heron/pull/2900#issuecomment-389922301
 
 
   We can merge it
   
   Sent from my iPhone
   
   > On May 17, 2018, at 7:54 AM, Andrew Jorgensen  
wrote:
   > 
   > It's an improvement I think since there is a potential for a double free 
bug here if packets fail to send but I haven't seen a crash in production 
related to this yet
   > 
   > —
   > You are receiving this because you commented.
   > Reply to this email directly, view it on GitHub, or mute the thread.
   > 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ajorgensen commented on issue #2900: Fix Potential Double Delete Bug

[GitBox]

ajorgensen commented on issue #2900: Fix Potential Double Delete Bug
URL: https://github.com/apache/incubator-heron/pull/2900#issuecomment-389895629
 
 
   It's an improvement I think since there is a potential for a double free bug 
here if packets fail to send but I haven't seen a crash in production related 
to this yet


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kramasamy closed pull request #2898: add documentation about nomad consul integration

[GitBox]

kramasamy closed pull request #2898: add documentation about nomad consul 
integration
URL: https://github.com/apache/incubator-heron/pull/2898
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/heron/config/src/yaml/conf/nomad/scheduler.yaml 
b/heron/config/src/yaml/conf/nomad/scheduler.yaml
index 8e7e3a5cf8..832ca76164 100644
--- a/heron/config/src/yaml/conf/nomad/scheduler.yaml
+++ b/heron/config/src/yaml/conf/nomad/scheduler.yaml
@@ -58,5 +58,5 @@ heron.nomad.metrics.service.check.interval.sec: 10
 heron.nomad.metrics.service.check.timeout.sec: 2
 
 # additional tags to be attached to metrics service
-# A tag of - with be automaticallu attached
-heron.nomad.metrics.service.additional.tags: "prometheus,metrics,heron"
\ No newline at end of file
+# A tag of - with be automatically attached
+heron.nomad.metrics.service.additional.tags: "prometheus,metrics,heron"
diff --git a/heron/config/src/yaml/conf/standalone/scheduler.yaml 
b/heron/config/src/yaml/conf/standalone/scheduler.yaml
index fc1cd27a94..a492ef2e86 100644
--- a/heron/config/src/yaml/conf/standalone/scheduler.yaml
+++ b/heron/config/src/yaml/conf/standalone/scheduler.yaml
@@ -51,5 +51,5 @@ heron.nomad.metrics.service.check.interval.sec: 10
 heron.nomad.metrics.service.check.timeout.sec: 2
 
 # additional tags to be attached to metrics service in a comma delimited list
-# A tag of - with be automaticallu attached
+# A tag of - with be automatically attached
 heron.nomad.metrics.service.additional.tags: "prometheus,metrics,heron"
diff --git 
a/website/content/docs/operators/deployment/schedulers/nomad-docker.md 
b/website/content/docs/operators/deployment/schedulers/nomad-docker.md
index 063a4cc993..cde8320573 100644
--- a/website/content/docs/operators/deployment/schedulers/nomad-docker.md
+++ b/website/content/docs/operators/deployment/schedulers/nomad-docker.md
@@ -4,7 +4,7 @@ title: Running Heron via Docker Containers on Nomad
 
 Below are instructions on how to run Heron on Nomad via docker containers.  In 
this mode, Heron executors will run as docker containers on host machines.
 
-### Requirements
+## Requirements
 
 When setting up your Nomad cluster, the following are required:
 
@@ -14,7 +14,7 @@ When setting up your Nomad cluster, the following are 
required:
 * Docker installed and enabled on every machine
 * Each machine must also be able to pull the official Heron docker image from 
DockerHub or have the image preloaded.
 
-### Configuring Heron settings
+## Configuring Heron settings
 
 Before running Heron via Nomad, you'll need to configure some settings. Once 
you've [installed Heron](../../../../getting-started), all of the 
configurations you'll need to modify will be in the `~/.heron/conf/nomad` 
diredctory.
 
@@ -132,3 +132,33 @@ Heron users can upload their Heron topology packages to 
the Heron API server usi
 heron.class.uploader:org.apache.heron.uploader.http.HttpUploader
 heron.uploader.http.uri: http://localhost:9000/api/v1/file/upload
 ```
+
+## Integration with Consul for metrics
+Each container part of a Heron topology serves metrics out of a port randomly 
generated by Nomad.  Thus, Consul is needed for service discovery for users to 
determine which port the container is serving the metrics out of.
+Every Heron executor running in a docker container will automatically register 
itself as a service with Consul given that there is a Consul cluster running. 
The port Heron will be serving metrics will be registered with Consul.
+  
+The service will be registered with the name with the following format:
+
+```yaml
+metrics-heron--
+```
+
+Each heron executor registered with Consul will be tagged with
+
+```yaml
+-
+```
+
+To add additional tags, please add specify them in a comma delimited list via
+
+```yaml
+heron.nomad.metrics.service.additional.tags
+```
+
+in `scheduler.yaml`. For example:
+
+```yaml
+heron.nomad.metrics.service.additional.tags: "prometheus,metrics,heron"
+```
+
+Users can then configure Prometheus to scrape metrics for each container based 
on these tags
diff --git 
a/website/content/docs/operators/deployment/schedulers/nomad-raw-execs.md 
b/website/content/docs/operators/deployment/schedulers/nomad-raw-execs.md
index b362123874..79cc02afcc 100644
--- a/website/content/docs/operators/deployment/schedulers/nomad-raw-execs.md
+++ b/website/content/docs/operators/deployment/schedulers/nomad-raw-execs.md
@@ -6,7 +6,7 @@ Below are instructions on how to to run Heron on Nomad via raw 
execs.  In this m
 
 The advantages of this mode is that it is incredibly lightweight and likely do 
not require sudo privileges to setup and run.  However in this mode, the setup 
procedure may be a little more complex compared to running via docker since 
there are more things 

[GitHub] nwangtw opened a new pull request #2902: Make MetricsManager.java a target so that it can be customized

[GitBox]

nwangtw opened a new pull request #2902: Make MetricsManager.java a target so 
that it can be customized
URL: https://github.com/apache/incubator-heron/pull/2902
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw opened a new pull request #2901: Refactor CheckpointManager to move initialization code out of constru…

[GitBox]

nwangtw opened a new pull request #2901: Refactor CheckpointManager to move 
initialization code out of constru…
URL: https://github.com/apache/incubator-heron/pull/2901
 
 
   …ctor


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ajorgensen closed issue #2897: Potential double free crash in BaseConnection destructor on 0.17.8

[GitBox]

ajorgensen closed issue #2897: Potential double free crash in BaseConnection 
destructor on 0.17.8
URL: https://github.com/apache/incubator-heron/issues/2897
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ajorgensen opened a new pull request #2900: Fix Stream Manager Crash

[GitBox]

ajorgensen opened a new pull request #2900: Fix Stream Manager Crash
URL: https://github.com/apache/incubator-heron/pull/2900
 
 
   This fixes https://github.com/apache/incubator-heron/issues/2897 which is a 
double free bug that periodically causes the stream manager to crash if other 
stream managers or are killed at the right time. 
   
   In this case opkt has the potential to be freed twice in the event of a 
failed packet sent in both `InternalSendResponse` and `conn->sendPacket(opkt)`
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu commented on issue #2880: Redirect gitbox commits to commits@

[GitBox]

huijunwu commented on issue #2880: Redirect gitbox commits to commits@
URL: 
https://github.com/apache/incubator-heron/issues/2880#issuecomment-389036496
 
 
   gitbox commits(when the PRs are merged) are to commits@ already.
   I think this Issue is: disable the PR/Issues comments email notification ?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu commented on issue #2880: Redirect gitbox commits to commits@

[GitBox]

huijunwu commented on issue #2880: Redirect gitbox commits to commits@
URL: 
https://github.com/apache/incubator-heron/issues/2880#issuecomment-389036496
 
 
   gitbox commits(when the PRs are merged) are to commits@ already.
   I think this Issue is: disable the PR comments email notification ?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu closed pull request #2899: add src export target for downloader

[GitBox]

huijunwu closed pull request #2899: add src export target for downloader
URL: https://github.com/apache/incubator-heron/pull/2899
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu opened a new pull request #2899: add src export target for downloader

[GitBox]

huijunwu opened a new pull request #2899: add src export target for downloader
URL: https://github.com/apache/incubator-heron/pull/2899
 
 
   In some environment, the developers would like to develop their own 
downloaders. This PR exports the downloader src so that they can be included 
into the customized downloader.
   
   This PR does not change existing behavior.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu commented on issue #2274: Topology package fetcher.

[GitBox]

huijunwu commented on issue #2274: Topology package fetcher.
URL: https://github.com/apache/incubator-heron/pull/2274#issuecomment-388449226
 
 
   replaced by #2886. close this pr


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2886: submit topology file by uri

[GitBox]

nlu90 closed pull request #2886: submit topology file by uri
URL: https://github.com/apache/incubator-heron/pull/2886
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

nwangtw commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187686955
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -195,6 +218,9 @@ message SaveInstanceStateRequest {
   // Information about the instance whose state this is
   required heron.proto.system.Instance instance = 1;
   required InstanceStateCheckpoint checkpoint = 2;
+  // Stmgrs need to send component metadata so that Checkpoint
+  // Mgrs know how to repartition if needed.
+  optional CheckpointComponentMetadata metadata = 3;
 
 Review comment:
   Checked the communication between stmgr and ckptmgr, it is a valid option to 
pass the topology structure information during the registration to simplify the 
requests.
   
   We don't need to make the decision for now about where the structure 
information should be passed. Therefore I am going to remove this field for now 
to keep it simple.
   
   Thanks again for the comments!


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2852: first of the connectors - twitter spout

[GitBox]

nlu90 commented on issue #2852: first of the connectors - twitter spout
URL: https://github.com/apache/incubator-heron/pull/2852#issuecomment-388425678
 
 
   @kramasamy  Ping


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

nwangtw commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187630121
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -195,6 +218,9 @@ message SaveInstanceStateRequest {
   // Information about the instance whose state this is
   required heron.proto.system.Instance instance = 1;
   required InstanceStateCheckpoint checkpoint = 2;
+  // Stmgrs need to send component metadata so that Checkpoint
+  // Mgrs know how to repartition if needed.
+  optional CheckpointComponentMetadata metadata = 3;
 
 Review comment:
   But on the other hand, this could be a good idea. If ckptmgr receives the 
topology structure during initialization, we can save the field from the 
requests. Let me see how to do it.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

nwangtw commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187540122
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -195,6 +218,9 @@ message SaveInstanceStateRequest {
   // Information about the instance whose state this is
   required heron.proto.system.Instance instance = 1;
   required InstanceStateCheckpoint checkpoint = 2;
+  // Stmgrs need to send component metadata so that Checkpoint
+  // Mgrs know how to repartition if needed.
+  optional CheckpointComponentMetadata metadata = 3;
 
 Review comment:
   chptmgr is stateless and it doesn't have the physical plan, so it doesn't 
know the parallelism by itself.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

nwangtw commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187539899
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,26 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
 
 Review comment:
   kk. Let me remove it for now until we have a better idea/support.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] srkukarni commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

srkukarni commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187532107
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -195,6 +218,9 @@ message SaveInstanceStateRequest {
   // Information about the instance whose state this is
   required heron.proto.system.Instance instance = 1;
   required InstanceStateCheckpoint checkpoint = 2;
+  // Stmgrs need to send component metadata so that Checkpoint
+  // Mgrs know how to repartition if needed.
+  optional CheckpointComponentMetadata metadata = 3;
 
 Review comment:
   my comment was more that when you change the parallelism, wouldnt the 
ckptmgr be restarted also and thus it too knows the topology structure?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] srkukarni commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

srkukarni commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187531976
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,26 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
 
 Review comment:
   IMO this really pollutes the abstraction. You have now introduced some kind 
of versioning in the system at such a low level to begin with without any kind 
of higher level api/rationale. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

nwangtw commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187484567
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,26 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
 
 Review comment:
   We are thinking about calculation change. Not sure how useful it is though. 
Please feel free to let us know if you feel this is overkill.
   
   Normally the calculation in bolts won't change much and the version number 
doesn't need to change. However in case the equation is changed, for example,  
the old value is a duration in seconds and it is changed to duration in 
millisecond, or it is possible the value is a data structure and a new field is 
added. User can increase the version number and provide a conversion function 
between the two versions. Then the topology would be able to load the stateful 
data stored before the update, or skip/reject the data if topology doesn't know 
how to convert between the two versions.
   
   If topology cant tell the difference and just load the data, the state could 
be polluted without user knowing it. The current design seems to rely on users 
manually removing the data? it could be risky and hard since the topology needs 
to be running in order to do it I think?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] srkukarni commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

srkukarni commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187466926
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -195,6 +218,9 @@ message SaveInstanceStateRequest {
   // Information about the instance whose state this is
   required heron.proto.system.Instance instance = 1;
   required InstanceStateCheckpoint checkpoint = 2;
+  // Stmgrs need to send component metadata so that Checkpoint
+  // Mgrs know how to repartition if needed.
+  optional CheckpointComponentMetadata metadata = 3;
 
 Review comment:
   does this need to be passed to cjptmgr? Doesn't ckptmgr know this info


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] srkukarni commented on a change in pull request #2891: Refactor StatefulStorage API

[GitBox]

srkukarni commented on a change in pull request #2891: Refactor StatefulStorage 
API
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r187466788
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,26 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
 
 Review comment:
   exactly what does this data version do?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jerrypeng commented on issue #2898: add documentation about nomad consul integration

[GitBox]

jerrypeng commented on issue #2898: add documentation about nomad consul 
integration
URL: https://github.com/apache/incubator-heron/pull/2898#issuecomment-387957319
 
 
   @lucperkins @kramasamy please review


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2884: move downloader registry map to yaml

[GitBox]

nlu90 closed pull request #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw closed issue #2829: Update license text to apache incubator

[GitBox]

nwangtw closed issue #2829: Update license text to apache incubator
URL: https://github.com/apache/incubator-heron/issues/2829
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu commented on issue #2884: move downloader registry map to yaml

[GitBox]

huijunwu commented on issue #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#issuecomment-387594464
 
 
   @jerrypeng updated to be compatible with `downloader  
`. could you review it again


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] tomncooper commented on issue #2875: Duration reporting inconsistency

[GitBox]

tomncooper commented on issue #2875: Duration reporting inconsistency
URL: 
https://github.com/apache/incubator-heron/issues/2875#issuecomment-387559852
 
 
   Duplicate of #2874 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on a change in pull request #2852: first of the connectors - twitter spout

[GitBox]

nlu90 commented on a change in pull request #2852: first of the connectors - 
twitter spout
URL: https://github.com/apache/incubator-heron/pull/2852#discussion_r186883119
 
 

 ##
 File path: third_party/cereal/BUILD
 ##
 @@ -0,0 +1,126 @@
+licenses(["notice"])
 
 Review comment:
   We already have a cereal lib in the third_party dir 
[here](https://github.com/apache/incubator-heron/tree/master/third_party/cereal).
   
   So please either remove the files here or send out another separate PR for 
changing this lib


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on a change in pull request #2852: first of the connectors - twitter spout

[GitBox]

nlu90 commented on a change in pull request #2852: first of the connectors - 
twitter spout
URL: https://github.com/apache/incubator-heron/pull/2852#discussion_r186883787
 
 

 ##
 File path: 
connectors/heron-twitter/src/java/com/twitter/heron/twitter/spout/Twitter.java
 ##
 @@ -0,0 +1,122 @@
+package com.streamlio.connectors.twitter;
 
 Review comment:
   the package should be `org.apache.heron.connectors.twitter` now


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed issue #2877: Compilation error in a container

[GitBox]

nlu90 closed issue #2877: Compilation error in a container
URL: https://github.com/apache/incubator-heron/issues/2877
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2877: Compilation error in a container

[GitBox]

nlu90 commented on issue #2877: Compilation error in a container
URL: 
https://github.com/apache/incubator-heron/issues/2877#issuecomment-387558212
 
 
   @comes5 In short, Yes. You'll need to build the whole project to get the 
core jars for testing. But since bazel is smart enough to do the incremental 
building, it will only build targets affected by your changes after the initial 
full building.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] ajorgensen opened a new issue #2897: Potential double free crash in BaseConnection destructor

[GitBox]

ajorgensen opened a new issue #2897: Potential double free crash in 
BaseConnection destructor
URL: https://github.com/apache/incubator-heron/issues/2897
 
 
   On version `0.17.8` it appears that there may be a double free error that 
causes a stream manager to crash sometimes in the destructor for the 
BaseConnection
   
   ```
   [Thread debugging using libthread_db enabled]
   Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
   Core was generated by `./heron-core/bin/heron-stmgr 
--topology_name=insights-speed-android --topology_'.
   Program terminated with signal SIGABRT, Aborted.
   #0  0x7ff4f8033c37 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
   56../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
   (gdb) bt
   #0  0x7ff4f8033c37 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
   #1  0x7ff4f8037028 in __GI_abort () at abort.c:89
   #2  0x00513481 in tcmalloc::Log (mode=mode@entry=tcmalloc::kCrash, 
filename=filename@entry=0x6109c2 "src/tcmalloc.cc", line=line@entry=278, a=..., 
b=..., c=..., d=...) at src/internal_logging.cc:120
   #3  0x0050ef35 in (anonymous namespace)::InvalidFree 
(ptr=ptr@entry=0x6f72646e412e6e6f) at src/tcmalloc.cc:278
   #4  0x0040a3e2 in free_null_or_invalid (invalid_free_fn=0x50eee0 
<(anonymous namespace)::InvalidFree(void*)>, ptr=ptr@entry=0x6f72646e412e6e6f) 
at src/tcmalloc.cc:1137
   #5  (anonymous namespace)::do_free_helper (ptr=ptr@entry=0x6f72646e412e6e6f, 
heap_must_be_valid=true, heap=0x21720a0, invalid_free_fn=0x50eee0 <(anonymous 
namespace)::InvalidFree(void*)>) at src/tcmalloc.cc:1181
   #6  0x00609cdc in do_free_helper (invalid_free_fn=0x50eee0 
<(anonymous namespace)::InvalidFree(void*)>, heap_must_be_valid=true, 
heap=0x21720a0, ptr=0x6f72646e412e6e6f) at src/thread_cache.h:381
   #7  do_free_with_callback (invalid_free_fn=0x50eee0 <(anonymous 
namespace)::InvalidFree(void*)>, ptr=0x6f72646e412e6e6f) at src/tcmalloc.cc:1221
   #8  do_free (ptr=0x6f72646e412e6e6f) at src/tcmalloc.cc:1230
   #9  tc_free (ptr=0x6f72646e412e6e6f) at src/tcmalloc.cc:1581
   #10 0x004c1d35 in BaseConnection::disableRateLimit() ()
   #11 0x004c1d71 in BaseConnection::~BaseConnection() ()
   #12 0x004ca4b1 in Connection::~Connection() ()
   #13 0x004cdccc in BaseClient::OnClose(NetworkErrorCode) ()
   #14 0x004c1e7a in BaseConnection::internalClose(NetworkErrorCode) ()
   #15 0x004c2131 in BaseConnection::handleEvent(short) ()
   #16 0x004d643d in bufferevent_run_deferred_callbacks_locked 
(cb=, arg=0x87d6000) at bufferevent.c:161
   #17 0x004deb10 in event_process_active_single_queue 
(base=base@entry=0x28dc000, activeq=0x28883f0, 
max_to_process=max_to_process@entry=2147483647, endtime=endtime@entry=0x0) at 
event.c:1675
   #18 0x004df597 in event_process_active (base=0x28dc000) at 
event.c:1738
   #19 event_base_loop (base=0x28dc000, flags=0) at event.c:1961
   #20 0x0040b07c in main ()```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] skanjila opened a new issue #2896: Design of streaming machine learning algorithms in heron

[GitBox]

skanjila opened a new issue #2896: Design of streaming machine learning 
algorithms in heron
URL: https://github.com/apache/incubator-heron/issues/2896
 
 
   The goal of this effort is to design and document streaming real time 
machine learning algorithms that can fit into a storm topology and eventually 
into a heron streamlet based architecture


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kramasamy closed pull request #2895: Nwang/update license in other files

[GitBox]

kramasamy closed pull request #2895: Nwang/update license in other files
URL: https://github.com/apache/incubator-heron/pull/2895
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/config/empty.cc b/config/empty.cc
index 3305740970..8c25c1ea4d 100644
--- a/config/empty.cc
+++ b/config/empty.cc
@@ -1,17 +1,20 @@
-/*
- * Copyright 2015 Twitter, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
  *
  *   http://www.apache.org/licenses/LICENSE-2.0
  *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
  */
 
 // TODO(kramasamy): Can we get rid of this file?
diff --git 
a/eco-heron-examples/src/java/org/apache/heron/examples/eco/sample.properties 
b/eco-heron-examples/src/java/org/apache/heron/examples/eco/sample.properties
index 65a85c4df6..67da8b194d 100644
--- 
a/eco-heron-examples/src/java/org/apache/heron/examples/eco/sample.properties
+++ 
b/eco-heron-examples/src/java/org/apache/heron/examples/eco/sample.properties
@@ -1,16 +1,19 @@
-# // Copyright 2018 Twitter. All rights reserved.
-# //
-# // Licensed under the Apache License, Version 2.0 (the "License");
-# // you may not use this file except in compliance with the License.
-# // You may obtain a copy of the License at
-# //
-# // http://www.apache.org/licenses/LICENSE-2.0
-# //
-# // Unless required by applicable law or agreed to in writing, software
-# // distributed under the License is distributed on an "AS IS" BASIS,
-# // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# // See the License for the specific language governing permissions and
-# // limitations under the License.
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
 
 ecoPropertyOne=thisValueWasSetFromAPropertiesFile
 
diff --git 
a/eco-storm-examples/src/java/org/apache/heron/examples/eco/sample.properties 
b/eco-storm-examples/src/java/org/apache/heron/examples/eco/sample.properties
index 65a85c4df6..e2cd8ad7b9 100644
--- 
a/eco-storm-examples/src/java/org/apache/heron/examples/eco/sample.properties
+++ 
b/eco-storm-examples/src/java/org/apache/heron/examples/eco/sample.properties
@@ -1,16 +1,19 @@
-# // Copyright 2018 Twitter. All rights reserved.
-# //
-# // Licensed under the Apache License, Version 2.0 (the "License");
-# // you may not use this file except in compliance with the License.
-# // You may obtain a copy of the License at
-# //
-# // http://www.apache.org/licenses/LICENSE-2.0
-# //
-# // Unless required by applicable law or agreed to in writing, software
-# // distributed under the License is distributed on an "AS IS" BASIS,
-# // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# // See the License for the specific language governing permissions and
-# // limitations under the License.
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license 

[GitHub] nwangtw commented on a change in pull request #2884: move downloader registry map to yaml

[GitBox]

nwangtw commented on a change in pull request #2884: move downloader registry 
map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#discussion_r186623390
 
 

 ##
 File path: 
heron/downloaders/src/java/org/apache/heron/downloader/DownloadRunner.java
 ##
 @@ -18,17 +18,160 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.CommandLineParser;
+import org.apache.commons.cli.DefaultParser;
+import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.ParseException;
+import org.apache.heron.spi.common.Config;
+import org.apache.heron.spi.common.ConfigLoader;
+
 public final class DownloadRunner {
 
+  public enum DownloaderMode {
+cluster,
+local
+  }
+
+  private enum CliArgs {
+HERON_HOME("heron_home"),
+CONFIG_PATH("config_path"),
+MODE("mode"),
+TOPOLOGY_PACKAGE_URI("topology_package_uri"),
+EXTRACT_DESTINATION("extract_destination");
+
+private String text;
+
+CliArgs(String name) {
+  this.text = name;
+}
+  }
+
+  // Print usage options
+  private static void usage(Options options) {
+HelpFormatter formatter = new HelpFormatter();
+formatter.printHelp(DownloadRunner.class.getSimpleName(), options);
+  }
+
+  // construct command line help options
+  private static Options constructHelpOptions() {
+Options options = new Options();
+Option help = Option.builder("h")
+.desc("List all options and their description")
+.longOpt("help")
+.build();
+
+options.addOption(help);
+return options;
+  }
+
+  // Construct all required command line options
+  private static Options constructCliOptions() {
+Options options = new Options();
+
+Option packageUri = Option.builder("u")
+.desc("Uri indicating from where to download the file")
+.longOpt(CliArgs.TOPOLOGY_PACKAGE_URI.text)
+.hasArgs()
+.argName(CliArgs.TOPOLOGY_PACKAGE_URI.text)
+.required()
+.build();
+
+Option destination = Option.builder("f")
+.desc("Destination to store the downloaded file")
+.longOpt(CliArgs.EXTRACT_DESTINATION.text)
+.hasArgs()
+.argName(CliArgs.EXTRACT_DESTINATION.text)
+.required()
+.build();
+
+Option heronHome = Option.builder("d")
+.desc("Directory where heron is installed")
+.longOpt(CliArgs.HERON_HOME.text)
+.hasArgs()
+.argName("heron home dir")
+.build();
+
+Option configFile = Option.builder("p")
+.desc("Path of the config files")
+.longOpt(CliArgs.CONFIG_PATH.text)
+.hasArgs()
+.argName("config path")
+.build();
+
+// candidates:
+// local: download to client local machine
+// cluster: download into the container in the cloud
+Option mode = Option.builder("m")
+.desc("download mode, cluster or local")
+.longOpt(CliArgs.MODE.text)
+.hasArg()
+.argName("download mode")
+.build();
+
+options.addOption(packageUri);
+options.addOption(destination);
+options.addOption(heronHome);
+options.addOption(configFile);
+options.addOption(mode);
+
+return options;
+  }
+
+
   // takes topology package URI and extracts it to a directory
   public static void main(String[] args) throws Exception {
-if (args.length != 2) {
-  System.err.println("Usage: downloader  
");
+CommandLineParser parser = new DefaultParser();
+Options slaManagerCliOptions = constructCliOptions();
+
+// parse the help options first.
+Options helpOptions = constructHelpOptions();
+CommandLine cmd = parser.parse(helpOptions, args, true);
+if (cmd.hasOption("h")) {
+  usage(slaManagerCliOptions);
   return;
 }
 
-final String uri = args[0];
-final String destination = args[1];
+try {
+  cmd = parser.parse(slaManagerCliOptions, args);
+} catch (ParseException e) {
+  usage(slaManagerCliOptions);
+  throw new RuntimeException("Error parsing command line options: ", e);
+}
+
+DownloaderMode mode = DownloaderMode.cluster;
+if (cmd.hasOption(CliArgs.MODE.text)) {
+  mode = DownloaderMode.valueOf(cmd.getOptionValue(CliArgs.MODE.text, 
null));
+}
+
+Config config;
+switch (mode) {
+  case cluster:
+config = Config.toClusterMode(Config.newBuilder()
+.putAll(ConfigLoader.loadClusterConfig())
+.build());
+break;
+
+  case local:
+if (!cmd.hasOption(CliArgs.HERON_HOME.text) || 
!cmd.hasOption(CliArgs.CONFIG_PATH.text)) {
+  throw new IllegalArgumentException("Missing heron_home or 
config_path argument");
+}
+String heronHome = cmd.getOptionValue(CliArgs.HERON_HOME.text, null);
+String configPath = 

[GitHub] joshfischer1108 commented on issue #2894: fixing test

[GitBox]

joshfischer1108 commented on issue #2894: fixing test
URL: https://github.com/apache/incubator-heron/pull/2894#issuecomment-387156035
 
 
   Just kicked it off again  


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jerrypeng commented on issue #2884: move downloader registry map to yaml

[GitBox]

jerrypeng commented on issue #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#issuecomment-387153148
 
 
   I feel the changes have made the downloader a lot more complicated.  Now we 
have to potentially pass in a cluster mode, heron home path, config path 
because we have to read the downloader.yaml from the conf directory.  However, 
I think the most common use case for the downloader is 
   
   ```
   downloader  
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jerrypeng commented on issue #2884: move downloader registry map to yaml

[GitBox]

jerrypeng commented on issue #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#issuecomment-387153148
 
 
   I feel the changes have made the downloader a lot more complicated.  Now we 
have to pass in a cluster mode or config path because we have to read the 
downloader.yaml.  However, I think the most common use case for the downloader 
is 
   
   ```
   downloader  
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jerrypeng commented on issue #2884: move downloader registry map to yaml

[GitBox]

jerrypeng commented on issue #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#issuecomment-387153148
 
 
   I feel the changes have made the downloader a lot more complicated.  Now we 
have to pass in a cluster mode or config path because we have to read the 
downloader.yaml.  However, I think the most common use case for the downloader 
is 
   
   "downloader  "


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] jerrypeng commented on issue #2884: move downloader registry map to yaml

[GitBox]

jerrypeng commented on issue #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#issuecomment-387148313
 
 
   So the arguments are changed for the downloader right?  Won't this break 
existing usages of the downloader.  Heron standalone/nomad calls the downloader 
to download topology packages.  Can we also support calling the downloader with 
the existing parameters as well? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on issue #2894: fixing test

[GitBox]

nwangtw commented on issue #2894: fixing test
URL: https://github.com/apache/incubator-heron/pull/2894#issuecomment-387137205
 
 
   "(15:41:38) ERROR: build interrupted". :p 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] joshfischer1108 closed pull request #2892: fixing eco parser test

[GitBox]

joshfischer1108 closed pull request #2892: fixing eco parser test
URL: https://github.com/apache/incubator-heron/pull/2892
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/eco/tests/java/org/apache/heron/eco/parser/EcoParserTest.java 
b/eco/tests/java/org/apache/heron/eco/parser/EcoParserTest.java
index 9e665b77b8..035c1d91b5 100644
--- a/eco/tests/java/org/apache/heron/eco/parser/EcoParserTest.java
+++ b/eco/tests/java/org/apache/heron/eco/parser/EcoParserTest.java
@@ -16,7 +16,114 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-\n"
+package org.apache.heron.eco.parser;
+import java.io.ByteArrayInputStream;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.powermock.api.mockito.PowerMockito;
+
+import org.apache.heron.eco.definition.BeanDefinition;
+import org.apache.heron.eco.definition.BeanReference;
+import org.apache.heron.eco.definition.BoltDefinition;
+import org.apache.heron.eco.definition.EcoTopologyDefinition;
+import org.apache.heron.eco.definition.GroupingDefinition;
+import org.apache.heron.eco.definition.PropertyDefinition;
+import org.apache.heron.eco.definition.StreamDefinition;
+
+import static junit.framework.TestCase.assertNotNull;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.*;
+
+
+/**
+ * Unit tests for {@link EcoParser}
+ */
+public class EcoParserTest {
+
+
+  private static final String BOLT_1 = "bolt-1";
+  private static final String BOLT_2 = "bolt-2";
+  private static final String YAML_NO_CONFIG_STR = "# Licensed to the Apache 
Software Foundation"
+  + " (ASF) under one\n"
+  + "# or more contributor license agreements.  See the NOTICE file\n"
+  + "# distributed with this work for additional information\n"
+  + "# regarding copyright ownership.  The ASF licenses this file\n"
+  + "# to you under the Apache License, Version 2.0 (the\n"
+  + "# \"License\"); you may not use this file except in compliance\n"
+  + "# with the License.  You may obtain a copy of the License at\n"
+  + "#\n"
+  + "# http://www.apache.org/licenses/LICENSE-2.0\n;
+  + "#\n"
+  + "# Unless required by applicable law or agreed to in writing, 
software\n"
+  + "# distributed under the License is distributed on an \"AS IS\" 
BASIS,\n"
+  + "# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.\n"
+  + "# See the License for the specific language governing permissions 
and\n"
+  + "# limitations under the License.\n"
+  + "\n"
+  + "---\n"
+  + "\n"
+  + "# topology definition\n"
+  + "# name to be used when submitting\n"
+  + "name: \"yaml-topology\"\n"
+  + "\n"
+  + "# topology configuration\n"
+  + "# this will be passed to the submitter as a map of config options\n"
+  + "#\n"
+  + "# spout definitions\n"
+  + "spouts:\n"
+  + "  - id: \"spout-1\"\n"
+  + "className: \"org.apache.heron.sample.TestWordSpout\"\n"
+  + "parallelism: 1\n"
+  + "\n"
+  + "# bolt definitions\n"
+  + "bolts:\n"
+  + "  - id: \"bolt-1\"\n"
+  + "className: \"org.apache.heron.sample.TestWordCounter\"\n"
+  + "parallelism: 2\n"
+  + "\n"
+  + "  - id: \"bolt-2\"\n"
+  + "className: \"org.apache.heron.sample.LogInfoBolt\"\n"
+  + "parallelism: 1\n"
+  + "\n"
+  + "#stream definitions\n"
+  + "# stream definitions define connections between spouts and bolts.\n"
+  + "# note that such connections can be cyclical\n"
+  + "streams:\n"
+  + "  - name: \"spout-1 --> bolt-1\" # name isn't used (placeholder for 
logging, UI, etc.)\n"
+  + "id: \"connection-1\"\n"
+  + "from: \"spout-1\"\n"
+  + "to: \"bolt-1\"\n"
+  + "grouping:\n"
+  + "  type: FIELDS\n"
+  + "  args: [\"word\"]\n"
+  + "\n"
+  + "  - name: \"bolt-1 --> bolt2\"\n"
+  + "id: \"connection-2\"\n"
+  + "from: \"bolt-1\"\n"
+  + "to: \"bolt-2\"\n"
+  + "grouping:\n"
+  + "  type: SHUFFLE";
+  private static final String YAML_STR = "# Licensed to the Apache Software 
Foundation"
+  + " (ASF) under one\n"
+  + "# or more contributor license agreements.  See the NOTICE file\n"
+  + "# distributed with this work for additional information\n"
+  + "# regarding copyright ownership.  The ASF licenses this file\n"
+  + "# to you under the Apache License, Version 2.0 (the\n"
+  + "# \"License\"); you may not use this file except in compliance\n"
+  + "# with the License.  You 

[GitHub] joshfischer1108 opened a new pull request #2894: fixing test

[GitBox]

joshfischer1108 opened a new pull request #2894: fixing test
URL: https://github.com/apache/incubator-heron/pull/2894
 
 
   @nwangtw Lets try this one.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] joshfischer1108 commented on issue #2892: fixing eco parser test

[GitBox]

joshfischer1108 commented on issue #2892: fixing eco parser test
URL: https://github.com/apache/incubator-heron/pull/2892#issuecomment-387101604
 
 
   @nwangtw Can you look at this CI error please?  I'm not sure where this is 
coming from.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] joshfischer1108 commented on issue #2892: fixing eco parser test

[GitBox]

joshfischer1108 commented on issue #2892: fixing eco parser test
URL: https://github.com/apache/incubator-heron/pull/2892#issuecomment-387085922
 
 
   @nwangtw Didn't realize I had the permissions to restart build.  I don't 
think I did when Heron was in Twitter repo.   Just restarted it.  Thanks for 
bringing this up.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on issue #2892: fixing eco parser test

[GitBox]

nwangtw commented on issue #2892: fixing eco parser test
URL: https://github.com/apache/incubator-heron/pull/2892#issuecomment-387083443
 
 
   Looks like integration test failed. Need to retrigger travis.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on issue #2890: YAML cpp noboost deprecated

[GitBox]

nwangtw commented on issue #2890: YAML cpp noboost deprecated
URL: 
https://github.com/apache/incubator-heron/issues/2890#issuecomment-386988477
 
 
   Related PR:
   https://github.com/apache/incubator-heron/pull/2893/files


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2891: [WIP] Refactor StatefulStorage

[GitBox]

nwangtw commented on a change in pull request #2891: [WIP] Refactor 
StatefulStorage
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r186349909
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,25 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
+  // checkpoint data from older versions to the latest version when necessary.
+  optional string data_version = 3;
+}
+
+// This message encapsulates the info associated with
+// checkpoint metadata of a component
+message CheckpointComponentMetadata {
+  required int32 parallelism = 1;
 
 Review comment:
   @skanjila here is the doc: 
https://docs.google.com/document/d/1p443mcdD607Vf9aCnMq5ESVMFrEuneGlceXmtUSvVMQ/edit#heading=h.myl6mycadhfh


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kramasamy closed pull request #2893: pull yaml cpp for official release page

[GitBox]

kramasamy closed pull request #2893: pull yaml cpp for official release page
URL: https://github.com/apache/incubator-heron/pull/2893
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kramasamy opened a new pull request #2893: pull yaml cpp for official release page

[GitBox]

kramasamy opened a new pull request #2893: pull yaml cpp for official release 
page
URL: https://github.com/apache/incubator-heron/pull/2893
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] joshfischer1108 commented on issue #2892: fixing eco parser test

[GitBox]

joshfischer1108 commented on issue #2892: fixing eco parser test
URL: https://github.com/apache/incubator-heron/pull/2892#issuecomment-386822849
 
 
   @kramasamy  Once master build is fixed I will merge in those changes


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] joshfischer1108 commented on a change in pull request #2892: fixing eco parser test

[GitBox]

joshfischer1108 commented on a change in pull request #2892: fixing eco parser 
test
URL: https://github.com/apache/incubator-heron/pull/2892#discussion_r186265179
 
 

 ##
 File path: eco/tests/java/org/apache/heron/eco/parser/EcoParserTest.java
 ##
 @@ -33,17 +140,17 @@
   + "# spout definitions\n"
   + "spouts:\n"
   + "  - id: \"spout-1\"\n"
-  + "className: \"org.apache.heron.sample.TestWordSpout\"\n"
+  + "className: \"com.twitter.heron.sample.TestWordSpout\"\n"
 
 Review comment:
   @nwangtw 
   oh yeah..  Will fix.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] srkukarni commented on a change in pull request #2891: [WIP] Refactor StatefulStorage

[GitBox]

srkukarni commented on a change in pull request #2891: [WIP] Refactor 
StatefulStorage
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r186254708
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,25 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
+  // checkpoint data from older versions to the latest version when necessary.
+  optional string data_version = 3;
+}
+
+// This message encapsulates the info associated with
+// checkpoint metadata of a component
+message CheckpointComponentMetadata {
+  required int32 parallelism = 1;
 
 Review comment:
   is there any documentation that describes whats this pr aims to do? Without 
it its very hard to understand whats going on


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kramasamy commented on a change in pull request #2892: fixing eco parser test

[GitBox]

kramasamy commented on a change in pull request #2892: fixing eco parser test
URL: https://github.com/apache/incubator-heron/pull/2892#discussion_r186253432
 
 

 ##
 File path: eco/tests/java/org/apache/heron/eco/parser/EcoParserTest.java
 ##
 @@ -33,17 +140,17 @@
   + "# spout definitions\n"
   + "spouts:\n"
   + "  - id: \"spout-1\"\n"
-  + "className: \"org.apache.heron.sample.TestWordSpout\"\n"
+  + "className: \"com.twitter.heron.sample.TestWordSpout\"\n"
   + "parallelism: 1\n"
   + "\n"
   + "# bolt definitions\n"
   + "bolts:\n"
   + "  - id: \"bolt-1\"\n"
-  + "className: \"org.apache.heron.sample.TestWordCounter\"\n"
+  + "className: \"com.twitter.heron.sample.TestWordCounter\"\n"
   + "parallelism: 2\n"
   + "\n"
   + "  - id: \"bolt-2\"\n"
-  + "className: \"org.apache.heron.sample.LogInfoBolt\"\n"
+  + "className: \"com.twitter.heron.sample.LogInfoBolt\"\n"
 
 Review comment:
   same as before.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2892: fixing eco parser test

[GitBox]

nwangtw commented on a change in pull request #2892: fixing eco parser test
URL: https://github.com/apache/incubator-heron/pull/2892#discussion_r186253357
 
 

 ##
 File path: eco/tests/java/org/apache/heron/eco/parser/EcoParserTest.java
 ##
 @@ -33,17 +140,17 @@
   + "# spout definitions\n"
   + "spouts:\n"
   + "  - id: \"spout-1\"\n"
-  + "className: \"org.apache.heron.sample.TestWordSpout\"\n"
+  + "className: \"com.twitter.heron.sample.TestWordSpout\"\n"
 
 Review comment:
   I guess these package names should be reverted?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2892: fixing eco parser test

[GitBox]

nwangtw commented on a change in pull request #2892: fixing eco parser test
URL: https://github.com/apache/incubator-heron/pull/2892#discussion_r186253366
 
 

 ##
 File path: eco/tests/java/org/apache/heron/eco/parser/EcoParserTest.java
 ##
 @@ -16,7 +16,114 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-\n"
+package org.apache.heron.eco.parser;
+import java.io.ByteArrayInputStream;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.powermock.api.mockito.PowerMockito;
+
+import org.apache.heron.eco.definition.BeanDefinition;
+import org.apache.heron.eco.definition.BeanReference;
+import org.apache.heron.eco.definition.BoltDefinition;
+import org.apache.heron.eco.definition.EcoTopologyDefinition;
+import org.apache.heron.eco.definition.GroupingDefinition;
+import org.apache.heron.eco.definition.PropertyDefinition;
+import org.apache.heron.eco.definition.StreamDefinition;
+
+import static junit.framework.TestCase.assertNotNull;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.*;
+
+
+/**
+ * Unit tests for {@link EcoParser}
+ */
+public class EcoParserTest {
+
+
+  private static final String BOLT_1 = "bolt-1";
+  private static final String BOLT_2 = "bolt-2";
+  private static final String YAML_NO_CONFIG_STR = "# Licensed to the Apache 
Software Foundation"
+  + " (ASF) under one\n"
+  + "# or more contributor license agreements.  See the NOTICE file\n"
+  + "# distributed with this work for additional information\n"
+  + "# regarding copyright ownership.  The ASF licenses this file\n"
+  + "# to you under the Apache License, Version 2.0 (the\n"
+  + "# \"License\"); you may not use this file except in compliance\n"
+  + "# with the License.  You may obtain a copy of the License at\n"
+  + "#\n"
+  + "# http://www.apache.org/licenses/LICENSE-2.0\n;
+  + "#\n"
+  + "# Unless required by applicable law or agreed to in writing, 
software\n"
+  + "# distributed under the License is distributed on an \"AS IS\" 
BASIS,\n"
+  + "# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.\n"
+  + "# See the License for the specific language governing permissions 
and\n"
+  + "# limitations under the License.\n"
+  + "\n"
+  + "---\n"
+  + "\n"
+  + "# topology definition\n"
+  + "# name to be used when submitting\n"
+  + "name: \"yaml-topology\"\n"
+  + "\n"
+  + "# topology configuration\n"
+  + "# this will be passed to the submitter as a map of config options\n"
+  + "#\n"
+  + "# spout definitions\n"
+  + "spouts:\n"
+  + "  - id: \"spout-1\"\n"
+  + "className: \"com.twitter.heron.sample.TestWordSpout\"\n"
 
 Review comment:
   And these package names?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunw commented on a change in pull request #2886: [wip] submit topology file by uri

[GitBox]

huijunw commented on a change in pull request #2886: [wip] submit topology file 
by uri
URL: https://github.com/apache/incubator-heron/pull/2886#discussion_r186236468
 
 

 ##
 File path: heron/tools/cli/src/python/submit.py
 ##
 @@ -366,6 +368,16 @@ def submit_cpp(cl_args, unknown_args, tmp_dir):
 
   return launch_topologies(cl_args, topology_file, tmp_dir)
 
+def download(uri):
+  current_path = os.path.realpath(__file__)
+  print current_path
+  tmp_dir = tempfile.mkdtemp()
+  subprocess.call([current_path + "downloader.sh", uri, tmp_dir])
+  suffix = (".jar", ".tar", ".tar.gz", ".pex", ".dylib", ".so")
+  for f in os.listdir(tmp_dir):
+if f.endswith(suffix):
 
 Review comment:
   we need to make an agreement on this with users. the users may assume there 
is only one file, or assume there are multiple files


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2884: move downloader registry map to yaml

[GitBox]

nlu90 commented on issue #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#issuecomment-386751809
 
 
   @kramasamy  This change may break your downloader usage, please update your 
code accordingly using the new `downloader.yaml` file.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2891: [WIP] Refactor StatefulStorage

[GitBox]

nwangtw commented on a change in pull request #2891: [WIP] Refactor 
StatefulStorage
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r186226835
 
 

 ##
 File path: 
heron/spi/src/java/org/apache/heron/spi/statefulstorage/IStatefulStorage.java
 ##
 @@ -16,37 +16,65 @@
 
 import java.util.Map;
 
-import org.apache.heron.proto.system.PhysicalPlans;
-
+/**
+ * The interface of all storage classes for checkpoints.
+ * For each checkpoint, two types of data are stored:
+ * - Component Meta Data (one per component).
+ * - Instance Checkpoint Data (one per instance or patition)
+ * Each Stateful Storage implementation needs to handle them accordingly.
+ */
 public interface IStatefulStorage {
   /**
* Initialize the Stateful Storage
-   *
+   * @param topologyName The name of the topology.
* @param conf An unmodifiableMap containing basic configuration
-   * Attempts to modify the returned map,
-   * whether direct or via its collection views, result in an 
UnsupportedOperationException.
*/
-  void init(Map conf) throws StatefulStorageException;
+  void init(String topologyName, final Map conf)
+  throws StatefulStorageException;
 
   /**
* Closes the Stateful Storage
*/
   void close();
 
-  // Store the checkpoint
-  void store(Checkpoint checkpoint) throws StatefulStorageException;
-
-  // Retrieve the checkpoint
-  Checkpoint restore(String topologyName, String checkpointId,
- PhysicalPlans.Instance instanceInfo) throws 
StatefulStorageException;
-
-  // TODO(mfu): We should refactor all interfaces in IStatefulStorage,
-  // TODO(mfu): instead providing Class Checkpoint, we should provide an 
Context class,
-  // TODO(mfu): It should:
-  // TODO(mfu): 1. Provide meta data access, like topologyName
-  // TODO(mfu): 2. Provide utils method to parse the protobuf object, like 
getTaskId()
-  // TODO(mfu): 3. Common methods, like getCheckpointDir()
-  // Dispose the checkpoint
-  void dispose(String topologyName, String oldestCheckpointId, boolean 
deleteAll)
-  throws StatefulStorageException;
+  /**
+   * Store instance checkpoint.
+   * @param info The information (reference key) for the checkpoint partition.
+   * @param checkpoint The checkpoint data.
+   */
+  void storeCheckpoint(final CheckpointPartitionInfo info, final Checkpoint 
checkpoint)
+  throws StatefulStorageException;
+
+  /**
+   * Retrieve instance checkpoint.
+   * @param info The information (reference key) for the checkpoint partition.
+   * @return The checkpoint data from the specified blob id.
+   */
+  Checkpoint restoreCheckpoint(final CheckpointPartitionInfo info) throws 
StatefulStorageException;
+
+  /**
+   * Store medata data for component. Ideally this function should only be 
called once
+   * for each component.
+   * @param info The information (reference key) for the checkpoint partition.
+   * @param metadata The checkpoint metadata from a component.
+   */
+  void storeComponentMetaData(final CheckpointPartitionInfo info, final 
CheckpointMetadata metadata)
 
 Review comment:
   Good point!
   
   Then storage implementation need to make this decision.
   
   Instance 0 should be responsible for the meta data in distributed storages; 
and for local storages, all instances need to export the data (and the data 
needs to be stored in the instance/partition directory). StatefulStorage 
implementation should know the information and handle the function correctly.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on a change in pull request #2891: [WIP] Refactor StatefulStorage

[GitBox]

nlu90 commented on a change in pull request #2891: [WIP] Refactor 
StatefulStorage
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r186222743
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,25 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
+  // checkpoint data from older versions to the latest version when necessary.
+  optional string data_version = 3;
+}
+
+// This message encapsulates the info associated with
+// checkpoint metadata of a component
+message CheckpointComponentMetadata {
+  required int32 parallelism = 1;
 
 Review comment:
   kk, I got the idea here. Just one simple correction, the stateful data 
directory is in the `topology/checkpoint_id/component_name/task_id` format. So 
this component metadata is stored  at the dir 
`topology/checkpoint_id/component_name`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on a change in pull request #2891: [WIP] Refactor StatefulStorage

[GitBox]

nlu90 commented on a change in pull request #2891: [WIP] Refactor 
StatefulStorage
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r186222743
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,25 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
+  // checkpoint data from older versions to the latest version when necessary.
+  optional string data_version = 3;
+}
+
+// This message encapsulates the info associated with
+// checkpoint metadata of a component
+message CheckpointComponentMetadata {
+  required int32 parallelism = 1;
 
 Review comment:
   kk


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw commented on a change in pull request #2891: [WIP] Refactor StatefulStorage

[GitBox]

nwangtw commented on a change in pull request #2891: [WIP] Refactor 
StatefulStorage
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r186219947
 
 

 ##
 File path: heron/proto/ckptmgr.proto
 ##
 @@ -171,7 +175,25 @@ message CleanStatefulCheckpointResponse {
   repeated string cleaned_checkpoint_ids = 2;
 }
 
-// stmgr -> ckptmgr messages
+/*
+ * stmgr -> ckptmgr messages
+ */
+
+// This message encapsulates the info associated with
+// state of an instance/partition
+message InstanceStateCheckpoint {
+  required string checkpoint_id = 1;
+  required bytes state = 2;
+  // A version string that can be specified by user. It can be used to 
translate
+  // checkpoint data from older versions to the latest version when necessary.
+  optional string data_version = 3;
+}
+
+// This message encapsulates the info associated with
+// checkpoint metadata of a component
+message CheckpointComponentMetadata {
+  required int32 parallelism = 1;
 
 Review comment:
   kk. component name could be useful for viewing data. Will add.
   
   Firstly, stateful data is organized around components (the schema should be 
the same for all instances/partitions of the component and could be different 
between components). It is overall a 3-level tree structure: topology is the 
root, component is the mid level and instance/partition is the leaf level. We 
need to store component level data in storage.
   
   Secondly, the parallelism value is important to detect the plan changes and 
handle the change correctly. In our first state, we can drop the stateful data 
when parallelism doesn't match. In future, we should handle repartition based 
on the old and new parallelisms of the component.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2883: check checkpoint file exists before deleting it

[GitBox]

nlu90 closed pull request #2883: check checkpoint file exists before deleting it
URL: https://github.com/apache/incubator-heron/pull/2883
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/heron/statefulstorages/src/java/org/apache/heron/statefulstorage/localfs/LocalFileSystemStorage.java
 
b/heron/statefulstorages/src/java/org/apache/heron/statefulstorage/localfs/LocalFileSystemStorage.java
index 13f4d5c4bd..7b4735c874 100644
--- 
a/heron/statefulstorages/src/java/org/apache/heron/statefulstorage/localfs/LocalFileSystemStorage.java
+++ 
b/heron/statefulstorages/src/java/org/apache/heron/statefulstorage/localfs/LocalFileSystemStorage.java
@@ -109,17 +109,21 @@ public void dispose(String topologyName, String 
oldestCheckpointPreserved,
   }
 } else {
   String[] names = new File(topologyCheckpointRoot).list();
-  for (String name : names) {
-if (name.compareTo(oldestCheckpointPreserved) < 0) {
-  FileUtils.deleteDir(new File(topologyCheckpointRoot, name), true);
+  if (names == null) {
+LOG.warning("There is no such checkpoint root path: " + 
topologyCheckpointRoot);
+  } else {
+for (String name : names) {
+  if (name.compareTo(oldestCheckpointPreserved) < 0) {
+FileUtils.deleteDir(new File(topologyCheckpointRoot, name), true);
+  }
 }
-  }
 
-  // Do a double check. Now all checkpoints with smaller checkpoint id 
should be cleaned
-  names = new File(topologyCheckpointRoot).list();
-  for (String name : names) {
-if (name.compareTo(oldestCheckpointPreserved) < 0) {
-  throw new StatefulStorageException("Failed to delete " + name);
+// Do a double check. Now all checkpoints with smaller checkpoint id 
should be cleaned
+names = new File(topologyCheckpointRoot).list();
+for (String name : names) {
+  if (name.compareTo(oldestCheckpointPreserved) < 0) {
+throw new StatefulStorageException("Failed to delete " + name);
+  }
 }
   }
 }


 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on a change in pull request #2891: [WIP] Refactor StatefulStorage

[GitBox]

nlu90 commented on a change in pull request #2891: [WIP] Refactor 
StatefulStorage
URL: https://github.com/apache/incubator-heron/pull/2891#discussion_r186203313
 
 

 ##
 File path: 
heron/spi/src/java/org/apache/heron/spi/statefulstorage/CheckpointMetadata.java
 ##
 @@ -0,0 +1,39 @@
+// Copyright 2017 Twitter. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.heron.spi.statefulstorage;
+
+import org.apache.heron.proto.ckptmgr.CheckpointManager;
+
+/**
+ * The checkpoint metadata for a component.
+ */
+public class CheckpointMetadata {
+  private CheckpointManager.CheckpointComponentMetadata metadata;
+  private int nBytes;
+
+  public CheckpointMetadata(int parallelism) {
+//this.metadata.setParallelism(parallelism);
 
 Review comment:
   Please remove this line if it's not used. Otherwise add some TODO comments


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw opened a new pull request #2891: [WIP] Refactor StatefulStorage

[GitBox]

nwangtw opened a new pull request #2891: [WIP] Refactor StatefulStorage
URL: https://github.com/apache/incubator-heron/pull/2891
 
 
   - Refactor Stateful Storage to be more independent of topology
   - Refactor data structures.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] comes5 opened a new issue #2890: Error building with Bazel

[GitBox]

comes5 opened a new issue #2890: Error building with Bazel
URL: https://github.com/apache/incubator-heron/issues/2890
 
 
   After cloning the repository: `git clone --branch 0.17.8 
https://github.com/apache/incubator-heron.git`
   I started building with Bazel, but there was an error:
   `ERROR: 
/media/sf_VBoxFolder/incubator-heron/heron/instance/src/cpp/BUILD:3:1: no such 
package '@com_github_jbeder_yaml_cpp//': Error downloading 
[https://storage.googleapis.com/heron-packages/yaml-cpp-noboost.tar.gz] to 
/home/alberto/.cache/bazel/_bazel_alberto/55d63e2b32405818ff4fc73c3f078816/external/com_github_jbeder_yaml_cpp/yaml-cpp-noboost.tar.gz:
 GET returned 400 Bad Request and referenced by 
'//heron/instance/src/cpp:heron-cpp-instance'
   ERROR: Analysis of target '//heron/instance/src/cpp:heron-cpp-instance' 
failed; build aborted: no such package '@com_github_jbeder_yaml_cpp//': Error 
downloading 
[https://storage.googleapis.com/heron-packages/yaml-cpp-noboost.tar.gz] to 
/home/alberto/.cache/bazel/_bazel_alberto/55d63e2b32405818ff4fc73c3f078816/external/com_github_jbeder_yaml_cpp/yaml-cpp-noboost.tar.gz:
 GET returned 400 Bad Request
   INFO: Elapsed time: 4.749s
   INFO: 0 processes.
   FAILED: Build did NOT complete successfully (1 packages loaded)
   `
   It seems that yaml-cpp-noboost.tar.gz can't be downloaded from googleapi 
storage.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 closed pull request #2888: Update license text in scala files

[GitBox]

nlu90 closed pull request #2888: Update license text in scala files
URL: https://github.com/apache/incubator-heron/pull/2888
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/examples/src/scala/org/apache/heron/examples/SomeHeronBolt.scala 
b/examples/src/scala/org/apache/heron/examples/SomeHeronBolt.scala
index e679433122..616bd3c2fe 100644
--- a/examples/src/scala/org/apache/heron/examples/SomeHeronBolt.scala
+++ b/examples/src/scala/org/apache/heron/examples/SomeHeronBolt.scala
@@ -1,16 +1,21 @@
-//  Copyright 2016 Twitter. All rights reserved.
-//
-//  Licensed under the Apache License, Version 2.0 (the "License");
-//  you may not use this file except in compliance with the License.
-//  You may obtain a copy of the License at
-//
-//  http://www.apache.org/licenses/LICENSE-2.0
-//
-//  Unless required by applicable law or agreed to in writing, software
-//  distributed under the License is distributed on an "AS IS" BASIS,
-//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-//  See the License for the specific language governing permissions and
-//  limitations under the License
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
 package org.apache.heron.examples
 import java.util.{Map => JMap}
 import java.lang.{Long => JLong, Double => JDouble}
diff --git a/examples/src/scala/org/apache/heron/examples/SomeStormBolt.scala 
b/examples/src/scala/org/apache/heron/examples/SomeStormBolt.scala
index 5f5a03e6e7..78d6a1d86f 100644
--- a/examples/src/scala/org/apache/heron/examples/SomeStormBolt.scala
+++ b/examples/src/scala/org/apache/heron/examples/SomeStormBolt.scala
@@ -1,16 +1,21 @@
-//  Copyright 2016 Twitter. All rights reserved.
-//
-//  Licensed under the Apache License, Version 2.0 (the "License");
-//  you may not use this file except in compliance with the License.
-//  You may obtain a copy of the License at
-//
-//  http://www.apache.org/licenses/LICENSE-2.0
-//
-//  Unless required by applicable law or agreed to in writing, software
-//  distributed under the License is distributed on an "AS IS" BASIS,
-//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-//  See the License for the specific language governing permissions and
-//  limitations under the License
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
 package org.apache.heron.examples
 import java.util
 
diff --git 
a/examples/src/scala/org/apache/heron/examples/streamlet/scala/ScalaClassicalMusicTopology.scala
 
b/examples/src/scala/org/apache/heron/examples/streamlet/scala/ScalaClassicalMusicTopology.scala
index 1c94d10918..f155bcce35 100644
--- 
a/examples/src/scala/org/apache/heron/examples/streamlet/scala/ScalaClassicalMusicTopology.scala
+++ 
b/examples/src/scala/org/apache/heron/examples/streamlet/scala/ScalaClassicalMusicTopology.scala
@@ -1,16 +1,21 @@
-//  Copyright 2018 Twitter. All rights reserved.
-//
-//  Licensed under the Apache License, Version 2.0 (the "License");
-//  you may not use this file except in compliance with the License.
-//  You may obtain a copy of the License at
-//
-//  http://www.apache.org/licenses/LICENSE-2.0
-//
-//  Unless required by applicable law or agreed to in writing, software

[GitHub] comes5 commented on issue #2877: Compilation error in a container

[GitBox]

comes5 commented on issue #2877: Compilation error in a container
URL: 
https://github.com/apache/incubator-heron/issues/2877#issuecomment-386245751
 
 
   Solved, Bazel couldn't find the .git directory which I had removed. One 
question though: do I have to build everything if I modify only one component 
and I want to test it?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw opened a new pull request #2889: Update license text in python files

[GitBox]

nwangtw opened a new pull request #2889: Update license text in python files
URL: https://github.com/apache/incubator-heron/pull/2889
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nwangtw opened a new pull request #2888: Update license text in scala files

[GitBox]

nwangtw opened a new pull request #2888: Update license text in scala files
URL: https://github.com/apache/incubator-heron/pull/2888
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on a change in pull request #2886: [wip] submit topology file by uri

[GitBox]

nlu90 commented on a change in pull request #2886: [wip] submit topology file 
by uri
URL: https://github.com/apache/incubator-heron/pull/2886#discussion_r185703931
 
 

 ##
 File path: heron/tools/cli/src/python/submit.py
 ##
 @@ -366,6 +368,16 @@ def submit_cpp(cl_args, unknown_args, tmp_dir):
 
   return launch_topologies(cl_args, topology_file, tmp_dir)
 
+def download(uri):
+  current_path = os.path.realpath(__file__)
+  print current_path
+  tmp_dir = tempfile.mkdtemp()
+  subprocess.call([current_path + "downloader.sh", uri, tmp_dir])
+  suffix = (".jar", ".tar", ".tar.gz", ".pex", ".dylib", ".so")
+  for f in os.listdir(tmp_dir):
+if f.endswith(suffix):
 
 Review comment:
   If there are multiple files in this tmp directory, then this `download` 
method will return the first file it encountered ending with a valid suffix. 
But is this returned file truly a valid topology file?
   
   If there will be only one file downloaded to this directory, I suggest we 
remove this `for` loop.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu opened a new pull request #2886: [wip] submit topology file by uri

[GitBox]

huijunwu opened a new pull request #2886: [wip] submit topology file by uri
URL: https://github.com/apache/incubator-heron/pull/2886
 
 
   replace #2274
   after #2884
   
   `$ heron submit [options] cluster/[role]/[env] topology-file-uri 
topology-class-name [topology-args]`
   
   for example:
   `$ heron submit [options] cluster/[role]/[env] http://some-uri 
topology-class-name [topology-args]`


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] kramasamy commented on issue #2879: remove thrift and scribe to prepare for apache release

[GitBox]

kramasamy commented on issue #2879: remove thrift and scribe to prepare for 
apache release
URL: https://github.com/apache/incubator-heron/pull/2879#issuecomment-386197899
 
 
   @nlu90 - done. updating the master.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2884: move downloader registry map to yaml

[GitBox]

nlu90 commented on issue #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#issuecomment-386154314
 
 
   LGTM. Let's see if @kramasamy has any comments.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] nlu90 commented on issue #2884: move downloader registry map to yaml

[GitBox]

nlu90 commented on issue #2884: move downloader registry map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#issuecomment-386154314
 
 
   LGTM


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu commented on a change in pull request #2884: move downloader registry map to yaml

[GitBox]

huijunwu commented on a change in pull request #2884: move downloader registry 
map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#discussion_r185668816
 
 

 ##
 File path: 
heron/downloaders/src/java/org/apache/heron/downloader/DownloadRunner.java
 ##
 @@ -38,7 +184,14 @@ public static void main(String[] args) throws Exception {
   file.mkdirs();
 }
 
-final Downloader downloader = 
Registry.get().getDownloader(topologyLocation);
+Map downloaders = new HashMap<>();
+for (Map.Entry e
 
 Review comment:
   updated


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] huijunwu commented on a change in pull request #2884: move downloader registry map to yaml

[GitBox]

huijunwu commented on a change in pull request #2884: move downloader registry 
map to yaml
URL: https://github.com/apache/incubator-heron/pull/2884#discussion_r185668803
 
 

 ##
 File path: 
heron/downloaders/src/java/org/apache/heron/downloader/DownloadRunner.java
 ##
 @@ -38,7 +184,14 @@ public static void main(String[] args) throws Exception {
   file.mkdirs();
 }
 
-final Downloader downloader = 
Registry.get().getDownloader(topologyLocation);
+Map downloaders = new HashMap<>();
+for (Map.Entry e
+: ((Map) 
config.get(Key.DOWNLOADER_PROTOCOLS)).entrySet()) {
+  Class clazz = Class.forName((String) e.getValue());
+  downloaders.put(e.getKey(), clazz);
+}
+
+final Downloader downloader = Registry.getDownloader(downloaders, 
topologyLocation);
 
 Review comment:
   updated


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services