Re: [PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
jnioche commented on PR #3615: URL: https://github.com/apache/storm/pull/3615#issuecomment-1892317878 Thanks @Scomocouk -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
jnioche merged PR #3615: URL: https://github.com/apache/storm/pull/3615 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
Scomocouk commented on PR #3615: URL: https://github.com/apache/storm/pull/3615#issuecomment-1881677336 As suggested from my previous pull request (https://github.com/apache/storm/pull/3614), I have resubmitted this for merging into master instead of 2.4.x-branch -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
Scomocouk opened a new pull request, #3615: URL: https://github.com/apache/storm/pull/3615 ## What is the purpose of the change Fixes STORM-4017 I raised recently so that isAnyWindowsProcessAlive works correctly in Windows. What prompted this was that we are evaluating Storm 2.5.0 under Windows and noticed that when workers are restarted the child processes are left running. We noticed logs reporting that "None of the processes , , are alive" when they were in fact still running. Further investigation showed why this was happening, hence raised STORM-4017 How was the change tested ## How was the change tested Existing unit test 'testIsAnyProcessAlive' in ServerUtilsTest.java did not work under Windows but now does with this fix. We have confirmed that we only now see the "None of the processes ... are alive" when the processes are genuinely not running See attached unit test run before and after fix ![STORM-4017-not-fixed-2](https://github.com/apache/storm/assets/152920539/4b905714-cf31-45c5-9316-29bae229fdf4) ![STORM-4017-fixed-2](https://github.com/apache/storm/assets/152920539/de9a3315-5a6f-46d4-816e-419e25d1e7d4) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
Scomocouk closed pull request #3614: STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids URL: https://github.com/apache/storm/pull/3614 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
rzo1 commented on PR #3614: URL: https://github.com/apache/storm/pull/3614#issuecomment-1881242017 Therefore, I suggest to target `main` with the change and post a related mail on dev@ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
rzo1 commented on PR #3614: URL: https://github.com/apache/storm/pull/3614#issuecomment-1881216003 Thanks for the PR. I definitley see it in the next release of Storm. However, given the limited amount of volunteers working on Storm (it was discussed to move it to the attic a couple of months ago due to limited resources / volunteers willing to contribute), there might be no intention / capacity to acutally release a 2.5.x version of Storm. You are free to bring up this topic on the dev@ mailing list. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
Scomocouk commented on PR #3614: URL: https://github.com/apache/storm/pull/3614#issuecomment-1881120469 This request is to merge to 2.4.x-branch. However, we are actaully working with 2.5.0 but notice there is no corresponding 2.5.x-branch to merge in to. Can this be added? (We are working on 2.5.0 as this is the latest version to work with Java8, which we are currently restricted to) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)
Scomocouk opened a new pull request, #3614: URL: https://github.com/apache/storm/pull/3614 ## What is the purpose of the change Fixes STORM-4017 I raised recently so that isAnyWindowsProcessAlive works correctly in Windows. What prompted this was that we are evaluating Storm 2.5.0 under Windows and noticed that when workers are restarted the child processes are left running. We noticed logs reporting that "None of the processes , , are alive" when they were in fact still running. Further investigation showed why this was happening, hence raised STORM-4017 ## How was the change tested Existing unit test 'testIsAnyProcessAlive' in ServerUtilsTest.java did not work under Windows but now does with this fix. We have confirmed that we only now see the "None of the processes ... are alive" when the processes are genuinely not running See attached unit test run before and after fix ![STORM-4017-not-fixed](https://github.com/apache/storm/assets/152920539/079c08e5-7062-4da3-b78b-001ba211301d) ![STORM-4017-fixed](https://github.com/apache/storm/assets/152920539/39f69296-decc-4119-8b73-d17c732539bf) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org