[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2660 @raghavgautam I completely missed something with my test. Originally started a tcp server and a tcp client in separate processes and sent KILLTERM to the client side. The server port stayed in TIME_WAIT then started a new client to check if it can connect to the server side. When #STORM-3039 is landed on our lines and the original exception was still present, rechecked the test and realized that even without the change the new client could connect to the server at least on ubuntu16. Sorry for the mistake! ---
[GitHub] storm pull request #2633: STORM-3028 HdfsSpout: handle empty file in case of...
Github user ghajos commented on a diff in the pull request:
https://github.com/apache/storm/pull/2633#discussion_r188606821
--- Diff:
external/storm-hdfs/src/main/java/org/apache/storm/hdfs/spout/HdfsSpout.java ---
@@ -221,14 +221,18 @@ public void nextTuple() {
while (true) {
try {
// 3) Select a new file if one is not open already
+boolean newReader = false;
if (reader == null) {
reader = pickNextFile();
if (reader == null) {
LOG.debug("Currently no new files to process under
: " + sourceDirPath);
return;
} else {
fileReadCompletely = false;
+newReader = true;
}
+} else {
+newReader = false;
--- End diff --
Thank you for pointing this out!
---
[GitHub] storm issue #2633: STORM-3028 HdfsSpout: handle empty file in case of ackers
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2633 @revans2 @srdo I'm sorry! There is no reason to set both commit frequency count and commit frequency sec. I hope that this is already addressed in the last commit. ---
[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2660 @raghavgautam Thanks for the quick reaction! Unfortunately the newly introduced test passes without #2643, so it does not test the original issue. Since in this modification both the server and the client is closed gracefully the port won't stuck in TIME_WAIT state, no need to use SO_REUSEADDR to restart new instances. To test #2643 the server port should stuck in TIME_WAIT state and then a new server instance should be started successfully. I still think that it is out of scope to test an operating system level TCP setting in a Storm test. cc: @HeartSaVioR ---
[GitHub] storm issue #2643: STORM-3039 handle slot ports in TIME_WAIT state
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2643 @HeartSaVioR @raghavgautam @arunmahadevan Thank you for the review! ---
[GitHub] storm issue #2643: STORM-3039 handle slot ports in TIME_WAIT state
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2643 @raghavgautam I think it is out of scope to test a tcp testing in Storm. Do you have an idea how to do that? ---
[GitHub] storm issue #2633: STORM-3028 HdfsSpout: handle empty file in case of ackers
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2633 @srdo Can you please take a look at? ---
[GitHub] storm issue #2643: STORM-3039 handle slot ports in TIME_WAIT state
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2643 @HeartSaVioR Can you please take a look at? ---
[GitHub] storm pull request #2643: STORM-3039 handle slot ports in TIME_WAIT state
GitHub user ghajos opened a pull request: https://github.com/apache/storm/pull/2643 STORM-3039 handle slot ports in TIME_WAIT state When worker is killed slot port remains in TIME_WAIT state. Since worker process is killed it is secure to start a new worker process on the same port. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ghajos/storm STORM-3039 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2643.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 #2643 commit a8978ab3543e00cf4e94ae361d4d1935b44b69f2 Author: Gergely Hajos <rogozsin@...> Date: 2018-04-23T12:31:14Z STORM-3039 handle slot ports in TIME_WAIT state ---
[GitHub] storm pull request #2633: STORM-3028 HdfsSpout: handle empty file in case of...
GitHub user ghajos opened a pull request: https://github.com/apache/storm/pull/2633 STORM-3028 HdfsSpout: handle empty file in case of ackers There is a problem with all the storm-hdfs junit tests. The test results may not be valid. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ghajos/storm STORM-3028 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2633.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 #2633 commit 285e287c063c6a12c79553c7a96c9878b0ba3884 Author: Gergely Hajos <rogozsin@...> Date: 2018-04-12T11:58:13Z STORM-3028 HdfsSpout: handle empty file in case of ackers ---
[GitHub] storm issue #2583: STORM-2649 More detailed check of config serialization
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2583 @srdo @HeartSaVioR @revans2 Thank you for the review and the patience! ---
[GitHub] storm pull request #2583: STORM-2649 More detailed check of config serializa...
Github user ghajos commented on a diff in the pull request:
https://github.com/apache/storm/pull/2583#discussion_r178527233
--- Diff: storm-client/test/jvm/org/apache/storm/utils/UtilsTest.java ---
@@ -173,4 +176,49 @@ public void
isZkAuthenticationConfiguredStormServerWithPropertyTest() {
}
}
}
+
+@Test
--- End diff --
Found a positive test case in TestConfigValidate and added negative test
cases. Should I collect some more test cases?
---
[GitHub] storm issue #2583: STORM-2649 More detailed check of config serialization
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2583 Sorry for the late reply! ---
[GitHub] storm issue #2585: STORM-2762 use guava collection where ever it is possible
Github user ghajos commented on the issue: https://github.com/apache/storm/pull/2585 @HeartSaVioR @revans2 Thank you for the review! ---
[GitHub] storm pull request #2585: STORM-2762 use guava collection where ever it is p...
GitHub user ghajos opened a pull request: https://github.com/apache/storm/pull/2585 STORM-2762 use guava collection where ever it is possible Searched the code with git grep to find unnecessarily defined collection functions. I could only find these two. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ghajos/storm STORM-2762 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2585.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 #2585 commit 2189abfa1e15ce859969d93ad4656c5b84a345fd Author: Gergely Hajos <rogozsin@...> Date: 2018-03-04T16:30:41Z STORM-2762 use guava collection where ever it is possible ---
[GitHub] storm pull request #2583: STORM-2649 More detailed check of config serializa...
GitHub user ghajos opened a pull request: https://github.com/apache/storm/pull/2583 STORM-2649 More detailed check of config serialization You can merge this pull request into a Git repository by running: $ git pull https://github.com/ghajos/storm STORM-2649 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2583.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 #2583 commit 840c6c969f0fda0292c4fe3c07a3126ab1072d18 Author: Gergely Hajos <rogozsin@...> Date: 2018-02-25T18:32:45Z STORM-2649 More detailed check of config serialization ---
[GitHub] storm pull request #2574: STORM-2911 add serialVersionUID to storm-kafka Spo...
GitHub user ghajos opened a pull request: https://github.com/apache/storm/pull/2574 STORM-2911 add serialVersionUID to storm-kafka SpoutConfig You can merge this pull request into a Git repository by running: $ git pull https://github.com/ghajos/storm STORM-2911 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/2574.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 #2574 commit 408e53174d3fc0f82a542f38ab0d49ca7c8992e4 Author: Gergely Hajos <rogozsin@...> Date: 2018-02-25T14:32:21Z STORM-2911 add serialVersionUID to storm-kafka SpoutConfig ---
