You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Taher Alkhateeb <sl...@gmail.com> on 2017/06/24 18:04:58 UTC
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
This commit is wrong and bad on multiple levels. Please revert
On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
> Author: jleroux
> Date: Sat Jun 24 07:56:45 2017
> New Revision: 1799736
>
> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
> Log:
> No functional change
>
> Improves terminateOfbiz byt using TERM before KILL
> https://fr.wikipedia.org/wiki/Kill_(Unix)
> https://unix.stackexchange.com/questions/8916/when-
> should-i-not-kill-9-a-process
>
> Modified:
> ofbiz/ofbiz-framework/trunk/build.gradle
>
> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
> ============================================================
> ==================
> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
> standardOutput = processOutput
> }
> processOutput.toString().split(System.lineSeparator()).each
> { line ->
> + // Try to terminate cleanly
> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/)
> {
> - exec { commandLine 'kill', '-9',
> line.tokenize().first() }
> + exec { commandLine 'kill', '-TERM',
> line.tokenize().first() }
> + }
> + // Only kill if needed
> + if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/)
> {
> + exec { commandLine 'kill', '-KILL',
> line.tokenize().first() }
> }
> }
> }
>
>
>
Re: A problem with terminateOfbiz Gradle task (was [Re: svn commit:
r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle)
Posted by Jacques Le Roux <ja...@les7arts.com>.
OK, I think nobody else cares because you indeed rarely run several OFBiz instances on the same machine, apart in demos and maybe in development.
So I'll create a script specific to demos where I need to sometimes kill only one of the 3 instances and not all.
Jacques
Le 27/06/2017 à 15:12, Taher Alkhateeb a écrit :
> I think terminateOfbiz should NOT be portoffset-aware nor should it
> kill only one instance. terminateOfbiz is a convenience task to kill
> all instances which is mostly used during development. That's the only
> purpose it has. I believe it should NOT be used in any automation
> suite or buildbot or anything like that.
>
> I have already spent a lot of time correcting your errors in that task
> [1]. Please let's try to avoid repeating this time consuming process.
>
> [1] https://s.apache.org/fvBB
>
> On Tue, Jun 27, 2017 at 3:39 PM, Jacques Le Roux
> <ja...@les7arts.com> wrote:
>> We still have a problem with terminateOfbiz: it's not portoffset aware.
>>
>> So when you run it, it closes all OFBiz instances running on a machine (eg
>> on VM demo)
>>
>> I guess we only need to improve terminateOfbiz by passing a portoffset
>> parameter
>>
>> I'll create a Jira and provide a patch for review
>>
>> Jacques
>>
>>
>> Le 26/06/2017 à 09:37, Jacques Le Roux a écrit :
>>> OK, the shutdown command is somehow our SIGTERM, makes sense
>>>
>>> Jacques
>>>
>>>
>>> Le 26/06/2017 à 08:45, Taher Alkhateeb a écrit :
>>>> I do not think we should send SIGTERM in the first place. The proper
>>>> termination of OFBiz is with the --shutdown command which cleanly stops
>>>> everything and calls the shutdown hook. The gradle task "terminateOfbiz"
>>>> should only be used if the shutdown command fails (last resort) as cleary
>>>> mentioned in the docs.
>>>>
>>>> On Jun 26, 2017 9:19 AM, "Jacques Le Roux" <ja...@les7arts.com>
>>>> wrote:
>>>>
>>>> Thanks Taher,
>>>>
>>>> I indeed missed that kill is not an atomic operation. I reverted at
>>>> r1799852.
>>>>
>>>> Before reading your last message I wanted to propose to set a delay
>>>> between
>>>> the 2 operations.
>>>>
>>>> So SIGTERM fails. I'd really like to send SIGTERM before killing the
>>>> process and only kill it if it's necessary (ie if SIGTERM did not
>>>> terminate
>>>> the process)
>>>>
>>>> So we need to catch the SIGTERM exec and send SIGKILL if it fails, right?
>>>>
>>>> Jacques
>>>>
>>>>
>>>>
>>>> Le 25/06/2017 à 16:05, Taher Alkhateeb a écrit :
>>>>
>>>>> Oh and to prove the second scenario, just add a sleep for a few seconds
>>>>> after the SIGTERM kill command and observe how the system will crash:
>>>>>
>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>>>>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>>>>> Thread.sleep(5000)
>>>>> }
>>>>>
>>>>> Again, this shows you how poor the quality of this code is. It
>>>>> introduces a
>>>>> subtle bug, it repeats an if condition unnecessarily, and the comments
>>>>> are
>>>>> wrong "Only kill if needed"
>>>>>
>>>>> On Sun, Jun 25, 2017 at 4:22 PM, Taher Alkhateeb <
>>>>> slidingfilaments@gmail.com
>>>>>
>>>>>> wrote:
>>>>>> I'm completing my answer for the record to show exactly where your code
>>>>>> is
>>>>>> faulty and wrong just like in the other previous cases.
>>>>>>
>>>>>> 1- you have a copy-paste pattern in your code
>>>>>> 2- you are calling the kill command twice immediately one after another
>>>>>> without checking whether SIGTERM killed the process properly. The whole
>>>>>> purpose of doing SIGTERM is to allow cleaning resources before
>>>>>> enforcing
>>>>>> SIGKILL. You're not giving the JVM an opportunity to call the shutdown
>>>>>> hook
>>>>>> because you're looking up the same output from the same "ps ax" command
>>>>>> for
>>>>>> both kill commands.
>>>>>>
>>>>>> So your code is working almost exactly as if typing:
>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>>>>>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>>>>>> exec { commandLine 'kill', '-KILL', line.tokenize().first() }
>>>>>> }
>>>>>>
>>>>>> Now what's happening is that SIGKILL is executing before SIGTERM
>>>>>> completes, so the build is successful but OFBiz is killed with SIGKILL
>>>>>> not
>>>>>> SIGTERM. If SIGTERM finishes very quickly (does not happen with me)
>>>>>> then
>>>>>> the build system would crash because the exit value of SIGKILL would be
>>>>>> 1,
>>>>>> not 0 (process not found). So either way it is a problem (either
>>>>>> killing
>>>>>> with SIGKILL or crashing the build system)
>>>>>>
>>>>>> Convinced? I hope you can revert now.
>>>>>>
>>>>>> On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
>>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>>
>>>>>> But what's wrong? It's not clear to me, do you see it Michael?
>>>>>>> Not clearly answering this question just adds more work on my side, I
>>>>>>> still don't see why I should revert.
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>>>>>>>
>>>>>>> I's like to propose the following process to get this worked out
>>>>>>> without
>>>>>>>> endless ping-pong here on the mailing lists:
>>>>>>>>
>>>>>>>> 1. revert the change as requested
>>>>>>>>
>>>>>>>> 2. provide a Jira with patch and review/discuss it there (what's the
>>>>>>>> problem, how should it be solved etc.)
>>>>>>>>
>>>>>>>> 3. come to a conclusion and a thumbsup by other committers
>>>>>>>>
>>>>>>>> 4. commit the solution.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Michael
>>>>>>>>
>>>>>>>>
>>>>>>>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>>>>>>>
>>>>>>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>>>>>>>> done
>>>>>>>>> spending time here and I leave it up to the community to see if they
>>>>>>>>> want
>>>>>>>>> incorrect code in the code base.
>>>>>>>>>
>>>>>>>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>>>>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>>>>>
>>>>>>>>> Mmm but anyway, I remember now how I thought that.
>>>>>>>>>
>>>>>>>>>> In the 2nd "if", if the process was terminated, kill will just say
>>>>>>>>>> that it
>>>>>>>>>> can't find the process and exec will return. Else the process will
>>>>>>>>>> be
>>>>>>>>>> killed.
>>>>>>>>>>
>>>>>>>>>> What's wrong then?
>>>>>>>>>>
>>>>>>>>>> Jacques
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>>>>>>>
>>>>>>>>>> Ah wait, I see line still contains "start" so will be executed
>>>>>>>>>> twice
>>>>>>>>>>
>>>>>>>>>>> anyway. OK I'll improve that.
>>>>>>>>>>>
>>>>>>>>>>> Jacques
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>>>>>>>
>>>>>>>>>>> I don't revert without explanations on why I should revert. Sorry,
>>>>>>>>>>>
>>>>>>>>>>>> but I
>>>>>>>>>>>> don't find your explanations clear.
>>>>>>>>>>>>
>>>>>>>>>>>> My explanation, tell my what's wrong:
>>>>>>>>>>>>
>>>>>>>>>>>> The 2 "if" blocks are executed sequentially for each line
>>>>>>>>>>>> containing
>>>>>>>>>>>> "a
>>>>>>>>>>>> start"and ignore other lines. I did not change the previous
>>>>>>>>>>>> logic,
>>>>>>>>>>>> just
>>>>>>>>>>>> added a new if.
>>>>>>>>>>>>
>>>>>>>>>>>> Gradle exec spawns a new process and waits till it ends (this
>>>>>>>>>>>> point
>>>>>>>>>>>> is
>>>>>>>>>>>> important).
>>>>>>>>>>>>
>>>>>>>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>>>>>>>
>>>>>>>>>>>> If it worked the 2nd "if" does not execute. This is better than
>>>>>>>>>>>> before
>>>>>>>>>>>> because it allows the "'start" process "a chance to clean up
>>>>>>>>>>>> after
>>>>>>>>>>>> itself"
>>>>>>>>>>>> (cf unix.stackexchange.com below)
>>>>>>>>>>>>
>>>>>>>>>>>> If the "start" process is not terminated then it's killed by the
>>>>>>>>>>>> 2nd
>>>>>>>>>>>> "if", like it was done before.
>>>>>>>>>>>>
>>>>>>>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>>>>>>>> background.
>>>>>>>>>>>>
>>>>>>>>>>>> Now tell me what's wrong?
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks
>>>>>>>>>>>>
>>>>>>>>>>>> Jacques
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>>>>>>>
>>>>>>>>>>>> As usual, you refuse to revert because you don't understand the
>>>>>>>>>>>> code
>>>>>>>>>>>>
>>>>>>>>>>>>> and I
>>>>>>>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>>>>>>>> reconsider
>>>>>>>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The quick version:
>>>>>>>>>>>>> - copy and paste antipattern
>>>>>>>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>>>>>>>> executed or
>>>>>>>>>>>>> both blocks not executing
>>>>>>>>>>>>>
>>>>>>>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>>>>>>>> killed
>>>>>>>>>>>>> is
>>>>>>>>>>>>> amazing! It means you do not understand what this code is doing
>>>>>>>>>>>>> and
>>>>>>>>>>>>> what is
>>>>>>>>>>>>> causing the failure.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>>>>>>>> jacques.le.roux@les7arts.com>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>>>>>>>> background
>>>>>>>>>>>>> instances and it cleaned worked.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>>>>>>>> works,
>>>>>>>>>>>>> and
>>>>>>>>>>>>> you get this message
>>>>>>>>>>>>>
>>>>>>>>>>>>> :ofbiz FAILED
>>>>>>>>>>>>> FAILURE: Build failed with an exception.
>>>>>>>>>>>>> * What went wrong:
>>>>>>>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>>>>>>>
>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> non-zero exit value 137
>>>>>>>>>>>>>>
>>>>>>>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>>>>>>>
>>>>>>>>>>>>> Jacques
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>>>>>>>
>>>>>>>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>>>>>>>> Author: jleroux
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> New Revision: 1799736
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>>>>>>>> Log:
>>>>>>>>>>>>>>> No functional change
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Modified:
>>>>>>>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>>>>>>>> ============================================================
>>>>>>>>>>>>>>> ==================
>>>>>>>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24
>>>>>>>>>>>>>>> 07:56:45
>>>>>>>>>>>>>>> 2017
>>>>>>>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>>>>>>>> standardOutput = processOutput
>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>>>>>>>> { line ->
>>>>>>>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>>>>>>>> if (line ==~
>>>>>>>>>>>>>>> /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>>>>>> {
>>>>>>>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>> + // Only kill if needed
>>>>>>>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>>>>>> {
>>>>>>>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>
Re: A problem with terminateOfbiz Gradle task (was [Re: svn commit:
r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle)
Posted by Taher Alkhateeb <sl...@gmail.com>.
I think terminateOfbiz should NOT be portoffset-aware nor should it
kill only one instance. terminateOfbiz is a convenience task to kill
all instances which is mostly used during development. That's the only
purpose it has. I believe it should NOT be used in any automation
suite or buildbot or anything like that.
I have already spent a lot of time correcting your errors in that task
[1]. Please let's try to avoid repeating this time consuming process.
[1] https://s.apache.org/fvBB
On Tue, Jun 27, 2017 at 3:39 PM, Jacques Le Roux
<ja...@les7arts.com> wrote:
> We still have a problem with terminateOfbiz: it's not portoffset aware.
>
> So when you run it, it closes all OFBiz instances running on a machine (eg
> on VM demo)
>
> I guess we only need to improve terminateOfbiz by passing a portoffset
> parameter
>
> I'll create a Jira and provide a patch for review
>
> Jacques
>
>
> Le 26/06/2017 à 09:37, Jacques Le Roux a écrit :
>>
>> OK, the shutdown command is somehow our SIGTERM, makes sense
>>
>> Jacques
>>
>>
>> Le 26/06/2017 à 08:45, Taher Alkhateeb a écrit :
>>>
>>> I do not think we should send SIGTERM in the first place. The proper
>>> termination of OFBiz is with the --shutdown command which cleanly stops
>>> everything and calls the shutdown hook. The gradle task "terminateOfbiz"
>>> should only be used if the shutdown command fails (last resort) as cleary
>>> mentioned in the docs.
>>>
>>> On Jun 26, 2017 9:19 AM, "Jacques Le Roux" <ja...@les7arts.com>
>>> wrote:
>>>
>>> Thanks Taher,
>>>
>>> I indeed missed that kill is not an atomic operation. I reverted at
>>> r1799852.
>>>
>>> Before reading your last message I wanted to propose to set a delay
>>> between
>>> the 2 operations.
>>>
>>> So SIGTERM fails. I'd really like to send SIGTERM before killing the
>>> process and only kill it if it's necessary (ie if SIGTERM did not
>>> terminate
>>> the process)
>>>
>>> So we need to catch the SIGTERM exec and send SIGKILL if it fails, right?
>>>
>>> Jacques
>>>
>>>
>>>
>>> Le 25/06/2017 à 16:05, Taher Alkhateeb a écrit :
>>>
>>>> Oh and to prove the second scenario, just add a sleep for a few seconds
>>>> after the SIGTERM kill command and observe how the system will crash:
>>>>
>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>>>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>>>> Thread.sleep(5000)
>>>> }
>>>>
>>>> Again, this shows you how poor the quality of this code is. It
>>>> introduces a
>>>> subtle bug, it repeats an if condition unnecessarily, and the comments
>>>> are
>>>> wrong "Only kill if needed"
>>>>
>>>> On Sun, Jun 25, 2017 at 4:22 PM, Taher Alkhateeb <
>>>> slidingfilaments@gmail.com
>>>>
>>>>> wrote:
>>>>> I'm completing my answer for the record to show exactly where your code
>>>>> is
>>>>> faulty and wrong just like in the other previous cases.
>>>>>
>>>>> 1- you have a copy-paste pattern in your code
>>>>> 2- you are calling the kill command twice immediately one after another
>>>>> without checking whether SIGTERM killed the process properly. The whole
>>>>> purpose of doing SIGTERM is to allow cleaning resources before
>>>>> enforcing
>>>>> SIGKILL. You're not giving the JVM an opportunity to call the shutdown
>>>>> hook
>>>>> because you're looking up the same output from the same "ps ax" command
>>>>> for
>>>>> both kill commands.
>>>>>
>>>>> So your code is working almost exactly as if typing:
>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>>>>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>>>>> exec { commandLine 'kill', '-KILL', line.tokenize().first() }
>>>>> }
>>>>>
>>>>> Now what's happening is that SIGKILL is executing before SIGTERM
>>>>> completes, so the build is successful but OFBiz is killed with SIGKILL
>>>>> not
>>>>> SIGTERM. If SIGTERM finishes very quickly (does not happen with me)
>>>>> then
>>>>> the build system would crash because the exit value of SIGKILL would be
>>>>> 1,
>>>>> not 0 (process not found). So either way it is a problem (either
>>>>> killing
>>>>> with SIGKILL or crashing the build system)
>>>>>
>>>>> Convinced? I hope you can revert now.
>>>>>
>>>>> On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>
>>>>> But what's wrong? It's not clear to me, do you see it Michael?
>>>>>>
>>>>>> Not clearly answering this question just adds more work on my side, I
>>>>>> still don't see why I should revert.
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>>
>>>>>> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>>>>>>
>>>>>> I's like to propose the following process to get this worked out
>>>>>> without
>>>>>>>
>>>>>>> endless ping-pong here on the mailing lists:
>>>>>>>
>>>>>>> 1. revert the change as requested
>>>>>>>
>>>>>>> 2. provide a Jira with patch and review/discuss it there (what's the
>>>>>>> problem, how should it be solved etc.)
>>>>>>>
>>>>>>> 3. come to a conclusion and a thumbsup by other committers
>>>>>>>
>>>>>>> 4. commit the solution.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Michael
>>>>>>>
>>>>>>>
>>>>>>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>>>>>>
>>>>>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>>>>>>>
>>>>>>>> done
>>>>>>>> spending time here and I leave it up to the community to see if they
>>>>>>>> want
>>>>>>>> incorrect code in the code base.
>>>>>>>>
>>>>>>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>>>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>>>>
>>>>>>>> Mmm but anyway, I remember now how I thought that.
>>>>>>>>
>>>>>>>>> In the 2nd "if", if the process was terminated, kill will just say
>>>>>>>>> that it
>>>>>>>>> can't find the process and exec will return. Else the process will
>>>>>>>>> be
>>>>>>>>> killed.
>>>>>>>>>
>>>>>>>>> What's wrong then?
>>>>>>>>>
>>>>>>>>> Jacques
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>>>>>>
>>>>>>>>> Ah wait, I see line still contains "start" so will be executed
>>>>>>>>> twice
>>>>>>>>>
>>>>>>>>>> anyway. OK I'll improve that.
>>>>>>>>>>
>>>>>>>>>> Jacques
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>>>>>>
>>>>>>>>>> I don't revert without explanations on why I should revert. Sorry,
>>>>>>>>>>
>>>>>>>>>>> but I
>>>>>>>>>>> don't find your explanations clear.
>>>>>>>>>>>
>>>>>>>>>>> My explanation, tell my what's wrong:
>>>>>>>>>>>
>>>>>>>>>>> The 2 "if" blocks are executed sequentially for each line
>>>>>>>>>>> containing
>>>>>>>>>>> "a
>>>>>>>>>>> start"and ignore other lines. I did not change the previous
>>>>>>>>>>> logic,
>>>>>>>>>>> just
>>>>>>>>>>> added a new if.
>>>>>>>>>>>
>>>>>>>>>>> Gradle exec spawns a new process and waits till it ends (this
>>>>>>>>>>> point
>>>>>>>>>>> is
>>>>>>>>>>> important).
>>>>>>>>>>>
>>>>>>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>>>>>>
>>>>>>>>>>> If it worked the 2nd "if" does not execute. This is better than
>>>>>>>>>>> before
>>>>>>>>>>> because it allows the "'start" process "a chance to clean up
>>>>>>>>>>> after
>>>>>>>>>>> itself"
>>>>>>>>>>> (cf unix.stackexchange.com below)
>>>>>>>>>>>
>>>>>>>>>>> If the "start" process is not terminated then it's killed by the
>>>>>>>>>>> 2nd
>>>>>>>>>>> "if", like it was done before.
>>>>>>>>>>>
>>>>>>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>>>>>>> background.
>>>>>>>>>>>
>>>>>>>>>>> Now tell me what's wrong?
>>>>>>>>>>>
>>>>>>>>>>> Thanks
>>>>>>>>>>>
>>>>>>>>>>> Jacques
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>>>>>>
>>>>>>>>>>> As usual, you refuse to revert because you don't understand the
>>>>>>>>>>> code
>>>>>>>>>>>
>>>>>>>>>>>> and I
>>>>>>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>>>>>>> reconsider
>>>>>>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>>>>>>
>>>>>>>>>>>> The quick version:
>>>>>>>>>>>> - copy and paste antipattern
>>>>>>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>>>>>>> executed or
>>>>>>>>>>>> both blocks not executing
>>>>>>>>>>>>
>>>>>>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>>>>>>> killed
>>>>>>>>>>>> is
>>>>>>>>>>>> amazing! It means you do not understand what this code is doing
>>>>>>>>>>>> and
>>>>>>>>>>>> what is
>>>>>>>>>>>> causing the failure.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>>>>>>> jacques.le.roux@les7arts.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>>>>>>> background
>>>>>>>>>>>> instances and it cleaned worked.
>>>>>>>>>>>>
>>>>>>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>>>>>>> works,
>>>>>>>>>>>> and
>>>>>>>>>>>> you get this message
>>>>>>>>>>>>
>>>>>>>>>>>> :ofbiz FAILED
>>>>>>>>>>>> FAILURE: Build failed with an exception.
>>>>>>>>>>>> * What went wrong:
>>>>>>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>>>>>>
>>>>>>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>>>>>>
>>>>>>>>>>>>> with
>>>>>>>>>>>>>
>>>>>>>>>>>>> non-zero exit value 137
>>>>>>>>>>>>>
>>>>>>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>>>>>>
>>>>>>>>>>>> Jacques
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>>>>>>
>>>>>>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>>>>>>
>>>>>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Author: jleroux
>>>>>>>>>>>>>
>>>>>>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>>>>>>
>>>>>>>>>>>>>> New Revision: 1799736
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>>>>>>> Log:
>>>>>>>>>>>>>> No functional change
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Modified:
>>>>>>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>>>>>>> ============================================================
>>>>>>>>>>>>>> ==================
>>>>>>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24
>>>>>>>>>>>>>> 07:56:45
>>>>>>>>>>>>>> 2017
>>>>>>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>>>>>>> standardOutput = processOutput
>>>>>>>>>>>>>> }
>>>>>>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>>>>>>> { line ->
>>>>>>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>>>>>>> if (line ==~
>>>>>>>>>>>>>> /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>>>>> {
>>>>>>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>> + // Only kill if needed
>>>>>>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>>>>> {
>>>>>>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>>> }
>>>>>>>>>>>>>> }
>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>
>>
>
A problem with terminateOfbiz Gradle task (was [Re: svn commit:
r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle)
Posted by Jacques Le Roux <ja...@les7arts.com>.
We still have a problem with terminateOfbiz: it's not portoffset aware.
So when you run it, it closes all OFBiz instances running on a machine (eg on VM demo)
I guess we only need to improve terminateOfbiz by passing a portoffset parameter
I'll create a Jira and provide a patch for review
Jacques
Le 26/06/2017 à 09:37, Jacques Le Roux a écrit :
> OK, the shutdown command is somehow our SIGTERM, makes sense
>
> Jacques
>
>
> Le 26/06/2017 à 08:45, Taher Alkhateeb a écrit :
>> I do not think we should send SIGTERM in the first place. The proper
>> termination of OFBiz is with the --shutdown command which cleanly stops
>> everything and calls the shutdown hook. The gradle task "terminateOfbiz"
>> should only be used if the shutdown command fails (last resort) as cleary
>> mentioned in the docs.
>>
>> On Jun 26, 2017 9:19 AM, "Jacques Le Roux" <ja...@les7arts.com>
>> wrote:
>>
>> Thanks Taher,
>>
>> I indeed missed that kill is not an atomic operation. I reverted at
>> r1799852.
>>
>> Before reading your last message I wanted to propose to set a delay between
>> the 2 operations.
>>
>> So SIGTERM fails. I'd really like to send SIGTERM before killing the
>> process and only kill it if it's necessary (ie if SIGTERM did not terminate
>> the process)
>>
>> So we need to catch the SIGTERM exec and send SIGKILL if it fails, right?
>>
>> Jacques
>>
>>
>>
>> Le 25/06/2017 à 16:05, Taher Alkhateeb a écrit :
>>
>>> Oh and to prove the second scenario, just add a sleep for a few seconds
>>> after the SIGTERM kill command and observe how the system will crash:
>>>
>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>>> Thread.sleep(5000)
>>> }
>>>
>>> Again, this shows you how poor the quality of this code is. It introduces a
>>> subtle bug, it repeats an if condition unnecessarily, and the comments are
>>> wrong "Only kill if needed"
>>>
>>> On Sun, Jun 25, 2017 at 4:22 PM, Taher Alkhateeb <
>>> slidingfilaments@gmail.com
>>>
>>>> wrote:
>>>> I'm completing my answer for the record to show exactly where your code is
>>>> faulty and wrong just like in the other previous cases.
>>>>
>>>> 1- you have a copy-paste pattern in your code
>>>> 2- you are calling the kill command twice immediately one after another
>>>> without checking whether SIGTERM killed the process properly. The whole
>>>> purpose of doing SIGTERM is to allow cleaning resources before enforcing
>>>> SIGKILL. You're not giving the JVM an opportunity to call the shutdown
>>>> hook
>>>> because you're looking up the same output from the same "ps ax" command
>>>> for
>>>> both kill commands.
>>>>
>>>> So your code is working almost exactly as if typing:
>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>>>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>>>> exec { commandLine 'kill', '-KILL', line.tokenize().first() }
>>>> }
>>>>
>>>> Now what's happening is that SIGKILL is executing before SIGTERM
>>>> completes, so the build is successful but OFBiz is killed with SIGKILL not
>>>> SIGTERM. If SIGTERM finishes very quickly (does not happen with me) then
>>>> the build system would crash because the exit value of SIGKILL would be 1,
>>>> not 0 (process not found). So either way it is a problem (either killing
>>>> with SIGKILL or crashing the build system)
>>>>
>>>> Convinced? I hope you can revert now.
>>>>
>>>> On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
>>>> jacques.le.roux@les7arts.com> wrote:
>>>>
>>>> But what's wrong? It's not clear to me, do you see it Michael?
>>>>> Not clearly answering this question just adds more work on my side, I
>>>>> still don't see why I should revert.
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>>
>>>>> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>>>>>
>>>>> I's like to propose the following process to get this worked out without
>>>>>> endless ping-pong here on the mailing lists:
>>>>>>
>>>>>> 1. revert the change as requested
>>>>>>
>>>>>> 2. provide a Jira with patch and review/discuss it there (what's the
>>>>>> problem, how should it be solved etc.)
>>>>>>
>>>>>> 3. come to a conclusion and a thumbsup by other committers
>>>>>>
>>>>>> 4. commit the solution.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Michael
>>>>>>
>>>>>>
>>>>>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>>>>>
>>>>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>>>>>> done
>>>>>>> spending time here and I leave it up to the community to see if they
>>>>>>> want
>>>>>>> incorrect code in the code base.
>>>>>>>
>>>>>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>>>
>>>>>>> Mmm but anyway, I remember now how I thought that.
>>>>>>>
>>>>>>>> In the 2nd "if", if the process was terminated, kill will just say
>>>>>>>> that it
>>>>>>>> can't find the process and exec will return. Else the process will be
>>>>>>>> killed.
>>>>>>>>
>>>>>>>> What's wrong then?
>>>>>>>>
>>>>>>>> Jacques
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>>>>>
>>>>>>>> Ah wait, I see line still contains "start" so will be executed twice
>>>>>>>>
>>>>>>>>> anyway. OK I'll improve that.
>>>>>>>>>
>>>>>>>>> Jacques
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>>>>>
>>>>>>>>> I don't revert without explanations on why I should revert. Sorry,
>>>>>>>>>
>>>>>>>>>> but I
>>>>>>>>>> don't find your explanations clear.
>>>>>>>>>>
>>>>>>>>>> My explanation, tell my what's wrong:
>>>>>>>>>>
>>>>>>>>>> The 2 "if" blocks are executed sequentially for each line containing
>>>>>>>>>> "a
>>>>>>>>>> start"and ignore other lines. I did not change the previous logic,
>>>>>>>>>> just
>>>>>>>>>> added a new if.
>>>>>>>>>>
>>>>>>>>>> Gradle exec spawns a new process and waits till it ends (this point
>>>>>>>>>> is
>>>>>>>>>> important).
>>>>>>>>>>
>>>>>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>>>>>
>>>>>>>>>> If it worked the 2nd "if" does not execute. This is better than
>>>>>>>>>> before
>>>>>>>>>> because it allows the "'start" process "a chance to clean up after
>>>>>>>>>> itself"
>>>>>>>>>> (cf unix.stackexchange.com below)
>>>>>>>>>>
>>>>>>>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>>>>>>>> "if", like it was done before.
>>>>>>>>>>
>>>>>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>>>>>> background.
>>>>>>>>>>
>>>>>>>>>> Now tell me what's wrong?
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>>
>>>>>>>>>> Jacques
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>>>>>
>>>>>>>>>> As usual, you refuse to revert because you don't understand the code
>>>>>>>>>>
>>>>>>>>>>> and I
>>>>>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>>>>>> reconsider
>>>>>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>>>>>
>>>>>>>>>>> The quick version:
>>>>>>>>>>> - copy and paste antipattern
>>>>>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>>>>>> executed or
>>>>>>>>>>> both blocks not executing
>>>>>>>>>>>
>>>>>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>>>>>> killed
>>>>>>>>>>> is
>>>>>>>>>>> amazing! It means you do not understand what this code is doing and
>>>>>>>>>>> what is
>>>>>>>>>>> causing the failure.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>>>>>> jacques.le.roux@les7arts.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>>>>>> background
>>>>>>>>>>> instances and it cleaned worked.
>>>>>>>>>>>
>>>>>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>>>>>> works,
>>>>>>>>>>> and
>>>>>>>>>>> you get this message
>>>>>>>>>>>
>>>>>>>>>>> :ofbiz FAILED
>>>>>>>>>>> FAILURE: Build failed with an exception.
>>>>>>>>>>> * What went wrong:
>>>>>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>>>>>
>>>>>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>>>>>
>>>>>>>>>>>> with
>>>>>>>>>>>>
>>>>>>>>>>>> non-zero exit value 137
>>>>>>>>>>>>
>>>>>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>>>>>
>>>>>>>>>>> Jacques
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>>>>>
>>>>>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>>>>>
>>>>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>>>>>> Author: jleroux
>>>>>>>>>>>>
>>>>>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>>>>>
>>>>>>>>>>>>> New Revision: 1799736
>>>>>>>>>>>>>
>>>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>>>>>> Log:
>>>>>>>>>>>>> No functional change
>>>>>>>>>>>>>
>>>>>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>>>>>
>>>>>>>>>>>>> Modified:
>>>>>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>>>
>>>>>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>>>>>> ============================================================
>>>>>>>>>>>>> ==================
>>>>>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45
>>>>>>>>>>>>> 2017
>>>>>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>>>>>> standardOutput = processOutput
>>>>>>>>>>>>> }
>>>>>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>>>>>> { line ->
>>>>>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>>>> {
>>>>>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>> + }
>>>>>>>>>>>>> + // Only kill if needed
>>>>>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>>>> {
>>>>>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>>> }
>>>>>>>>>>>>> }
>>>>>>>>>>>>> }
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>
>
Re: svn commit: r1799736 -
/ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Jacques Le Roux <ja...@les7arts.com>.
OK, the shutdown command is somehow our SIGTERM, makes sense
Jacques
Le 26/06/2017 à 08:45, Taher Alkhateeb a écrit :
> I do not think we should send SIGTERM in the first place. The proper
> termination of OFBiz is with the --shutdown command which cleanly stops
> everything and calls the shutdown hook. The gradle task "terminateOfbiz"
> should only be used if the shutdown command fails (last resort) as cleary
> mentioned in the docs.
>
> On Jun 26, 2017 9:19 AM, "Jacques Le Roux" <ja...@les7arts.com>
> wrote:
>
> Thanks Taher,
>
> I indeed missed that kill is not an atomic operation. I reverted at
> r1799852.
>
> Before reading your last message I wanted to propose to set a delay between
> the 2 operations.
>
> So SIGTERM fails. I'd really like to send SIGTERM before killing the
> process and only kill it if it's necessary (ie if SIGTERM did not terminate
> the process)
>
> So we need to catch the SIGTERM exec and send SIGKILL if it fails, right?
>
> Jacques
>
>
>
> Le 25/06/2017 à 16:05, Taher Alkhateeb a écrit :
>
>> Oh and to prove the second scenario, just add a sleep for a few seconds
>> after the SIGTERM kill command and observe how the system will crash:
>>
>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>> Thread.sleep(5000)
>> }
>>
>> Again, this shows you how poor the quality of this code is. It introduces a
>> subtle bug, it repeats an if condition unnecessarily, and the comments are
>> wrong "Only kill if needed"
>>
>> On Sun, Jun 25, 2017 at 4:22 PM, Taher Alkhateeb <
>> slidingfilaments@gmail.com
>>
>>> wrote:
>>> I'm completing my answer for the record to show exactly where your code is
>>> faulty and wrong just like in the other previous cases.
>>>
>>> 1- you have a copy-paste pattern in your code
>>> 2- you are calling the kill command twice immediately one after another
>>> without checking whether SIGTERM killed the process properly. The whole
>>> purpose of doing SIGTERM is to allow cleaning resources before enforcing
>>> SIGKILL. You're not giving the JVM an opportunity to call the shutdown
>>> hook
>>> because you're looking up the same output from the same "ps ax" command
>>> for
>>> both kill commands.
>>>
>>> So your code is working almost exactly as if typing:
>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>>> exec { commandLine 'kill', '-KILL', line.tokenize().first() }
>>> }
>>>
>>> Now what's happening is that SIGKILL is executing before SIGTERM
>>> completes, so the build is successful but OFBiz is killed with SIGKILL not
>>> SIGTERM. If SIGTERM finishes very quickly (does not happen with me) then
>>> the build system would crash because the exit value of SIGKILL would be 1,
>>> not 0 (process not found). So either way it is a problem (either killing
>>> with SIGKILL or crashing the build system)
>>>
>>> Convinced? I hope you can revert now.
>>>
>>> On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
>>> jacques.le.roux@les7arts.com> wrote:
>>>
>>> But what's wrong? It's not clear to me, do you see it Michael?
>>>> Not clearly answering this question just adds more work on my side, I
>>>> still don't see why I should revert.
>>>>
>>>> Jacques
>>>>
>>>>
>>>>
>>>> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>>>>
>>>> I's like to propose the following process to get this worked out without
>>>>> endless ping-pong here on the mailing lists:
>>>>>
>>>>> 1. revert the change as requested
>>>>>
>>>>> 2. provide a Jira with patch and review/discuss it there (what's the
>>>>> problem, how should it be solved etc.)
>>>>>
>>>>> 3. come to a conclusion and a thumbsup by other committers
>>>>>
>>>>> 4. commit the solution.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Michael
>>>>>
>>>>>
>>>>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>>>>
>>>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>>>>> done
>>>>>> spending time here and I leave it up to the community to see if they
>>>>>> want
>>>>>> incorrect code in the code base.
>>>>>>
>>>>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>>
>>>>>> Mmm but anyway, I remember now how I thought that.
>>>>>>
>>>>>>> In the 2nd "if", if the process was terminated, kill will just say
>>>>>>> that it
>>>>>>> can't find the process and exec will return. Else the process will be
>>>>>>> killed.
>>>>>>>
>>>>>>> What's wrong then?
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>>>>
>>>>>>> Ah wait, I see line still contains "start" so will be executed twice
>>>>>>>
>>>>>>>> anyway. OK I'll improve that.
>>>>>>>>
>>>>>>>> Jacques
>>>>>>>>
>>>>>>>>
>>>>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>>>>
>>>>>>>> I don't revert without explanations on why I should revert. Sorry,
>>>>>>>>
>>>>>>>>> but I
>>>>>>>>> don't find your explanations clear.
>>>>>>>>>
>>>>>>>>> My explanation, tell my what's wrong:
>>>>>>>>>
>>>>>>>>> The 2 "if" blocks are executed sequentially for each line containing
>>>>>>>>> "a
>>>>>>>>> start"and ignore other lines. I did not change the previous logic,
>>>>>>>>> just
>>>>>>>>> added a new if.
>>>>>>>>>
>>>>>>>>> Gradle exec spawns a new process and waits till it ends (this point
>>>>>>>>> is
>>>>>>>>> important).
>>>>>>>>>
>>>>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>>>>
>>>>>>>>> If it worked the 2nd "if" does not execute. This is better than
>>>>>>>>> before
>>>>>>>>> because it allows the "'start" process "a chance to clean up after
>>>>>>>>> itself"
>>>>>>>>> (cf unix.stackexchange.com below)
>>>>>>>>>
>>>>>>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>>>>>>> "if", like it was done before.
>>>>>>>>>
>>>>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>>>>> background.
>>>>>>>>>
>>>>>>>>> Now tell me what's wrong?
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>>
>>>>>>>>> Jacques
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>>>>
>>>>>>>>> As usual, you refuse to revert because you don't understand the code
>>>>>>>>>
>>>>>>>>>> and I
>>>>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>>>>> reconsider
>>>>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>>>>
>>>>>>>>>> The quick version:
>>>>>>>>>> - copy and paste antipattern
>>>>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>>>>> executed or
>>>>>>>>>> both blocks not executing
>>>>>>>>>>
>>>>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>>>>> killed
>>>>>>>>>> is
>>>>>>>>>> amazing! It means you do not understand what this code is doing and
>>>>>>>>>> what is
>>>>>>>>>> causing the failure.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>>>>> jacques.le.roux@les7arts.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>>>>> background
>>>>>>>>>> instances and it cleaned worked.
>>>>>>>>>>
>>>>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>>>>> works,
>>>>>>>>>> and
>>>>>>>>>> you get this message
>>>>>>>>>>
>>>>>>>>>> :ofbiz FAILED
>>>>>>>>>> FAILURE: Build failed with an exception.
>>>>>>>>>> * What went wrong:
>>>>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>>>>
>>>>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>>>>
>>>>>>>>>>> with
>>>>>>>>>>>
>>>>>>>>>>> non-zero exit value 137
>>>>>>>>>>>
>>>>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>>>>
>>>>>>>>>> Jacques
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>>>>
>>>>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>>>>
>>>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>>>>> Author: jleroux
>>>>>>>>>>>
>>>>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>>>>
>>>>>>>>>>>> New Revision: 1799736
>>>>>>>>>>>>
>>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>>>>> Log:
>>>>>>>>>>>> No functional change
>>>>>>>>>>>>
>>>>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>>>>
>>>>>>>>>>>> Modified:
>>>>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>>
>>>>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>>>>> ============================================================
>>>>>>>>>>>> ==================
>>>>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45
>>>>>>>>>>>> 2017
>>>>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>>>>> standardOutput = processOutput
>>>>>>>>>>>> }
>>>>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>>>>> { line ->
>>>>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>>> {
>>>>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>> + }
>>>>>>>>>>>> + // Only kill if needed
>>>>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>>> {
>>>>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>>> }
>>>>>>>>>>>> }
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Taher Alkhateeb <sl...@gmail.com>.
I do not think we should send SIGTERM in the first place. The proper
termination of OFBiz is with the --shutdown command which cleanly stops
everything and calls the shutdown hook. The gradle task "terminateOfbiz"
should only be used if the shutdown command fails (last resort) as cleary
mentioned in the docs.
On Jun 26, 2017 9:19 AM, "Jacques Le Roux" <ja...@les7arts.com>
wrote:
Thanks Taher,
I indeed missed that kill is not an atomic operation. I reverted at
r1799852.
Before reading your last message I wanted to propose to set a delay between
the 2 operations.
So SIGTERM fails. I'd really like to send SIGTERM before killing the
process and only kill it if it's necessary (ie if SIGTERM did not terminate
the process)
So we need to catch the SIGTERM exec and send SIGKILL if it fails, right?
Jacques
Le 25/06/2017 à 16:05, Taher Alkhateeb a écrit :
> Oh and to prove the second scenario, just add a sleep for a few seconds
> after the SIGTERM kill command and observe how the system will crash:
>
> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
> Thread.sleep(5000)
> }
>
> Again, this shows you how poor the quality of this code is. It introduces a
> subtle bug, it repeats an if condition unnecessarily, and the comments are
> wrong "Only kill if needed"
>
> On Sun, Jun 25, 2017 at 4:22 PM, Taher Alkhateeb <
> slidingfilaments@gmail.com
>
>> wrote:
>> I'm completing my answer for the record to show exactly where your code is
>> faulty and wrong just like in the other previous cases.
>>
>> 1- you have a copy-paste pattern in your code
>> 2- you are calling the kill command twice immediately one after another
>> without checking whether SIGTERM killed the process properly. The whole
>> purpose of doing SIGTERM is to allow cleaning resources before enforcing
>> SIGKILL. You're not giving the JVM an opportunity to call the shutdown
>> hook
>> because you're looking up the same output from the same "ps ax" command
>> for
>> both kill commands.
>>
>> So your code is working almost exactly as if typing:
>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>> exec { commandLine 'kill', '-KILL', line.tokenize().first() }
>> }
>>
>> Now what's happening is that SIGKILL is executing before SIGTERM
>> completes, so the build is successful but OFBiz is killed with SIGKILL not
>> SIGTERM. If SIGTERM finishes very quickly (does not happen with me) then
>> the build system would crash because the exit value of SIGKILL would be 1,
>> not 0 (process not found). So either way it is a problem (either killing
>> with SIGKILL or crashing the build system)
>>
>> Convinced? I hope you can revert now.
>>
>> On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
>> jacques.le.roux@les7arts.com> wrote:
>>
>> But what's wrong? It's not clear to me, do you see it Michael?
>>>
>>> Not clearly answering this question just adds more work on my side, I
>>> still don't see why I should revert.
>>>
>>> Jacques
>>>
>>>
>>>
>>> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>>>
>>> I's like to propose the following process to get this worked out without
>>>> endless ping-pong here on the mailing lists:
>>>>
>>>> 1. revert the change as requested
>>>>
>>>> 2. provide a Jira with patch and review/discuss it there (what's the
>>>> problem, how should it be solved etc.)
>>>>
>>>> 3. come to a conclusion and a thumbsup by other committers
>>>>
>>>> 4. commit the solution.
>>>>
>>>> Thanks,
>>>>
>>>> Michael
>>>>
>>>>
>>>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>>>
>>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>>>> done
>>>>> spending time here and I leave it up to the community to see if they
>>>>> want
>>>>> incorrect code in the code base.
>>>>>
>>>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>
>>>>> Mmm but anyway, I remember now how I thought that.
>>>>>
>>>>>> In the 2nd "if", if the process was terminated, kill will just say
>>>>>> that it
>>>>>> can't find the process and exec will return. Else the process will be
>>>>>> killed.
>>>>>>
>>>>>> What's wrong then?
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>>
>>>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>>>
>>>>>> Ah wait, I see line still contains "start" so will be executed twice
>>>>>>
>>>>>>> anyway. OK I'll improve that.
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>>>
>>>>>>> I don't revert without explanations on why I should revert. Sorry,
>>>>>>>
>>>>>>>> but I
>>>>>>>> don't find your explanations clear.
>>>>>>>>
>>>>>>>> My explanation, tell my what's wrong:
>>>>>>>>
>>>>>>>> The 2 "if" blocks are executed sequentially for each line containing
>>>>>>>> "a
>>>>>>>> start"and ignore other lines. I did not change the previous logic,
>>>>>>>> just
>>>>>>>> added a new if.
>>>>>>>>
>>>>>>>> Gradle exec spawns a new process and waits till it ends (this point
>>>>>>>> is
>>>>>>>> important).
>>>>>>>>
>>>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>>>
>>>>>>>> If it worked the 2nd "if" does not execute. This is better than
>>>>>>>> before
>>>>>>>> because it allows the "'start" process "a chance to clean up after
>>>>>>>> itself"
>>>>>>>> (cf unix.stackexchange.com below)
>>>>>>>>
>>>>>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>>>>>> "if", like it was done before.
>>>>>>>>
>>>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>>>> background.
>>>>>>>>
>>>>>>>> Now tell me what's wrong?
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> Jacques
>>>>>>>>
>>>>>>>>
>>>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>>>
>>>>>>>> As usual, you refuse to revert because you don't understand the code
>>>>>>>>
>>>>>>>>> and I
>>>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>>>> reconsider
>>>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>>>
>>>>>>>>> The quick version:
>>>>>>>>> - copy and paste antipattern
>>>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>>>> executed or
>>>>>>>>> both blocks not executing
>>>>>>>>>
>>>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>>>> killed
>>>>>>>>> is
>>>>>>>>> amazing! It means you do not understand what this code is doing and
>>>>>>>>> what is
>>>>>>>>> causing the failure.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>>>> jacques.le.roux@les7arts.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>>>> background
>>>>>>>>> instances and it cleaned worked.
>>>>>>>>>
>>>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>>>> works,
>>>>>>>>> and
>>>>>>>>> you get this message
>>>>>>>>>
>>>>>>>>> :ofbiz FAILED
>>>>>>>>> FAILURE: Build failed with an exception.
>>>>>>>>> * What went wrong:
>>>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>>>
>>>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>>>
>>>>>>>>>> with
>>>>>>>>>>
>>>>>>>>>> non-zero exit value 137
>>>>>>>>>>
>>>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>>>
>>>>>>>>> Jacques
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>>>
>>>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>>>
>>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>>>>
>>>>>>>>>> Author: jleroux
>>>>>>>>>>
>>>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>>>
>>>>>>>>>>> New Revision: 1799736
>>>>>>>>>>>
>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>>>> Log:
>>>>>>>>>>> No functional change
>>>>>>>>>>>
>>>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>>>
>>>>>>>>>>> Modified:
>>>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>
>>>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>>>> ============================================================
>>>>>>>>>>> ==================
>>>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45
>>>>>>>>>>> 2017
>>>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>>>> standardOutput = processOutput
>>>>>>>>>>> }
>>>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>>>> { line ->
>>>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>> {
>>>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>> + }
>>>>>>>>>>> + // Only kill if needed
>>>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>> {
>>>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>> }
>>>>>>>>>>> }
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>
Re: svn commit: r1799736 -
/ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Jacques Le Roux <ja...@les7arts.com>.
Thanks Taher,
I indeed missed that kill is not an atomic operation. I reverted at r1799852.
Before reading your last message I wanted to propose to set a delay between the 2 operations.
So SIGTERM fails. I'd really like to send SIGTERM before killing the process and only kill it if it's necessary (ie if SIGTERM did not terminate the
process)
So we need to catch the SIGTERM exec and send SIGKILL if it fails, right?
Jacques
Le 25/06/2017 à 16:05, Taher Alkhateeb a écrit :
> Oh and to prove the second scenario, just add a sleep for a few seconds
> after the SIGTERM kill command and observe how the system will crash:
>
> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
> Thread.sleep(5000)
> }
>
> Again, this shows you how poor the quality of this code is. It introduces a
> subtle bug, it repeats an if condition unnecessarily, and the comments are
> wrong "Only kill if needed"
>
> On Sun, Jun 25, 2017 at 4:22 PM, Taher Alkhateeb <slidingfilaments@gmail.com
>> wrote:
>> I'm completing my answer for the record to show exactly where your code is
>> faulty and wrong just like in the other previous cases.
>>
>> 1- you have a copy-paste pattern in your code
>> 2- you are calling the kill command twice immediately one after another
>> without checking whether SIGTERM killed the process properly. The whole
>> purpose of doing SIGTERM is to allow cleaning resources before enforcing
>> SIGKILL. You're not giving the JVM an opportunity to call the shutdown hook
>> because you're looking up the same output from the same "ps ax" command for
>> both kill commands.
>>
>> So your code is working almost exactly as if typing:
>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
>> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
>> exec { commandLine 'kill', '-KILL', line.tokenize().first() }
>> }
>>
>> Now what's happening is that SIGKILL is executing before SIGTERM
>> completes, so the build is successful but OFBiz is killed with SIGKILL not
>> SIGTERM. If SIGTERM finishes very quickly (does not happen with me) then
>> the build system would crash because the exit value of SIGKILL would be 1,
>> not 0 (process not found). So either way it is a problem (either killing
>> with SIGKILL or crashing the build system)
>>
>> Convinced? I hope you can revert now.
>>
>> On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
>> jacques.le.roux@les7arts.com> wrote:
>>
>>> But what's wrong? It's not clear to me, do you see it Michael?
>>>
>>> Not clearly answering this question just adds more work on my side, I
>>> still don't see why I should revert.
>>>
>>> Jacques
>>>
>>>
>>>
>>> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>>>
>>>> I's like to propose the following process to get this worked out without
>>>> endless ping-pong here on the mailing lists:
>>>>
>>>> 1. revert the change as requested
>>>>
>>>> 2. provide a Jira with patch and review/discuss it there (what's the
>>>> problem, how should it be solved etc.)
>>>>
>>>> 3. come to a conclusion and a thumbsup by other committers
>>>>
>>>> 4. commit the solution.
>>>>
>>>> Thanks,
>>>>
>>>> Michael
>>>>
>>>>
>>>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>>>
>>>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>>>> done
>>>>> spending time here and I leave it up to the community to see if they
>>>>> want
>>>>> incorrect code in the code base.
>>>>>
>>>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>>>> jacques.le.roux@les7arts.com> wrote:
>>>>>
>>>>> Mmm but anyway, I remember now how I thought that.
>>>>>> In the 2nd "if", if the process was terminated, kill will just say
>>>>>> that it
>>>>>> can't find the process and exec will return. Else the process will be
>>>>>> killed.
>>>>>>
>>>>>> What's wrong then?
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>>
>>>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>>>
>>>>>> Ah wait, I see line still contains "start" so will be executed twice
>>>>>>> anyway. OK I'll improve that.
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>>>
>>>>>>> I don't revert without explanations on why I should revert. Sorry,
>>>>>>>> but I
>>>>>>>> don't find your explanations clear.
>>>>>>>>
>>>>>>>> My explanation, tell my what's wrong:
>>>>>>>>
>>>>>>>> The 2 "if" blocks are executed sequentially for each line containing
>>>>>>>> "a
>>>>>>>> start"and ignore other lines. I did not change the previous logic,
>>>>>>>> just
>>>>>>>> added a new if.
>>>>>>>>
>>>>>>>> Gradle exec spawns a new process and waits till it ends (this point
>>>>>>>> is
>>>>>>>> important).
>>>>>>>>
>>>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>>>
>>>>>>>> If it worked the 2nd "if" does not execute. This is better than
>>>>>>>> before
>>>>>>>> because it allows the "'start" process "a chance to clean up after
>>>>>>>> itself"
>>>>>>>> (cf unix.stackexchange.com below)
>>>>>>>>
>>>>>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>>>>>> "if", like it was done before.
>>>>>>>>
>>>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>>>> background.
>>>>>>>>
>>>>>>>> Now tell me what's wrong?
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> Jacques
>>>>>>>>
>>>>>>>>
>>>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>>>
>>>>>>>> As usual, you refuse to revert because you don't understand the code
>>>>>>>>> and I
>>>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>>>> reconsider
>>>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>>>
>>>>>>>>> The quick version:
>>>>>>>>> - copy and paste antipattern
>>>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>>>> executed or
>>>>>>>>> both blocks not executing
>>>>>>>>>
>>>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>>>> killed
>>>>>>>>> is
>>>>>>>>> amazing! It means you do not understand what this code is doing and
>>>>>>>>> what is
>>>>>>>>> causing the failure.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>>>> jacques.le.roux@les7arts.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>>>> background
>>>>>>>>> instances and it cleaned worked.
>>>>>>>>>
>>>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>>>> works,
>>>>>>>>> and
>>>>>>>>> you get this message
>>>>>>>>>
>>>>>>>>> :ofbiz FAILED
>>>>>>>>> FAILURE: Build failed with an exception.
>>>>>>>>> * What went wrong:
>>>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>>>
>>>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>>>> with
>>>>>>>>>>
>>>>>>>>>> non-zero exit value 137
>>>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>>>
>>>>>>>>> Jacques
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>>>
>>>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>>>
>>>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>>>>
>>>>>>>>>> Author: jleroux
>>>>>>>>>>
>>>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>>>> New Revision: 1799736
>>>>>>>>>>>
>>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>>>> Log:
>>>>>>>>>>> No functional change
>>>>>>>>>>>
>>>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>>>
>>>>>>>>>>> Modified:
>>>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>>
>>>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>>>> ============================================================
>>>>>>>>>>> ==================
>>>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45
>>>>>>>>>>> 2017
>>>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>>>> standardOutput = processOutput
>>>>>>>>>>> }
>>>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>>>> { line ->
>>>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>> {
>>>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>> + }
>>>>>>>>>>> + // Only kill if needed
>>>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>>> {
>>>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>>> }
>>>>>>>>>>> }
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Taher Alkhateeb <sl...@gmail.com>.
Oh and to prove the second scenario, just add a sleep for a few seconds
after the SIGTERM kill command and observe how the system will crash:
if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
exec { commandLine 'kill', '-TERM', line.tokenize().first() }
Thread.sleep(5000)
}
Again, this shows you how poor the quality of this code is. It introduces a
subtle bug, it repeats an if condition unnecessarily, and the comments are
wrong "Only kill if needed"
On Sun, Jun 25, 2017 at 4:22 PM, Taher Alkhateeb <slidingfilaments@gmail.com
> wrote:
> I'm completing my answer for the record to show exactly where your code is
> faulty and wrong just like in the other previous cases.
>
> 1- you have a copy-paste pattern in your code
> 2- you are calling the kill command twice immediately one after another
> without checking whether SIGTERM killed the process properly. The whole
> purpose of doing SIGTERM is to allow cleaning resources before enforcing
> SIGKILL. You're not giving the JVM an opportunity to call the shutdown hook
> because you're looking up the same output from the same "ps ax" command for
> both kill commands.
>
> So your code is working almost exactly as if typing:
> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
> exec { commandLine 'kill', '-TERM', line.tokenize().first() }
> exec { commandLine 'kill', '-KILL', line.tokenize().first() }
> }
>
> Now what's happening is that SIGKILL is executing before SIGTERM
> completes, so the build is successful but OFBiz is killed with SIGKILL not
> SIGTERM. If SIGTERM finishes very quickly (does not happen with me) then
> the build system would crash because the exit value of SIGKILL would be 1,
> not 0 (process not found). So either way it is a problem (either killing
> with SIGKILL or crashing the build system)
>
> Convinced? I hope you can revert now.
>
> On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
> jacques.le.roux@les7arts.com> wrote:
>
>> But what's wrong? It's not clear to me, do you see it Michael?
>>
>> Not clearly answering this question just adds more work on my side, I
>> still don't see why I should revert.
>>
>> Jacques
>>
>>
>>
>> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>>
>>> I's like to propose the following process to get this worked out without
>>> endless ping-pong here on the mailing lists:
>>>
>>> 1. revert the change as requested
>>>
>>> 2. provide a Jira with patch and review/discuss it there (what's the
>>> problem, how should it be solved etc.)
>>>
>>> 3. come to a conclusion and a thumbsup by other committers
>>>
>>> 4. commit the solution.
>>>
>>> Thanks,
>>>
>>> Michael
>>>
>>>
>>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>>
>>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>>> done
>>>> spending time here and I leave it up to the community to see if they
>>>> want
>>>> incorrect code in the code base.
>>>>
>>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>>> jacques.le.roux@les7arts.com> wrote:
>>>>
>>>> Mmm but anyway, I remember now how I thought that.
>>>>>
>>>>> In the 2nd "if", if the process was terminated, kill will just say
>>>>> that it
>>>>> can't find the process and exec will return. Else the process will be
>>>>> killed.
>>>>>
>>>>> What's wrong then?
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>>
>>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>>
>>>>> Ah wait, I see line still contains "start" so will be executed twice
>>>>>> anyway. OK I'll improve that.
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>>
>>>>>> I don't revert without explanations on why I should revert. Sorry,
>>>>>>> but I
>>>>>>> don't find your explanations clear.
>>>>>>>
>>>>>>> My explanation, tell my what's wrong:
>>>>>>>
>>>>>>> The 2 "if" blocks are executed sequentially for each line containing
>>>>>>> "a
>>>>>>> start"and ignore other lines. I did not change the previous logic,
>>>>>>> just
>>>>>>> added a new if.
>>>>>>>
>>>>>>> Gradle exec spawns a new process and waits till it ends (this point
>>>>>>> is
>>>>>>> important).
>>>>>>>
>>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>>
>>>>>>> If it worked the 2nd "if" does not execute. This is better than
>>>>>>> before
>>>>>>> because it allows the "'start" process "a chance to clean up after
>>>>>>> itself"
>>>>>>> (cf unix.stackexchange.com below)
>>>>>>>
>>>>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>>>>> "if", like it was done before.
>>>>>>>
>>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>>> background.
>>>>>>>
>>>>>>> Now tell me what's wrong?
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>>
>>>>>>> As usual, you refuse to revert because you don't understand the code
>>>>>>>> and I
>>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>>> reconsider
>>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>>
>>>>>>>> The quick version:
>>>>>>>> - copy and paste antipattern
>>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>>> executed or
>>>>>>>> both blocks not executing
>>>>>>>>
>>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>>> killed
>>>>>>>> is
>>>>>>>> amazing! It means you do not understand what this code is doing and
>>>>>>>> what is
>>>>>>>> causing the failure.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>>> jacques.le.roux@les7arts.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>>> background
>>>>>>>> instances and it cleaned worked.
>>>>>>>>
>>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>>> works,
>>>>>>>> and
>>>>>>>> you get this message
>>>>>>>>
>>>>>>>> :ofbiz FAILED
>>>>>>>> FAILURE: Build failed with an exception.
>>>>>>>> * What went wrong:
>>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>>
>>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>>> with
>>>>>>>>>
>>>>>>>>> non-zero exit value 137
>>>>>>>>
>>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>>
>>>>>>>> Jacques
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>>
>>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>>
>>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>>>
>>>>>>>>> Author: jleroux
>>>>>>>>>
>>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>>> New Revision: 1799736
>>>>>>>>>>
>>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>>> Log:
>>>>>>>>>> No functional change
>>>>>>>>>>
>>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>>
>>>>>>>>>> Modified:
>>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>>
>>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>>> ============================================================
>>>>>>>>>> ==================
>>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45
>>>>>>>>>> 2017
>>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>>> standardOutput = processOutput
>>>>>>>>>> }
>>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>>> { line ->
>>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>> {
>>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>> + }
>>>>>>>>>> + // Only kill if needed
>>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>>> tart\.Start.*/)
>>>>>>>>>> {
>>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>>> line.tokenize().first() }
>>>>>>>>>> }
>>>>>>>>>> }
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>>>
>>>
>>>
>>
>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Taher Alkhateeb <sl...@gmail.com>.
I'm completing my answer for the record to show exactly where your code is
faulty and wrong just like in the other previous cases.
1- you have a copy-paste pattern in your code
2- you are calling the kill command twice immediately one after another
without checking whether SIGTERM killed the process properly. The whole
purpose of doing SIGTERM is to allow cleaning resources before enforcing
SIGKILL. You're not giving the JVM an opportunity to call the shutdown hook
because you're looking up the same output from the same "ps ax" command for
both kill commands.
So your code is working almost exactly as if typing:
if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/) {
exec { commandLine 'kill', '-TERM', line.tokenize().first() }
exec { commandLine 'kill', '-KILL', line.tokenize().first() }
}
Now what's happening is that SIGKILL is executing before SIGTERM completes,
so the build is successful but OFBiz is killed with SIGKILL not SIGTERM. If
SIGTERM finishes very quickly (does not happen with me) then the build
system would crash because the exit value of SIGKILL would be 1, not 0
(process not found). So either way it is a problem (either killing with
SIGKILL or crashing the build system)
Convinced? I hope you can revert now.
On Sun, Jun 25, 2017 at 2:20 PM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:
> But what's wrong? It's not clear to me, do you see it Michael?
>
> Not clearly answering this question just adds more work on my side, I
> still don't see why I should revert.
>
> Jacques
>
>
>
> Le 25/06/2017 à 13:11, Michael Brohl a écrit :
>
>> I's like to propose the following process to get this worked out without
>> endless ping-pong here on the mailing lists:
>>
>> 1. revert the change as requested
>>
>> 2. provide a Jira with patch and review/discuss it there (what's the
>> problem, how should it be solved etc.)
>>
>> 3. come to a conclusion and a thumbsup by other committers
>>
>> 4. commit the solution.
>>
>> Thanks,
>>
>> Michael
>>
>>
>> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>>
>>> OK, I take this question as your refusal to revert and cooperate. I'm
>>> done
>>> spending time here and I leave it up to the community to see if they want
>>> incorrect code in the code base.
>>>
>>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>>> jacques.le.roux@les7arts.com> wrote:
>>>
>>> Mmm but anyway, I remember now how I thought that.
>>>>
>>>> In the 2nd "if", if the process was terminated, kill will just say that
>>>> it
>>>> can't find the process and exec will return. Else the process will be
>>>> killed.
>>>>
>>>> What's wrong then?
>>>>
>>>> Jacques
>>>>
>>>>
>>>>
>>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>>
>>>> Ah wait, I see line still contains "start" so will be executed twice
>>>>> anyway. OK I'll improve that.
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>>
>>>>> I don't revert without explanations on why I should revert. Sorry, but
>>>>>> I
>>>>>> don't find your explanations clear.
>>>>>>
>>>>>> My explanation, tell my what's wrong:
>>>>>>
>>>>>> The 2 "if" blocks are executed sequentially for each line containing
>>>>>> "a
>>>>>> start"and ignore other lines. I did not change the previous logic,
>>>>>> just
>>>>>> added a new if.
>>>>>>
>>>>>> Gradle exec spawns a new process and waits till it ends (this point is
>>>>>> important).
>>>>>>
>>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>>
>>>>>> If it worked the 2nd "if" does not execute. This is better than before
>>>>>> because it allows the "'start" process "a chance to clean up after
>>>>>> itself"
>>>>>> (cf unix.stackexchange.com below)
>>>>>>
>>>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>>>> "if", like it was done before.
>>>>>>
>>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>>> background.
>>>>>>
>>>>>> Now tell me what's wrong?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>>
>>>>>> As usual, you refuse to revert because you don't understand the code
>>>>>>> and I
>>>>>>> pay the price of spending my time explaining. I hope you will
>>>>>>> reconsider
>>>>>>> this time consuming and non-cooperative behavior.
>>>>>>>
>>>>>>> The quick version:
>>>>>>> - copy and paste antipattern
>>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>>> executed or
>>>>>>> both blocks not executing
>>>>>>>
>>>>>>> Your belief that Gradle fails because java does not expect to be
>>>>>>> killed
>>>>>>> is
>>>>>>> amazing! It means you do not understand what this code is doing and
>>>>>>> what is
>>>>>>> causing the failure.
>>>>>>>
>>>>>>>
>>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>>> jacques.le.roux@les7arts.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> What makes you think it's wrong? I tested it locally using 2
>>>>>>> background
>>>>>>> instances and it cleaned worked.
>>>>>>>
>>>>>>> I also tried with one standard instance (not in background). It
>>>>>>> works,
>>>>>>> and
>>>>>>> you get this message
>>>>>>>
>>>>>>> :ofbiz FAILED
>>>>>>> FAILURE: Build failed with an exception.
>>>>>>> * What went wrong:
>>>>>>> Execution failed for task ':ofbiz'.
>>>>>>>
>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished
>>>>>>>> with
>>>>>>>>
>>>>>>>> non-zero exit value 137
>>>>>>>
>>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>>
>>>>>>> Jacques
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>>
>>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>>
>>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>>
>>>>>>>> Author: jleroux
>>>>>>>>
>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>>> New Revision: 1799736
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>>> Log:
>>>>>>>>> No functional change
>>>>>>>>>
>>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>>
>>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>>> ============================================================
>>>>>>>>> ==================
>>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45
>>>>>>>>> 2017
>>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>>> standardOutput = processOutput
>>>>>>>>> }
>>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>>> { line ->
>>>>>>>>> + // Try to terminate cleanly
>>>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>> tart\.Start.*/)
>>>>>>>>> {
>>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>>> line.tokenize().first() }
>>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>>> line.tokenize().first() }
>>>>>>>>> + }
>>>>>>>>> + // Only kill if needed
>>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>>> tart\.Start.*/)
>>>>>>>>> {
>>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>>> line.tokenize().first() }
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>
>>
>>
>
Re: svn commit: r1799736 -
/ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Jacques Le Roux <ja...@les7arts.com>.
But what's wrong? It's not clear to me, do you see it Michael?
Not clearly answering this question just adds more work on my side, I still don't see why I should revert.
Jacques
Le 25/06/2017 à 13:11, Michael Brohl a écrit :
> I's like to propose the following process to get this worked out without endless ping-pong here on the mailing lists:
>
> 1. revert the change as requested
>
> 2. provide a Jira with patch and review/discuss it there (what's the problem, how should it be solved etc.)
>
> 3. come to a conclusion and a thumbsup by other committers
>
> 4. commit the solution.
>
> Thanks,
>
> Michael
>
>
> Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
>> OK, I take this question as your refusal to revert and cooperate. I'm done
>> spending time here and I leave it up to the community to see if they want
>> incorrect code in the code base.
>>
>> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
>> jacques.le.roux@les7arts.com> wrote:
>>
>>> Mmm but anyway, I remember now how I thought that.
>>>
>>> In the 2nd "if", if the process was terminated, kill will just say that it
>>> can't find the process and exec will return. Else the process will be
>>> killed.
>>>
>>> What's wrong then?
>>>
>>> Jacques
>>>
>>>
>>>
>>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>>
>>>> Ah wait, I see line still contains "start" so will be executed twice
>>>> anyway. OK I'll improve that.
>>>>
>>>> Jacques
>>>>
>>>>
>>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>>
>>>>> I don't revert without explanations on why I should revert. Sorry, but I
>>>>> don't find your explanations clear.
>>>>>
>>>>> My explanation, tell my what's wrong:
>>>>>
>>>>> The 2 "if" blocks are executed sequentially for each line containing "a
>>>>> start"and ignore other lines. I did not change the previous logic, just
>>>>> added a new if.
>>>>>
>>>>> Gradle exec spawns a new process and waits till it ends (this point is
>>>>> important).
>>>>>
>>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>>
>>>>> If it worked the 2nd "if" does not execute. This is better than before
>>>>> because it allows the "'start" process "a chance to clean up after itself"
>>>>> (cf unix.stackexchange.com below)
>>>>>
>>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>>> "if", like it was done before.
>>>>>
>>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>>> background.
>>>>>
>>>>> Now tell me what's wrong?
>>>>>
>>>>> Thanks
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>>
>>>>>> As usual, you refuse to revert because you don't understand the code
>>>>>> and I
>>>>>> pay the price of spending my time explaining. I hope you will reconsider
>>>>>> this time consuming and non-cooperative behavior.
>>>>>>
>>>>>> The quick version:
>>>>>> - copy and paste antipattern
>>>>>> - incorrect conditional checking leading to both blocks getting
>>>>>> executed or
>>>>>> both blocks not executing
>>>>>>
>>>>>> Your belief that Gradle fails because java does not expect to be killed
>>>>>> is
>>>>>> amazing! It means you do not understand what this code is doing and
>>>>>> what is
>>>>>> causing the failure.
>>>>>>
>>>>>>
>>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>>> jacques.le.roux@les7arts.com>
>>>>>> wrote:
>>>>>>
>>>>>> What makes you think it's wrong? I tested it locally using 2 background
>>>>>> instances and it cleaned worked.
>>>>>>
>>>>>> I also tried with one standard instance (not in background). It works,
>>>>>> and
>>>>>> you get this message
>>>>>>
>>>>>> :ofbiz FAILED
>>>>>> FAILURE: Build failed with an exception.
>>>>>> * What went wrong:
>>>>>> Execution failed for task ':ofbiz'.
>>>>>>
>>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
>>>>>>>
>>>>>> non-zero exit value 137
>>>>>>
>>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>>
>>>>>>
>>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>>
>>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>>
>>>>>>> Author: jleroux
>>>>>>>
>>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>>> New Revision: 1799736
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>>> Log:
>>>>>>>> No functional change
>>>>>>>>
>>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>>> should-i-not-kill-9-a-process
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>>
>>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>>> ============================================================
>>>>>>>> ==================
>>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>>> standardOutput = processOutput
>>>>>>>> }
>>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>>> { line ->
>>>>>>>> + // Try to terminate cleanly
>>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>> tart\.Start.*/)
>>>>>>>> {
>>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>>> line.tokenize().first() }
>>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>>> line.tokenize().first() }
>>>>>>>> + }
>>>>>>>> + // Only kill if needed
>>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>>> tart\.Start.*/)
>>>>>>>> {
>>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>>> line.tokenize().first() }
>>>>>>>> }
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>
>>>>
>
>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Michael Brohl <mi...@ecomify.de>.
I's like to propose the following process to get this worked out without
endless ping-pong here on the mailing lists:
1. revert the change as requested
2. provide a Jira with patch and review/discuss it there (what's the
problem, how should it be solved etc.)
3. come to a conclusion and a thumbsup by other committers
4. commit the solution.
Thanks,
Michael
Am 25.06.17 um 12:36 schrieb Taher Alkhateeb:
> OK, I take this question as your refusal to revert and cooperate. I'm done
> spending time here and I leave it up to the community to see if they want
> incorrect code in the code base.
>
> On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
> jacques.le.roux@les7arts.com> wrote:
>
>> Mmm but anyway, I remember now how I thought that.
>>
>> In the 2nd "if", if the process was terminated, kill will just say that it
>> can't find the process and exec will return. Else the process will be
>> killed.
>>
>> What's wrong then?
>>
>> Jacques
>>
>>
>>
>> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>>
>>> Ah wait, I see line still contains "start" so will be executed twice
>>> anyway. OK I'll improve that.
>>>
>>> Jacques
>>>
>>>
>>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>>
>>>> I don't revert without explanations on why I should revert. Sorry, but I
>>>> don't find your explanations clear.
>>>>
>>>> My explanation, tell my what's wrong:
>>>>
>>>> The 2 "if" blocks are executed sequentially for each line containing "a
>>>> start"and ignore other lines. I did not change the previous logic, just
>>>> added a new if.
>>>>
>>>> Gradle exec spawns a new process and waits till it ends (this point is
>>>> important).
>>>>
>>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>>
>>>> If it worked the 2nd "if" does not execute. This is better than before
>>>> because it allows the "'start" process "a chance to clean up after itself"
>>>> (cf unix.stackexchange.com below)
>>>>
>>>> If the "start" process is not terminated then it's killed by the 2nd
>>>> "if", like it was done before.
>>>>
>>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>>> background.
>>>>
>>>> Now tell me what's wrong?
>>>>
>>>> Thanks
>>>>
>>>> Jacques
>>>>
>>>>
>>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>>
>>>>> As usual, you refuse to revert because you don't understand the code
>>>>> and I
>>>>> pay the price of spending my time explaining. I hope you will reconsider
>>>>> this time consuming and non-cooperative behavior.
>>>>>
>>>>> The quick version:
>>>>> - copy and paste antipattern
>>>>> - incorrect conditional checking leading to both blocks getting
>>>>> executed or
>>>>> both blocks not executing
>>>>>
>>>>> Your belief that Gradle fails because java does not expect to be killed
>>>>> is
>>>>> amazing! It means you do not understand what this code is doing and
>>>>> what is
>>>>> causing the failure.
>>>>>
>>>>>
>>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>>> jacques.le.roux@les7arts.com>
>>>>> wrote:
>>>>>
>>>>> What makes you think it's wrong? I tested it locally using 2 background
>>>>> instances and it cleaned worked.
>>>>>
>>>>> I also tried with one standard instance (not in background). It works,
>>>>> and
>>>>> you get this message
>>>>>
>>>>> :ofbiz FAILED
>>>>> FAILURE: Build failed with an exception.
>>>>> * What went wrong:
>>>>> Execution failed for task ':ofbiz'.
>>>>>
>>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
>>>>>>
>>>>> non-zero exit value 137
>>>>>
>>>>> Which I believe is OK because Java does not expect to be killed!
>>>>>
>>>>> Jacques
>>>>>
>>>>>
>>>>>
>>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>>
>>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>>
>>>>>> Author: jleroux
>>>>>>
>>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>>> New Revision: 1799736
>>>>>>>
>>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>>> Log:
>>>>>>> No functional change
>>>>>>>
>>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>>> should-i-not-kill-9-a-process
>>>>>>>
>>>>>>> Modified:
>>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>>
>>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>>> standardOutput = processOutput
>>>>>>> }
>>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>>> { line ->
>>>>>>> + // Try to terminate cleanly
>>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>> tart\.Start.*/)
>>>>>>> {
>>>>>>> - exec { commandLine 'kill', '-9',
>>>>>>> line.tokenize().first() }
>>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>>> line.tokenize().first() }
>>>>>>> + }
>>>>>>> + // Only kill if needed
>>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>>> tart\.Start.*/)
>>>>>>> {
>>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>>> line.tokenize().first() }
>>>>>>> }
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>>>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Taher Alkhateeb <sl...@gmail.com>.
OK, I take this question as your refusal to revert and cooperate. I'm done
spending time here and I leave it up to the community to see if they want
incorrect code in the code base.
On Sun, Jun 25, 2017 at 1:34 PM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:
> Mmm but anyway, I remember now how I thought that.
>
> In the 2nd "if", if the process was terminated, kill will just say that it
> can't find the process and exec will return. Else the process will be
> killed.
>
> What's wrong then?
>
> Jacques
>
>
>
> Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
>
>> Ah wait, I see line still contains "start" so will be executed twice
>> anyway. OK I'll improve that.
>>
>> Jacques
>>
>>
>> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>>
>>> I don't revert without explanations on why I should revert. Sorry, but I
>>> don't find your explanations clear.
>>>
>>> My explanation, tell my what's wrong:
>>>
>>> The 2 "if" blocks are executed sequentially for each line containing "a
>>> start"and ignore other lines. I did not change the previous logic, just
>>> added a new if.
>>>
>>> Gradle exec spawns a new process and waits till it ends (this point is
>>> important).
>>>
>>> If the line contains "a start" the 1st "if" try to terminate it.
>>>
>>> If it worked the 2nd "if" does not execute. This is better than before
>>> because it allows the "'start" process "a chance to clean up after itself"
>>> (cf unix.stackexchange.com below)
>>>
>>> If the "start" process is not terminated then it's killed by the 2nd
>>> "if", like it was done before.
>>>
>>> As I said it cleanly worked with 2 OFBiz instances running in the
>>> background.
>>>
>>> Now tell me what's wrong?
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>>
>>>> As usual, you refuse to revert because you don't understand the code
>>>> and I
>>>> pay the price of spending my time explaining. I hope you will reconsider
>>>> this time consuming and non-cooperative behavior.
>>>>
>>>> The quick version:
>>>> - copy and paste antipattern
>>>> - incorrect conditional checking leading to both blocks getting
>>>> executed or
>>>> both blocks not executing
>>>>
>>>> Your belief that Gradle fails because java does not expect to be killed
>>>> is
>>>> amazing! It means you do not understand what this code is doing and
>>>> what is
>>>> causing the failure.
>>>>
>>>>
>>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>>> jacques.le.roux@les7arts.com>
>>>> wrote:
>>>>
>>>> What makes you think it's wrong? I tested it locally using 2 background
>>>> instances and it cleaned worked.
>>>>
>>>> I also tried with one standard instance (not in background). It works,
>>>> and
>>>> you get this message
>>>>
>>>> :ofbiz FAILED
>>>> FAILURE: Build failed with an exception.
>>>> * What went wrong:
>>>> Execution failed for task ':ofbiz'.
>>>>
>>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
>>>>>
>>>> non-zero exit value 137
>>>>
>>>> Which I believe is OK because Java does not expect to be killed!
>>>>
>>>> Jacques
>>>>
>>>>
>>>>
>>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>>
>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>>
>>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>>
>>>>> Author: jleroux
>>>>>
>>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>>> New Revision: 1799736
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>>> Log:
>>>>>> No functional change
>>>>>>
>>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>>> should-i-not-kill-9-a-process
>>>>>>
>>>>>> Modified:
>>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>>
>>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>>> ============================================================
>>>>>> ==================
>>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>>> standardOutput = processOutput
>>>>>> }
>>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>>> { line ->
>>>>>> + // Try to terminate cleanly
>>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>> tart\.Start.*/)
>>>>>> {
>>>>>> - exec { commandLine 'kill', '-9',
>>>>>> line.tokenize().first() }
>>>>>> + exec { commandLine 'kill', '-TERM',
>>>>>> line.tokenize().first() }
>>>>>> + }
>>>>>> + // Only kill if needed
>>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>>> tart\.Start.*/)
>>>>>> {
>>>>>> + exec { commandLine 'kill', '-KILL',
>>>>>> line.tokenize().first() }
>>>>>> }
>>>>>> }
>>>>>> }
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>
>>>
>>
>>
>
Re: svn commit: r1799736 -
/ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Jacques Le Roux <ja...@les7arts.com>.
Mmm but anyway, I remember now how I thought that.
In the 2nd "if", if the process was terminated, kill will just say that it can't find the process and exec will return. Else the process will be killed.
What's wrong then?
Jacques
Le 25/06/2017 à 12:28, Jacques Le Roux a écrit :
> Ah wait, I see line still contains "start" so will be executed twice anyway. OK I'll improve that.
>
> Jacques
>
>
> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>> I don't revert without explanations on why I should revert. Sorry, but I don't find your explanations clear.
>>
>> My explanation, tell my what's wrong:
>>
>> The 2 "if" blocks are executed sequentially for each line containing "a start"and ignore other lines. I did not change the previous logic, just
>> added a new if.
>>
>> Gradle exec spawns a new process and waits till it ends (this point is important).
>>
>> If the line contains "a start" the 1st "if" try to terminate it.
>>
>> If it worked the 2nd "if" does not execute. This is better than before because it allows the "'start" process "a chance to clean up after itself"
>> (cf unix.stackexchange.com below)
>>
>> If the "start" process is not terminated then it's killed by the 2nd "if", like it was done before.
>>
>> As I said it cleanly worked with 2 OFBiz instances running in the background.
>>
>> Now tell me what's wrong?
>>
>> Thanks
>>
>> Jacques
>>
>>
>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>> As usual, you refuse to revert because you don't understand the code and I
>>> pay the price of spending my time explaining. I hope you will reconsider
>>> this time consuming and non-cooperative behavior.
>>>
>>> The quick version:
>>> - copy and paste antipattern
>>> - incorrect conditional checking leading to both blocks getting executed or
>>> both blocks not executing
>>>
>>> Your belief that Gradle fails because java does not expect to be killed is
>>> amazing! It means you do not understand what this code is doing and what is
>>> causing the failure.
>>>
>>>
>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <ja...@les7arts.com>
>>> wrote:
>>>
>>> What makes you think it's wrong? I tested it locally using 2 background
>>> instances and it cleaned worked.
>>>
>>> I also tried with one standard instance (not in background). It works, and
>>> you get this message
>>>
>>> :ofbiz FAILED
>>> FAILURE: Build failed with an exception.
>>> * What went wrong:
>>> Execution failed for task ':ofbiz'.
>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
>>> non-zero exit value 137
>>>
>>> Which I believe is OK because Java does not expect to be killed!
>>>
>>> Jacques
>>>
>>>
>>>
>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>
>>>> This commit is wrong and bad on multiple levels. Please revert
>>>>
>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>
>>>> Author: jleroux
>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>> New Revision: 1799736
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>> Log:
>>>>> No functional change
>>>>>
>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>> should-i-not-kill-9-a-process
>>>>>
>>>>> Modified:
>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>
>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>> standardOutput = processOutput
>>>>> }
>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>> { line ->
>>>>> + // Try to terminate cleanly
>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>> tart\.Start.*/)
>>>>> {
>>>>> - exec { commandLine 'kill', '-9',
>>>>> line.tokenize().first() }
>>>>> + exec { commandLine 'kill', '-TERM',
>>>>> line.tokenize().first() }
>>>>> + }
>>>>> + // Only kill if needed
>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>> tart\.Start.*/)
>>>>> {
>>>>> + exec { commandLine 'kill', '-KILL',
>>>>> line.tokenize().first() }
>>>>> }
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>
>>
>>
>
>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Taher Alkhateeb <sl...@gmail.com>.
Good, now I kindly ask you to try not to take up _my_ time for _your_
mistakes by reverting instead of arguing. You refuse every time I ask for a
revert and end up doing it anyway but after taking up what little time we
have by arguing when there is so much work to do.
On Sun, Jun 25, 2017 at 1:28 PM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:
> Ah wait, I see line still contains "start" so will be executed twice
> anyway. OK I'll improve that.
>
> Jacques
>
>
>
> Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
>
>> I don't revert without explanations on why I should revert. Sorry, but I
>> don't find your explanations clear.
>>
>> My explanation, tell my what's wrong:
>>
>> The 2 "if" blocks are executed sequentially for each line containing "a
>> start"and ignore other lines. I did not change the previous logic, just
>> added a new if.
>>
>> Gradle exec spawns a new process and waits till it ends (this point is
>> important).
>>
>> If the line contains "a start" the 1st "if" try to terminate it.
>>
>> If it worked the 2nd "if" does not execute. This is better than before
>> because it allows the "'start" process "a chance to clean up after itself"
>> (cf unix.stackexchange.com below)
>>
>> If the "start" process is not terminated then it's killed by the 2nd
>> "if", like it was done before.
>>
>> As I said it cleanly worked with 2 OFBiz instances running in the
>> background.
>>
>> Now tell me what's wrong?
>>
>> Thanks
>>
>> Jacques
>>
>>
>> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>>
>>> As usual, you refuse to revert because you don't understand the code and
>>> I
>>> pay the price of spending my time explaining. I hope you will reconsider
>>> this time consuming and non-cooperative behavior.
>>>
>>> The quick version:
>>> - copy and paste antipattern
>>> - incorrect conditional checking leading to both blocks getting executed
>>> or
>>> both blocks not executing
>>>
>>> Your belief that Gradle fails because java does not expect to be killed
>>> is
>>> amazing! It means you do not understand what this code is doing and what
>>> is
>>> causing the failure.
>>>
>>>
>>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <
>>> jacques.le.roux@les7arts.com>
>>> wrote:
>>>
>>> What makes you think it's wrong? I tested it locally using 2 background
>>> instances and it cleaned worked.
>>>
>>> I also tried with one standard instance (not in background). It works,
>>> and
>>> you get this message
>>>
>>> :ofbiz FAILED
>>> FAILURE: Build failed with an exception.
>>> * What went wrong:
>>> Execution failed for task ':ofbiz'.
>>>
>>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
>>>>
>>> non-zero exit value 137
>>>
>>> Which I believe is OK because Java does not expect to be killed!
>>>
>>> Jacques
>>>
>>>
>>>
>>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>>
>>> This commit is wrong and bad on multiple levels. Please revert
>>>>
>>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>>
>>>> Author: jleroux
>>>>
>>>>> Date: Sat Jun 24 07:56:45 2017
>>>>> New Revision: 1799736
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>>> Log:
>>>>> No functional change
>>>>>
>>>>> Improves terminateOfbiz byt using TERM before KILL
>>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>>> https://unix.stackexchange.com/questions/8916/when-
>>>>> should-i-not-kill-9-a-process
>>>>>
>>>>> Modified:
>>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>>
>>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>>> ============================================================
>>>>> ==================
>>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>>> standardOutput = processOutput
>>>>> }
>>>>> processOutput.toString().split(System.lineSeparator()).each
>>>>> { line ->
>>>>> + // Try to terminate cleanly
>>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>> tart\.Start.*/)
>>>>> {
>>>>> - exec { commandLine 'kill', '-9',
>>>>> line.tokenize().first() }
>>>>> + exec { commandLine 'kill', '-TERM',
>>>>> line.tokenize().first() }
>>>>> + }
>>>>> + // Only kill if needed
>>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>>> tart\.Start.*/)
>>>>> {
>>>>> + exec { commandLine 'kill', '-KILL',
>>>>> line.tokenize().first() }
>>>>> }
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>
>>
>
Re: svn commit: r1799736 -
/ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Jacques Le Roux <ja...@les7arts.com>.
Ah wait, I see line still contains "start" so will be executed twice anyway. OK I'll improve that.
Jacques
Le 25/06/2017 à 12:18, Jacques Le Roux a écrit :
> I don't revert without explanations on why I should revert. Sorry, but I don't find your explanations clear.
>
> My explanation, tell my what's wrong:
>
> The 2 "if" blocks are executed sequentially for each line containing "a start"and ignore other lines. I did not change the previous logic, just
> added a new if.
>
> Gradle exec spawns a new process and waits till it ends (this point is important).
>
> If the line contains "a start" the 1st "if" try to terminate it.
>
> If it worked the 2nd "if" does not execute. This is better than before because it allows the "'start" process "a chance to clean up after itself"
> (cf unix.stackexchange.com below)
>
> If the "start" process is not terminated then it's killed by the 2nd "if", like it was done before.
>
> As I said it cleanly worked with 2 OFBiz instances running in the background.
>
> Now tell me what's wrong?
>
> Thanks
>
> Jacques
>
>
> Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
>> As usual, you refuse to revert because you don't understand the code and I
>> pay the price of spending my time explaining. I hope you will reconsider
>> this time consuming and non-cooperative behavior.
>>
>> The quick version:
>> - copy and paste antipattern
>> - incorrect conditional checking leading to both blocks getting executed or
>> both blocks not executing
>>
>> Your belief that Gradle fails because java does not expect to be killed is
>> amazing! It means you do not understand what this code is doing and what is
>> causing the failure.
>>
>>
>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <ja...@les7arts.com>
>> wrote:
>>
>> What makes you think it's wrong? I tested it locally using 2 background
>> instances and it cleaned worked.
>>
>> I also tried with one standard instance (not in background). It works, and
>> you get this message
>>
>> :ofbiz FAILED
>> FAILURE: Build failed with an exception.
>> * What went wrong:
>> Execution failed for task ':ofbiz'.
>>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
>> non-zero exit value 137
>>
>> Which I believe is OK because Java does not expect to be killed!
>>
>> Jacques
>>
>>
>>
>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>
>>> This commit is wrong and bad on multiple levels. Please revert
>>>
>>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>>
>>> Author: jleroux
>>>> Date: Sat Jun 24 07:56:45 2017
>>>> New Revision: 1799736
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>>> Log:
>>>> No functional change
>>>>
>>>> Improves terminateOfbiz byt using TERM before KILL
>>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>>> https://unix.stackexchange.com/questions/8916/when-
>>>> should-i-not-kill-9-a-process
>>>>
>>>> Modified:
>>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>>
>>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>>> standardOutput = processOutput
>>>> }
>>>> processOutput.toString().split(System.lineSeparator()).each
>>>> { line ->
>>>> + // Try to terminate cleanly
>>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>> tart\.Start.*/)
>>>> {
>>>> - exec { commandLine 'kill', '-9',
>>>> line.tokenize().first() }
>>>> + exec { commandLine 'kill', '-TERM',
>>>> line.tokenize().first() }
>>>> + }
>>>> + // Only kill if needed
>>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>>> tart\.Start.*/)
>>>> {
>>>> + exec { commandLine 'kill', '-KILL',
>>>> line.tokenize().first() }
>>>> }
>>>> }
>>>> }
>>>>
>>>>
>>>>
>>>>
>
>
Re: svn commit: r1799736 -
/ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Jacques Le Roux <ja...@les7arts.com>.
I don't revert without explanations on why I should revert. Sorry, but I don't find your explanations clear.
My explanation, tell my what's wrong:
The 2 "if" blocks are executed sequentially for each line containing "a start"and ignore other lines. I did not change the previous logic, just added
a new if.
Gradle exec spawns a new process and waits till it ends (this point is important).
If the line contains "a start" the 1st "if" try to terminate it.
If it worked the 2nd "if" does not execute. This is better than before because it allows the "'start" process "a chance to clean up after itself" (cf
unix.stackexchange.com below)
If the "start" process is not terminated then it's killed by the 2nd "if", like it was done before.
As I said it cleanly worked with 2 OFBiz instances running in the background.
Now tell me what's wrong?
Thanks
Jacques
Le 25/06/2017 à 11:22, Taher Alkhateeb a écrit :
> As usual, you refuse to revert because you don't understand the code and I
> pay the price of spending my time explaining. I hope you will reconsider
> this time consuming and non-cooperative behavior.
>
> The quick version:
> - copy and paste antipattern
> - incorrect conditional checking leading to both blocks getting executed or
> both blocks not executing
>
> Your belief that Gradle fails because java does not expect to be killed is
> amazing! It means you do not understand what this code is doing and what is
> causing the failure.
>
>
> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <ja...@les7arts.com>
> wrote:
>
> What makes you think it's wrong? I tested it locally using 2 background
> instances and it cleaned worked.
>
> I also tried with one standard instance (not in background). It works, and
> you get this message
>
> :ofbiz FAILED
> FAILURE: Build failed with an exception.
> * What went wrong:
> Execution failed for task ':ofbiz'.
>> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
> non-zero exit value 137
>
> Which I believe is OK because Java does not expect to be killed!
>
> Jacques
>
>
>
> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>
>> This commit is wrong and bad on multiple levels. Please revert
>>
>> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>>
>> Author: jleroux
>>> Date: Sat Jun 24 07:56:45 2017
>>> New Revision: 1799736
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>>> Log:
>>> No functional change
>>>
>>> Improves terminateOfbiz byt using TERM before KILL
>>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>>> https://unix.stackexchange.com/questions/8916/when-
>>> should-i-not-kill-9-a-process
>>>
>>> Modified:
>>> ofbiz/ofbiz-framework/trunk/build.gradle
>>>
>>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>>> ============================================================
>>> ==================
>>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>>> standardOutput = processOutput
>>> }
>>> processOutput.toString().split(System.lineSeparator()).each
>>> { line ->
>>> + // Try to terminate cleanly
>>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>> tart\.Start.*/)
>>> {
>>> - exec { commandLine 'kill', '-9',
>>> line.tokenize().first() }
>>> + exec { commandLine 'kill', '-TERM',
>>> line.tokenize().first() }
>>> + }
>>> + // Only kill if needed
>>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>>> tart\.Start.*/)
>>> {
>>> + exec { commandLine 'kill', '-KILL',
>>> line.tokenize().first() }
>>> }
>>> }
>>> }
>>>
>>>
>>>
>>>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Taher Alkhateeb <sl...@gmail.com>.
Hi Scott,
I really appreciate your feedback, especially with respect to how
other contributors might react. It is important to stress to all
contributors and committers that I'm a deep believer in community work
and I look forward to learning from and working with all of you. I
warmly remember the initiative that Sharan and I took with the
community in [1]. I also raise my hand as the first person to make
mistakes, both in code and also in communicating.
My language might be a bit harsh, and I will work to tone it down and
be more objective. However, I also ask you all to look at things in
context. This is not a thread in isolation from what has been going on
for a long time. There is a reason and history behind our reactions.
To avoid repeating what we discussed in [2] I will just get to the
point: Jacques, using his own words "Commits a lot". This machine-gun
commit pattern means more quantity and less quality which is the
reason why he gets vetoes. In fact, I cannot even remember when / if I
vetoed others.
Committing faulty / bad quality code in addition to being degrading to
the project, is also time consuming for other contributors /
committers to review. A time which could be spent writing code,
refactoring and doing useful stuff. If you are providing quality
commits consistently and then you make an error then I wouldn't react
the same way to my reaction here.
My perception of the reason you said you "I understand your
frustration" is that you see this pattern as clearly as I do. In many
occasions, I make an objection with (using your words) a "brief
description" for a certain commit, but end up spending lots of time
arguing because Jacques would refuse to revert. So you would expect
after all these incidents that either a) he will be more careful or b)
he would be more willing to revert and cooperate; neither seems to be
happening.
Jacques has an immense amount of energy that I really wish we can
positively utilize in the community by just slowing down and being a
bit more careful. I do not enjoy nor do I find it the best use of my
time to review commits and veto them, and it would be actually much
easier to just ignore all this stuff and say to hell with it. So
please try understand where I'm coming from.
Finally, I apologize for anyone who was displeased with my tone, and
again I look forward to working with all of you.
Cheers,
[1] https://cwiki.apache.org/confluence/display/OFBIZ/Re-Factor+To-Do+List
[2] https://s.apache.org/jB5f
Taher Alkhateeb
On Thu, Jun 29, 2017 at 1:13 AM, Scott Gray
<sc...@hotwaxsystems.com> wrote:
> Hi Taher,
>
> I can understand your frustration, but this:
>
>> This commit is wrong and bad on multiple levels. Please revert
>
>
> is an unproductive and unreasonable request. Whether you want to or not,
> you have to justify the request. Obviously the developer doesn't know what
> is wrong or they wouldn't have committed it in the first place. Asking
> them to revert without giving even a brief explanation of what you find
> wrong leaves the committer completely in the dark and it isn't how vetoes
> are supposed to work. Asking for blind obedience to your demands to save
> you the time of explaining them will never fly with any committer.
>
> Again, I understand your frustration and I've been there myself many times
> in the past but being hostile accomplishes nothing other than your ability
> to work productively with other committers in the future. I can say from
> bitter experience that it does nothing to improve the behavior of whoever
> you're communicating with and if you're being obviously unreasonable it
> will damage your standing with other contributors as well.
>
> Regards
> Scott
>
> On 25 June 2017 at 21:22, Taher Alkhateeb <sl...@gmail.com>
> wrote:
>
>> As usual, you refuse to revert because you don't understand the code and I
>> pay the price of spending my time explaining. I hope you will reconsider
>> this time consuming and non-cooperative behavior.
>>
>> The quick version:
>> - copy and paste antipattern
>> - incorrect conditional checking leading to both blocks getting executed or
>> both blocks not executing
>>
>> Your belief that Gradle fails because java does not expect to be killed is
>> amazing! It means you do not understand what this code is doing and what is
>> causing the failure.
>>
>>
>> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <ja...@les7arts.com>
>> wrote:
>>
>> What makes you think it's wrong? I tested it locally using 2 background
>> instances and it cleaned worked.
>>
>> I also tried with one standard instance (not in background). It works, and
>> you get this message
>>
>> :ofbiz FAILED
>> FAILURE: Build failed with an exception.
>> * What went wrong:
>> Execution failed for task ':ofbiz'.
>> > Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
>> non-zero exit value 137
>>
>> Which I believe is OK because Java does not expect to be killed!
>>
>> Jacques
>>
>>
>>
>> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>>
>> > This commit is wrong and bad on multiple levels. Please revert
>> >
>> > On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>> >
>> > Author: jleroux
>> >> Date: Sat Jun 24 07:56:45 2017
>> >> New Revision: 1799736
>> >>
>> >> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>> >> Log:
>> >> No functional change
>> >>
>> >> Improves terminateOfbiz byt using TERM before KILL
>> >> https://fr.wikipedia.org/wiki/Kill_(Unix)
>> >> https://unix.stackexchange.com/questions/8916/when-
>> >> should-i-not-kill-9-a-process
>> >>
>> >> Modified:
>> >> ofbiz/ofbiz-framework/trunk/build.gradle
>> >>
>> >> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>> >> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>> >> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>> >> ============================================================
>> >> ==================
>> >> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>> >> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>> >> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>> >> standardOutput = processOutput
>> >> }
>> >> processOutput.toString().split(System.lineSeparator()).
>> each
>> >> { line ->
>> >> + // Try to terminate cleanly
>> >> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>> >> tart\.Start.*/)
>> >> {
>> >> - exec { commandLine 'kill', '-9',
>> >> line.tokenize().first() }
>> >> + exec { commandLine 'kill', '-TERM',
>> >> line.tokenize().first() }
>> >> + }
>> >> + // Only kill if needed
>> >> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>> >> tart\.Start.*/)
>> >> {
>> >> + exec { commandLine 'kill', '-KILL',
>> >> line.tokenize().first() }
>> >> }
>> >> }
>> >> }
>> >>
>> >>
>> >>
>> >>
>>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Scott Gray <sc...@hotwaxsystems.com>.
Hi Taher,
I can understand your frustration, but this:
> This commit is wrong and bad on multiple levels. Please revert
is an unproductive and unreasonable request. Whether you want to or not,
you have to justify the request. Obviously the developer doesn't know what
is wrong or they wouldn't have committed it in the first place. Asking
them to revert without giving even a brief explanation of what you find
wrong leaves the committer completely in the dark and it isn't how vetoes
are supposed to work. Asking for blind obedience to your demands to save
you the time of explaining them will never fly with any committer.
Again, I understand your frustration and I've been there myself many times
in the past but being hostile accomplishes nothing other than your ability
to work productively with other committers in the future. I can say from
bitter experience that it does nothing to improve the behavior of whoever
you're communicating with and if you're being obviously unreasonable it
will damage your standing with other contributors as well.
Regards
Scott
On 25 June 2017 at 21:22, Taher Alkhateeb <sl...@gmail.com>
wrote:
> As usual, you refuse to revert because you don't understand the code and I
> pay the price of spending my time explaining. I hope you will reconsider
> this time consuming and non-cooperative behavior.
>
> The quick version:
> - copy and paste antipattern
> - incorrect conditional checking leading to both blocks getting executed or
> both blocks not executing
>
> Your belief that Gradle fails because java does not expect to be killed is
> amazing! It means you do not understand what this code is doing and what is
> causing the failure.
>
>
> On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <ja...@les7arts.com>
> wrote:
>
> What makes you think it's wrong? I tested it locally using 2 background
> instances and it cleaned worked.
>
> I also tried with one standard instance (not in background). It works, and
> you get this message
>
> :ofbiz FAILED
> FAILURE: Build failed with an exception.
> * What went wrong:
> Execution failed for task ':ofbiz'.
> > Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
> non-zero exit value 137
>
> Which I believe is OK because Java does not expect to be killed!
>
> Jacques
>
>
>
> Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
>
> > This commit is wrong and bad on multiple levels. Please revert
> >
> > On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
> >
> > Author: jleroux
> >> Date: Sat Jun 24 07:56:45 2017
> >> New Revision: 1799736
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
> >> Log:
> >> No functional change
> >>
> >> Improves terminateOfbiz byt using TERM before KILL
> >> https://fr.wikipedia.org/wiki/Kill_(Unix)
> >> https://unix.stackexchange.com/questions/8916/when-
> >> should-i-not-kill-9-a-process
> >>
> >> Modified:
> >> ofbiz/ofbiz-framework/trunk/build.gradle
> >>
> >> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
> >> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
> >> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
> >> ============================================================
> >> ==================
> >> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
> >> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
> >> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
> >> standardOutput = processOutput
> >> }
> >> processOutput.toString().split(System.lineSeparator()).
> each
> >> { line ->
> >> + // Try to terminate cleanly
> >> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
> >> tart\.Start.*/)
> >> {
> >> - exec { commandLine 'kill', '-9',
> >> line.tokenize().first() }
> >> + exec { commandLine 'kill', '-TERM',
> >> line.tokenize().first() }
> >> + }
> >> + // Only kill if needed
> >> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
> >> tart\.Start.*/)
> >> {
> >> + exec { commandLine 'kill', '-KILL',
> >> line.tokenize().first() }
> >> }
> >> }
> >> }
> >>
> >>
> >>
> >>
>
Re: svn commit: r1799736 - /ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Taher Alkhateeb <sl...@gmail.com>.
As usual, you refuse to revert because you don't understand the code and I
pay the price of spending my time explaining. I hope you will reconsider
this time consuming and non-cooperative behavior.
The quick version:
- copy and paste antipattern
- incorrect conditional checking leading to both blocks getting executed or
both blocks not executing
Your belief that Gradle fails because java does not expect to be killed is
amazing! It means you do not understand what this code is doing and what is
causing the failure.
On Jun 25, 2017 10:42 AM, "Jacques Le Roux" <ja...@les7arts.com>
wrote:
What makes you think it's wrong? I tested it locally using 2 background
instances and it cleaned worked.
I also tried with one standard instance (not in background). It works, and
you get this message
:ofbiz FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':ofbiz'.
> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with
non-zero exit value 137
Which I believe is OK because Java does not expect to be killed!
Jacques
Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
> This commit is wrong and bad on multiple levels. Please revert
>
> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>
> Author: jleroux
>> Date: Sat Jun 24 07:56:45 2017
>> New Revision: 1799736
>>
>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>> Log:
>> No functional change
>>
>> Improves terminateOfbiz byt using TERM before KILL
>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>> https://unix.stackexchange.com/questions/8916/when-
>> should-i-not-kill-9-a-process
>>
>> Modified:
>> ofbiz/ofbiz-framework/trunk/build.gradle
>>
>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>> ============================================================
>> ==================
>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>> standardOutput = processOutput
>> }
>> processOutput.toString().split(System.lineSeparator()).each
>> { line ->
>> + // Try to terminate cleanly
>> if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>> tart\.Start.*/)
>> {
>> - exec { commandLine 'kill', '-9',
>> line.tokenize().first() }
>> + exec { commandLine 'kill', '-TERM',
>> line.tokenize().first() }
>> + }
>> + // Only kill if needed
>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.s
>> tart\.Start.*/)
>> {
>> + exec { commandLine 'kill', '-KILL',
>> line.tokenize().first() }
>> }
>> }
>> }
>>
>>
>>
>>
Re: svn commit: r1799736 -
/ofbiz/ofbiz-framework/trunk/build.gradle
Posted by Jacques Le Roux <ja...@les7arts.com>.
What makes you think it's wrong? I tested it locally using 2 background instances and it cleaned worked.
I also tried with one standard instance (not in background). It works, and you get this message
:ofbiz FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':ofbiz'.
> Process 'command '/usr/lib/jvm/java-8-oracle/bin/java'' finished with non-zero exit value 137
Which I believe is OK because Java does not expect to be killed!
Jacques
Le 24/06/2017 à 20:04, Taher Alkhateeb a écrit :
> This commit is wrong and bad on multiple levels. Please revert
>
> On Sat, Jun 24, 2017 at 10:56 AM, <jl...@apache.org> wrote:
>
>> Author: jleroux
>> Date: Sat Jun 24 07:56:45 2017
>> New Revision: 1799736
>>
>> URL: http://svn.apache.org/viewvc?rev=1799736&view=rev
>> Log:
>> No functional change
>>
>> Improves terminateOfbiz byt using TERM before KILL
>> https://fr.wikipedia.org/wiki/Kill_(Unix)
>> https://unix.stackexchange.com/questions/8916/when-
>> should-i-not-kill-9-a-process
>>
>> Modified:
>> ofbiz/ofbiz-framework/trunk/build.gradle
>>
>> Modified: ofbiz/ofbiz-framework/trunk/build.gradle
>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>> build.gradle?rev=1799736&r1=1799735&r2=1799736&view=diff
>> ============================================================
>> ==================
>> --- ofbiz/ofbiz-framework/trunk/build.gradle (original)
>> +++ ofbiz/ofbiz-framework/trunk/build.gradle Sat Jun 24 07:56:45 2017
>> @@ -332,8 +332,13 @@ task terminateOfbiz(group: ofbizServer,
>> standardOutput = processOutput
>> }
>> processOutput.toString().split(System.lineSeparator()).each
>> { line ->
>> + // Try to terminate cleanly
>> if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/)
>> {
>> - exec { commandLine 'kill', '-9',
>> line.tokenize().first() }
>> + exec { commandLine 'kill', '-TERM',
>> line.tokenize().first() }
>> + }
>> + // Only kill if needed
>> + if (line ==~ /.*org\.apache\.ofbiz\.base\.start\.Start.*/)
>> {
>> + exec { commandLine 'kill', '-KILL',
>> line.tokenize().first() }
>> }
>> }
>> }
>>
>>
>>