[GitHub] storm pull request #2782: STORM-3164: Fix usage of traceback.format_exc in m...
GitHub user mal opened a pull request: https://github.com/apache/storm/pull/2782 STORM-3164: Fix usage of traceback.format_exc in multilang (backport) Backport to 1.x. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mal/storm STORM-3164-1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2782.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 #2782 commit 9394f82f6da85fac89e52ce50073cf337bb52990 Author: Mal Graty Date: 2018-07-29T16:35:15Z Fix usage of traceback.format_exc in multilang ---
[GitHub] storm pull request #2781: STORM-3163: Make ShellBolt logger setup calls occu...
GitHub user mal opened a pull request: https://github.com/apache/storm/pull/2781 STORM-3163: Make ShellBolt logger setup calls occur in-thread to support MDC (BACKPORT) Backport to 1.x. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mal/storm STORM-3163-1.x-branch Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2781.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 #2781 commit cb008f89502b3891d2c23f534fd7d6bb81fc0e83 Author: Mal Graty Date: 2018-07-29T17:22:43Z Setup ShellBolt logger in the thread using it Prior to this the logger set up would occur in a separate thread (the parent), to the one doing the logging. This meant that MDC values, which are thread local, could not persist from the set up phase. By doing set up in the same thread, MDC values are now reliably available during log calls. ---
[GitHub] storm pull request #2780: STORM-3164: Fix usage of traceback.format_exc in m...
GitHub user mal opened a pull request:
https://github.com/apache/storm/pull/2780
STORM-3164: Fix usage of traceback.format_exc in multilang
Misuse of `traceback.format_exc` was causing exceptions thrown in python
bolts to throw `TypeError`s during attempts to handle them. Fixing the calls
results in expect behaviour.
Examples of the change can be seen below where the Storm side messages have
been mocked so as to directly inspect the python side response. Lines prefixed
with `#` are the prompt, `>` are `stdin` (manually entered) and `>` denotes
`stdout`. Unprefixed lines are `stderr`.
https://issues.apache.org/jira/browse/STORM-3164
**Test Code**
```py
from storm import Bolt
class TestBolt(Bolt):
def process(self, tup):
raise Exception('harakiri!')
if __name__ == '__main__':
TestBolt().run()
```
```sh
while true; do
printf '> ' > /dev/tty
read line
echo "$line"
sleep 0.1
printf '\n' # trigger sigpipe if python is dead
done | python3 multilang/resources/bolt.py | sed 's/^/< /'
```
**Before**
```
# sh ./bolttest
> {"conf": {}, "context": {}, "pidDir": "/tmp"}
> end
< {"pid": 10643}
< end
> {"id":1, "comp": 2, "stream": 3, "task": 4, "tuple": ["foo", "bar"]}
> end
Traceback (most recent call last):
File "/usr/src/test/storm.py", line 198, in run
self.process(tup)
File "bolt.py", line 6, in process
raise Exception('harakiri!')
Exception: harakiri!
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "bolt.py", line 10, in
TestBolt().run()
File "/usr/src/test/storm.py", line 200, in run
reportError(traceback.format_exc(e))
File "/usr/lib/python3.4/traceback.py", line 256, in format_exc
return "".join(format_exception(*sys.exc_info(), limit=limit,
chain=chain))
File "/usr/lib/python3.4/traceback.py", line 181, in format_exception
return list(_format_exception_iter(etype, value, tb, limit, chain))
File "/usr/lib/python3.4/traceback.py", line 153, in
_format_exception_iter
yield from _format_list_iter(_extract_tb_iter(tb, limit=limit))
File "/usr/lib/python3.4/traceback.py", line 18, in _format_list_iter
for filename, lineno, name, line in extracted_list:
File "/usr/lib/python3.4/traceback.py", line 58, in
_extract_tb_or_stack_iter
while curr is not None and (limit is None or n < limit):
TypeError: unorderable types: int() < Exception()
```
**After**
```
# sh ./bolttest
> {"conf": {}, "context": {}, "pidDir": "/tmp"}
> end
< {"pid": 10644}
< end
> {"id":1, "comp": 2, "stream": 3, "task": 4, "tuple": ["foo", "bar"]}
> end
< {"command": "error", "msg": "Traceback (most recent call last):\n File
\"/usr/src/test/storm.py\", line 198, in run\nself.process(tup)\n File
\"bolt.py\", line 6, in process\nraise Exception('harakiri!')\nException:
harakiri!\n"}
< end
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mal/storm STORM-3164
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2780.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 #2780
commit 5ed30b564f37deba00318f330c2b6e2dcf731cdf
Author: Mal Graty
Date: 2018-07-29T16:35:15Z
Fix usage of traceback.format_exc in multilang
---
[GitHub] storm pull request #2779: STORM-3163: Make ShellBolt logger thread local to ...
GitHub user mal opened a pull request: https://github.com/apache/storm/pull/2779 STORM-3163: Make ShellBolt logger thread local to support MDC Prior to this the logger set up and use would occur in separate threads, so any MDC values would not persist from set up, by using the same thread, MDC values are now reliably available during log calls. https://issues.apache.org/jira/browse/STORM-3163 --- _P.S._ While working on this, I also ended up pulling the log level routing out of `DefaultShellLogHandler` as this makes it trivial to reuse in customised `ShellLogHandler`s. --- Currently WIP, early feedback welcome, tests to follow ð You can merge this pull request into a Git repository by running: $ git pull https://github.com/mal/storm STORM-3163 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2779.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 #2779 commit 333effcd77dc01e9342ffdebbb35748c9ff79dc8 Author: Mal Graty Date: 2018-07-29T15:06:51Z Make ShellBolt logger thread local to support MDC Prior to this the logger set up and use would occur in separate threads, so any MDC values would not persist from set up, by using the same thread MDC values are now reliably available during log calls. ---
[GitHub] storm pull request #2772: STORM-3158 improve logging on login failure when k...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2772 ---
[GitHub] storm issue #2775: MINOR - Make raw type assignment type safe
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2775 Thanks @hmcl for providing the patch. Merged into master. ---
[GitHub] storm pull request #2775: MINOR - Make raw type assignment type safe
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2775 ---
[GitHub] storm pull request #2773: Blobstore sync bug fix
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2773#discussion_r205973135
--- Diff:
storm-server/src/main/java/org/apache/storm/blobstore/LocalFsBlobStore.java ---
@@ -197,7 +198,7 @@ public void run() {
throw new RuntimeException(e);
}
}
-}, 0,
ObjectReader.getInt(conf.get(DaemonConfig.NIMBUS_CODE_SYNC_FREQ_SECS)));
+}, 0,
ObjectReader.getInt(conf.get(DaemonConfig.NIMBUS_CODE_SYNC_FREQ_SECS))*1000);
--- End diff --
Nice catch!
---
[GitHub] storm pull request #2773: Blobstore sync bug fix
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2773#discussion_r205973204
--- Diff:
storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java ---
@@ -191,6 +192,8 @@ public static boolean downloadUpdatedBlob(Map conf, BlobStore bl
out.close();
}
isSuccess = true;
+} catch(FileNotFoundException fnf) {
--- End diff --
I'd rather leave it as it is, unless we feel confident that it is OK to
swallow here.
---
[GitHub] storm pull request #2778: (1.x) STORM-3121: Fix flaky metrics tests in storm...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2778 ---
[GitHub] storm issue #2778: (1.x) STORM-3121: Fix flaky metrics tests in storm-core
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2778 @srdo Thanks for the review. Since this is a patch for backport I'll skip waiting 24hrs for this. ---
[GitHub] storm issue #2714: STORM-3101: Fix unexpected metrics registration in StormM...
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2714 @zd-project I think the Enum idea sounds nice, but I'm not sure whether it will be so easy to implement. Some metrics depend on state in the class they're declared, e.g. the two gauges in Container. I'm not sure how you could move those to an enum without making the fields in Container public. I'd still like to investigate whether we can make the metrics registries non-static. Would you mind if I played around with it a bit? ---
[GitHub] storm issue #2778: (1.x) STORM-3121: Fix flaky metrics tests in storm-core
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2778 +1, thanks for backporting. ---
[GitHub] storm pull request #2778: (1.x) STORM-3121: Fix flaky metrics tests in storm...
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/2778#discussion_r205965687 --- Diff: storm-core/test/resources/log4j2-test.xml --- @@ -25,7 +25,8 @@ - + --- End diff -- @srdo I think we need to use origin package name. I missed here. Fixed. ---
[GitHub] storm pull request #2778: (1.x) STORM-3121: Fix flaky metrics tests in storm...
Github user srdo commented on a diff in the pull request: https://github.com/apache/storm/pull/2778#discussion_r205965554 --- Diff: storm-core/test/resources/log4j2-test.xml --- @@ -25,7 +25,8 @@ - + --- End diff -- Could you verify that this change makes sense on 1.x? I think it is here because master has the shaded-deps module, but in 1.x the shading happens as part of building storm-core, so I'm not sure the test classpath is affected by the relocation. ---
[GitHub] storm issue #2778: (1.x) STORM-3121: Fix flaky metrics tests in storm-core
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2778 https://travis-ci.org/HeartSaVioR/storm/jobs/409469987 (passed) https://travis-ci.org/HeartSaVioR/storm/jobs/409469988 (passed) https://travis-ci.org/apache/storm/jobs/409470450 (integration.org.apache.storm.integration-test failed) https://travis-ci.org/apache/storm/jobs/409470451 (integration.org.apache.storm.integration-test failed) ---
[GitHub] storm pull request #2778: (1.x) STORM-3121: Fix flaky metrics tests in storm...
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/2778#discussion_r205964723 --- Diff: storm-core/test/clj/org/apache/storm/metrics_test.clj --- @@ -15,16 +15,14 @@ ;; limitations under the License. (ns org.apache.storm.metrics-test (:use [clojure test]) - (:import [org.apache.storm.topology TopologyBuilder]) - (:import [org.apache.storm.generated InvalidTopologyException SubmitOptions TopologyInitialStatus]) - (:import [org.apache.storm.testing TestWordCounter TestWordSpout TestGlobalCount -TestAggregatesCounter TestConfBolt AckFailMapTracker PythonShellMetricsBolt PythonShellMetricsSpout]) - (:import [org.apache.storm.task ShellBolt]) - (:import [org.apache.storm.spout ShellSpout]) - (:import [org.apache.storm.metric.api CountMetric IMetricsConsumer$DataPoint IMetricsConsumer$TaskInfo]) - (:import [org.apache.storm.metric.api.rpc CountShellMetric]) - (:import [org.apache.storm.utils Utils]) - + (:import [org.apache.storm.testing AckFailMapTracker PythonShellMetricsBolt PythonShellMetricsSpout]) --- End diff -- Most of changes from import are removing unused import. ---
[GitHub] storm pull request #2778: (1.x) STORM-3121: Fix flaky metrics tests in storm...
GitHub user HeartSaVioR opened a pull request: https://github.com/apache/storm/pull/2778 (1.x) STORM-3121: Fix flaky metrics tests in storm-core This ports back #2735 to 1.x-branch since I can see flaky metrics tests from 1.x-branch. https://travis-ci.org/apache/storm/jobs/408847970#L1079-L1082 I also fixed merge conflict so would like to review rather than pushing the change without notice. @srdo Could you take a look? Thanks in advance! You can merge this pull request into a Git repository by running: $ git pull https://github.com/HeartSaVioR/storm STORM-3121-1.x Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2778.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 #2778 commit 50ea2149776adecdb270e81c60ab5192fbd50e79 Author: Stig Rohde Døssing Date: 2018-06-24T10:55:11Z STORM-3121: Fix flaky metrics tests in storm-core ---
[GitHub] storm pull request #2777: (1.x) STORM-3161 Local mode should force setting m...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2777 ---
[GitHub] storm pull request #2776: STORM-3161 Local mode should force setting min rep...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/2776 ---
[GitHub] storm issue #2711: STORM-3100 : Introducing CustomIndexArray to speedup Hash...
Github user HeartSaVioR commented on the issue: https://github.com/apache/storm/pull/2711 Looks like the build still fails and relevant to this patch. ---
