Re: [PR] STORM-4017 Fix for isAnyWindowsProcessAlive with multiple pids (storm)

2024-01-15 Thread via GitHub


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)

2024-01-15 Thread via GitHub


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)

2024-01-08 Thread via GitHub


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)

2024-01-08 Thread via GitHub


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)

2024-01-08 Thread via GitHub


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)

2024-01-08 Thread via GitHub


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)

2024-01-08 Thread via GitHub


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)

2024-01-08 Thread via GitHub


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)

2024-01-08 Thread via GitHub


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