[GitHub] storm pull request #2892: Added in better docs for local mode testing.
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2892 ---
[GitHub] storm pull request #2891: STORM-3269: Update version of httpclient, and fix ...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2891 ---
[GitHub] storm pull request #2882: STORM-3260: Add in support to print some state
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2882 ---
[GitHub] storm pull request #2893: STORM-3272 allow worker-launcher to delete dead sy...
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/2893#discussion_r227445654
--- Diff: storm-core/src/native/worker-launcher/impl/worker-launcher.c ---
@@ -593,14 +593,17 @@ int recursive_delete(const char *path, int
supervisor_owns_dir) {
return UNABLE_TO_BUILD_PATH;
}
+ struct stat file_stat;
+
if(access(path, F_OK) != 0) {
if(errno == ENOENT) {
- return 0;
-}
-// Can probably return here, but we'll try to lstat anyway.
- }
+ // we need to handle symlinks that target missing files.
+ if((lstat(path, _stat) != 0) || ((file_stat.st_mode & S_IFMT)
!= S_IFLNK)) {
+ return 0;
+ }
--- End diff --
Thanks. And I like that you have text that differentiates this from the
other `lstat` error case.
---
[GitHub] storm pull request #2893: STORM-3272 allow worker-launcher to delete dead sy...
Github user agresch commented on a diff in the pull request:
https://github.com/apache/storm/pull/2893#discussion_r227434073
--- Diff: storm-core/src/native/worker-launcher/impl/worker-launcher.c ---
@@ -593,14 +593,17 @@ int recursive_delete(const char *path, int
supervisor_owns_dir) {
return UNABLE_TO_BUILD_PATH;
}
+ struct stat file_stat;
+
if(access(path, F_OK) != 0) {
if(errno == ENOENT) {
- return 0;
-}
-// Can probably return here, but we'll try to lstat anyway.
- }
+ // we need to handle symlinks that target missing files.
+ if((lstat(path, _stat) != 0) || ((file_stat.st_mode & S_IFMT)
!= S_IFLNK)) {
+ return 0;
+ }
--- End diff --
@d2r @revans2 - added logging
---
[GitHub] storm pull request #2893: STORM-3272 allow worker-launcher to delete dead sy...
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2893#discussion_r227418284
--- Diff: storm-core/src/native/worker-launcher/impl/worker-launcher.c ---
@@ -593,14 +593,17 @@ int recursive_delete(const char *path, int
supervisor_owns_dir) {
return UNABLE_TO_BUILD_PATH;
}
+ struct stat file_stat;
+
if(access(path, F_OK) != 0) {
if(errno == ENOENT) {
- return 0;
-}
-// Can probably return here, but we'll try to lstat anyway.
- }
+ // we need to handle symlinks that target missing files.
+ if((lstat(path, _stat) != 0) || ((file_stat.st_mode & S_IFMT)
!= S_IFLNK)) {
+ return 0;
+ }
--- End diff --
+1 to logging the failure. The only time we get an ENOENT and lstat fails
is either there was a race that the file was deleted out from under us (which
should be rare so logging it is fine) or if something truly bad happened, and
we have another problem we need to debug.
---
[GitHub] storm pull request #2881: STORM-3259: NUMA Support for Storm
Github user govind-menon commented on a diff in the pull request:
https://github.com/apache/storm/pull/2881#discussion_r227413704
--- Diff: storm-client/src/jvm/org/apache/storm/Config.java ---
@@ -1041,6 +1041,19 @@
@isPositiveNumber
@NotNull
public static final String SUPERVISOR_WORKER_TIMEOUT_SECS =
"supervisor.worker.timeout.secs";
+
+/**
+ * A map with blobstore keys mapped to each NUMA Node on the
supervisor that will be used
+ * by scheduler. CPUs, memory and ports available on each NUMA node
will be provided.
+ * Each supervisor will have different map of NUMAs.
+ * Example: "supervisor.numa.meta": { "Numas": [
+ * {"Id": 0, "MemoryInMB": 122880, "Cores": [ 0, 12, 1, 13, 2, 14, 3,
15, 4, 16, 5, 17],
+ * "Ports": [6700, 6701]},
+ * {"Id": 1, "MemoryInMB": 122880, "Cores": [ 6, 18, 7, 19, 8, 20, 9,
21, 10, 22, 11, 23],
+ * "Ports": [6702, 6703]}]}
+ */
+public static final String SUPERVISOR_NUMA_META =
"supervisor.numa.meta";
--- End diff --
There is verification that we do on reading the config that all the fields
are present and exceptions that get thrown for invalid values.
---
[GitHub] storm pull request #2893: STORM-3272 allow worker-launcher to delete dead sy...
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/2893#discussion_r227412685
--- Diff: storm-core/src/native/worker-launcher/impl/worker-launcher.c ---
@@ -593,14 +593,17 @@ int recursive_delete(const char *path, int
supervisor_owns_dir) {
return UNABLE_TO_BUILD_PATH;
}
+ struct stat file_stat;
+
if(access(path, F_OK) != 0) {
if(errno == ENOENT) {
- return 0;
-}
-// Can probably return here, but we'll try to lstat anyway.
- }
+ // we need to handle symlinks that target missing files.
+ if((lstat(path, _stat) != 0) || ((file_stat.st_mode & S_IFMT)
!= S_IFLNK)) {
+ return 0;
+ }
--- End diff --
If we fail to `lstat` the path, do we want to log a message as we do below?
---
[GitHub] storm pull request #2894: STORM-3273: Remove storm.local.hostname from topol...
GitHub user revans2 opened a pull request: https://github.com/apache/storm/pull/2894 STORM-3273: Remove storm.local.hostname from topology conf. You can merge this pull request into a Git repository by running: $ git pull https://github.com/revans2/incubator-storm STORM-3273 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2894.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2894 commit 72be7085f8d9357295cd0991d7b4d0124d614cd7 Author: Robert (Bobby) Evans Date: 2018-10-23T14:11:28Z STORM-3273: Remove storm.local.hostname from topology conf. ---
[GitHub] storm pull request #2893: STORM-3272 allow worker-launcher to delete dead sy...
GitHub user agresch opened a pull request: https://github.com/apache/storm/pull/2893 STORM-3272 allow worker-launcher to delete dead symlinks When a topology folder contains a symlink to a missing file, the worker-launcher would not remove the symlink. This prevents force delete from deleting the folder when cleaning up a topology. When a cluster is short on resources, this can cause all sorts of issues on the supervisor when rescheduling the same topology when this folder still exists. You can merge this pull request into a Git repository by running: $ git pull https://github.com/agresch/storm agresch_storm-3272 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2893.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2893 commit 131344777ae4cbc4ca47952b3a95f35b11713f23 Author: Aaron Gresch Date: 2018-10-23T13:57:52Z STORM-3272 allow worker-launcher to delete dead symlinks ---
Re: [VOTE] Release Apache Storm 2.0.0 (rc3)
Great, thanks Bobby On Mon, 22 Oct 2018 at 18:31, Bobby Evans wrote: > Julien, > > I have put up pull requests for the docs and for fixing some of the issues > with LocalCluster that you found. > > https://github.com/apache/storm/pull/2891 > > https://github.com/apache/storm/pull/2892 > > The VersionInfo change is a blocker and we should fix it before releasing > (Sorry Taylor). > > For the other stuff if you find more issues we can move it to a different > thread and work through them. > > Thanks, > > Bobby > > On Mon, Oct 22, 2018 at 9:23 AM Bobby Evans wrote: > > > I'll look at upgrading that version of http client too. > > > > On Mon, Oct 22, 2018 at 9:15 AM Julien Nioche < > > lists.digitalpeb...@gmail.com> wrote: > > > >> Hi, > >> > >> I've looked into it a bit more and found that SC had a dependency on > >> storm-core and not storm-client; I've fixed this in 40612a3... > >> < > >> > https://github.com/DigitalPebble/storm-crawler/commit/40612a3588d66e1d410a70b1c7e5db58d5c2ba4d > >> > > >> however > >> this doesn't affect the issues I had last week. > >> > >> *httpclient dependency conflict* > >> As seen last week, this is not shaded by Storm and the version used > (4.3.3 > >> < > >> > https://github.com/apache/storm/blob/ce984cd31a16e7fe4b983659005f1f7648455404/pom.xml#L266 > >> >) > >> is quite old. Even within Storm, the Storm-SOLR module uses a more > recent > >> one (4.5 > >> < > >> > https://github.com/apache/storm/blob/master/external/storm-solr/pom.xml#L64 > >> >). > >> StormCrawler needs at least 4.5.5 > >> < > >> > https://github.com/DigitalPebble/storm-crawler/blob/master/core/pom.xml#L26 > >> >. > >> I expect other Storm users would use *httpclient* and have a similar > >> problem. Unless I am missing something, I can see the following > solutions > >> sorted by how convenient they are to me as a user: > >> > >>1. the dependency is shaded by Storm > >>2. the dependency is upgraded to 4.5.5 by Storm > >>3. the dependency is shaded by StormCrawler > >> > >> Obviously, I'd rather not have to deal with (3) and anyone using > >> httpclient with Storm would have to do the same. > >> > >> Note: I can get my topology to work by specifying a protocol > >> implementation > >> based on OKHttp > >> * http.protocol.implementation: > >> "com.digitalpebble.stormcrawler.protocol.okhttp.HttpProtocol"* > >> * https.protocol.implementation: > >> "com.digitalpebble.stormcrawler.protocol.okhttp.HttpProtocol"* > >> > >> *LocalCluster* > >> Since removing the dependency on storm-core, I can't use LocalCluster > >> directly. I'll create a separate branch on my test repo to try to > >> reproduce > >> the issue. > >> > >> *Documentation for Local mode* > >> http://storm.apache.org/releases/2.0.0-SNAPSHOT/Local-mode.html > >> does not mention *--local-ttl *would be good to document it and indicate > >> what the default value is otherwise users might wonder why their > >> topologies > >> run for 20 secs only. Personally, I'd rather be able to have a default > >> behaviour where the topology runs forever or at least be able to > >> deactivate > >> the TTL completely by setting it to -1. > >> > >> *ConfigurableTopology* > >> I am getting a different behavior between the original > >> ConfigurableTopology from > >> StormCrawler > >> < > >> > https://github.com/DigitalPebble/storm-crawler/blob/master/core/src/main/java/com/digitalpebble/stormcrawler/ConfigurableTopology.java > >> > > >> and when I extend the one in Storm > >> < > >> > https://github.com/apache/storm/blob/master/storm-client/src/jvm/org/apache/storm/topology/ConfigurableTopology.java > >> >; > >> with the latter, any configuration found in the conf files passed in > args > >> to the command line are added to the default values I provide instead of > >> replacing them. I'll investigate that further and open an issue if I > find > >> a > >> bug. > >> > >> *Distributed mode* > >> I managed to launch the various services and run my test topology in > >> remote > >> mode (by changing the protocol implementation as explained above) > >> > >> *Flux* > >> http://storm.apache.org/releases/2.0.0-SNAPSHOT/flux.html tells me to > run > >> > >> storm jar myTopology-0.1.0-SNAPSHOT.jar org.apache.storm.flux.Flux > >> --local my_config.yaml > >> > >> > >> > >> *apache-storm-2.0.0/bin/storm jar target/2-1.0-SNAPSHOT.jar > >> org.apache.storm.flux.Flux --local crawler.flux* > >> > >> but am getting > >> > >> *15:07:26.206 [main] ERROR o.a.s.f.Flux - To run in local mode run with > >> 'storm local' instead of 'storm jar'* > >> > >> *so *I tried both > >> > >> apache-storm-2.0.0/bin/storm local target/2-1.0-SNAPSHOT.jar > >> org.apache.storm.flux.Flux --local crawler.flux > >> > >> and > >> > >> *apache-storm-2.0.0/bin/storm local target/2-1.0-SNAPSHOT.jar > >> org.apache.storm.flux.Flux crawler.flux* > >> but in both cases I'm getting > >> > >> *15:12:06.784 [main] ERROR o.a.s.f.Flux - To run in local mode run with > >> 'storm local' instead of 'storm
