You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by raghavgautam <gi...@git.apache.org> on 2018/05/03 20:00:27 UTC
[GitHub] storm pull request #2660: STORM-3056: Add a test for quickly rebinding to a ...
GitHub user raghavgautam opened a pull request:
https://github.com/apache/storm/pull/2660
STORM-3056: Add a test for quickly rebinding to a port
Adding test as per discussion on https://github.com/apache/storm/pull/2643#issuecomment-385003933
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/raghavgautam/storm master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2660.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 #2660
----
commit 76d89b5ad2471080a501031d3ecf965b39ae283f
Author: Raghav Kumar Gautam <ra...@...>
Date: 2018-05-03T19:59:16Z
STORM-3056: Add a test for quickly rebinding to a port
----
---
[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Posted by raghavgautam <gi...@git.apache.org>.
Github user raghavgautam commented on the issue:
https://github.com/apache/storm/pull/2660
@ghajos Ping.
---
[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Posted by ghajos <gi...@git.apache.org>.
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 issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2660
@ghajos Thanks for the information. I didn't recognize the fact that the new test is passing without #2643. I agree this is out of scope, just think that if the new test can verify the new behavior "consistently", it is no reason to deny the new test.
@raghavgautam Could you fix this, or just would like to simply close both the PR and issue?
---
[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Posted by raghavgautam <gi...@git.apache.org>.
Github user raghavgautam commented on the issue:
https://github.com/apache/storm/pull/2660
@ghajos Can you tell me how you tested your patch ?
---
[GitHub] storm pull request #2660: STORM-3056: Add a test for quickly rebinding to a ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2660
---
[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/2660
@raghavgautam Can you please address the merge conflicts?
---
[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Posted by raghavgautam <gi...@git.apache.org>.
Github user raghavgautam commented on the issue:
https://github.com/apache/storm/pull/2660
@HeartSaVioR @ghajos Please review.
---
[GitHub] storm issue #2660: STORM-3056: Add a test for quickly rebinding to a port
Posted by ghajos <gi...@git.apache.org>.
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
---