On Fri, May 10, 2013 at 12:17 AM, sebb <[email protected]> wrote: > On 9 May 2013 21:47, Philippe Mouawad <[email protected]> wrote: > > Hello, > > I have implemented the shutdown hook on nightly build (thanks sebb for > > review and nice idea). > > Looks good, but I think there are some improvements that can be made. > For example, the hook is created for every single instance of RC, > however only one is needed as it flushes all the files. > Also, the hook is not needed if the code shuts down normally; it can > be removed upon normal shutdown. > > I think It didn't run in case of normal shutdown as it was removed in testEnded
And since the files field is static, the flushFileOutput method can be > static, so no need to pass the instance to the thread. > > Good catch > I'll make those changes. > > Thanks for the changes, it's much better now :-) > > Milamber if you have time to test and give feedback it would be nice. > > Any other users watching this discussion are also welcome to test. > > > > Test case: > > > > - Run a test > > - Call kill <PID> or CTRL+C during the run and see if you have results > > > > Note you can lose the results emitted during execution of Shutdown hook. > > Regards > > Philippe > > > > On Thu, May 9, 2013 at 1:41 PM, sebb <[email protected]> wrote: > > > >> On 9 May 2013 11:38, Milamber <[email protected]> wrote: > >> > > >> > Le 09/05/2013 11:31, sebb a ecrit : > >> > > >> >> On 9 May 2013 11:15, Milamber <[email protected]> wrote: > >> >>> > >> >>> Le 09/05/2013 10:45, sebb a ecrit : > >> >>> > >> >>>> I don't see any need to tidy up the properties. > >> >>>> > >> >>>> As to the autoflush, I agree that the default should be false, as > that > >> >>>> improves performance. > >> >>> > >> >>> > >> >>> I suppose you would says: the default should be true (activating the > >> >>> autoflush)? > >> >>> > >> >> No, I think the default should be false, as that improves performance > >> >> for the normal case, and it is the original behaviour. > >> > > >> > > >> > In my mind, the original behavior (current in 2.9) is autoflush to > true > >> > (don't lost data). > >> > >> Sorry, I was wrong, you are correct. > >> The original behaviour was autoflush = true as you say. > >> > >> Given that, I'm not 100% convinced we should change the behaviour. > >> > >> Maybe we can find a better way to improve the performance that reduces > >> the possibility of lost output. > >> > >> > > >> > > >> > > >> >> > >> >>>> Let's see if a shutdown hook works. > >> >>>> > >> >>>> On 9 May 2013 10:10, Milamber <[email protected]> wrote: > >> >>>>> > >> >>>>> Le 09/05/2013 09:32, Philippe Mouawad a ecrit : > >> >>>>> > >> >>>>>> On Thursday, May 9, 2013, Milamber wrote: > >> >>>>>> > >> >>>>>>> Le 09/05/2013 08:16, Philippe Mouawad a ecrit : > >> >>>>>>> > >> >>>>>>>> On Thursday, May 9, 2013, Milamber wrote: > >> >>>>>>>> > >> >>>>>>>> Le 08/05/2013 23:03, Philippe Mouawad a ecrit : > >> >>>>>>>>> > >> >>>>>>>>> Bad post, should have gone here > >> >>>>>>>>>> > >> >>>>>>>>>> ---------- Forwarded message ---------- > >> >>>>>>>>>> From: *Philippe Mouawad* > >> >>>>>>>>>> Date: Wednesday, May 8, 2013 > >> >>>>>>>>>> Subject: jmeter.properties cleanup > >> >>>>>>>>>> To: JMeter Users List <[email protected]> > >> >>>>>>>>>> > >> >>>>>>>>>> > >> >>>>>>>>>> Hello, > >> >>>>>>>>>> > >> >>>>>>>>>> jmeter.properties has grown with a lot of properties that > maybe > >> >>>>>>>>>> are > >> >>>>>>>>>> not > >> >>>>>>>>>> that useful. > >> >>>>>>>>>> I find it a good thing that lot of things are configurable in > >> >>>>>>>>>> JMeter > >> >>>>>>>>>> but > >> >>>>>>>>>> maybe it's too much and one of the issues is users may not > find > >> >>>>>>>>>> the > >> >>>>>>>>>> really > >> >>>>>>>>>> useful ones (recently for example with > https.socket.protocols). > >> >>>>>>>>>> > >> >>>>>>>>>> I propose to remove the following: > >> >>>>>>>>>> > >> >>>>>>>>>> - jmeter.loggerpanel.display=****false => It's so > easy > >> to > >> >>>>>>>>>> just > >> >>>>>>>>>> click it > >> >>>>>>>>>> - jmeter.errorscounter.display=****true => Why would > >> >>>>>>>>>> someone > >> >>>>>>>>>> not > >> >>>>>>>>>> want > >> >>>>>>>>>> this > >> >>>>>>>>>> feature ? > >> >>>>>>>>>> - jmeter.toolbar.display=true => Why would someone > not > >> >>>>>>>>>> want > >> >>>>>>>>>> this > >> >>>>>>>>>> cool > >> >>>>>>>>>> feature ? > >> >>>>>>>>>> - jmeter.toolbar => Will users really want to > reorganize > >> >>>>>>>>>> these > >> >>>>>>>>>> icons ? > >> >>>>>>>>>> - jmeter.toolbar.icons => Same as before > >> >>>>>>>>>> > >> >>>>>>>>>> If you are a JMeter plugins developer, you may want to > >> >>>>>>>>>> re-organize > >> >>>>>>>>>> or > >> >>>>>>>>> > >> >>>>>>>>> change the toolbar. > >> >>>>>>>>> > >> >>>>>>>>> - onload.expandtree => Current default behaviour > seems > >> >>>>>>>>> fine > >> >>>>>>>>> no ? > >> >>>>>>>>> > >> >>>>>>>>>> - jmeter.save.saveservice.****autoflush => After some > >> >>>>>>>>>> further > >> >>>>>>>>>> thinking, why > >> >>>>>>>>>> would users not need this one ? If JMeter crashes and > >> some > >> >>>>>>>>>> data > >> >>>>>>>>>> is > >> >>>>>>>>>> lost , > >> >>>>>>>>>> then there are big chances that the test was not that > >> fine > >> >>>>>>>>>> before > >> >>>>>>>>>> the > >> >>>>>>>>>> crash. > >> >>>>>>>>>> > >> >>>>>>>>>> No! I prefer (and I put) this property to the value > "true" ! > >> >>>>>>>>>> If > >> >>>>>>>>>> you > >> >>>>>>>>> > >> >>>>>>>>> make a > >> >>>>>>>>> simple load test and we stop the test with a Ctrl-C, we lost a > >> lot > >> >>>>>>>>> of > >> >>>>>>>>> results (with some tests in my case, I've lost the *entire* > >> results > >> >>>>>>>>> (small > >> >>>>>>>>> test of 5-10 min). Please don't touch this property, and I > >> >>>>>>>>> recommend > >> >>>>>>>>> to > >> >>>>>>>>> put > >> >>>>>>>>> to true by default. It's a very annoying behavior. > >> >>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>>> We could introduce a shutdown hook to handle these Ctrl+C > cases. > >> >>>>>>>>> > >> >>>>>>>> In my opinion it should be false as performances for high > >> throughput > >> >>>>>>>> tests > >> >>>>>>>> are way better. And imho default settings should be the most > >> >>>>>>>> performing > >> >>>>>>>> for a load testing tool no ? > >> >>>>>>>> > >> >>>>>>> Not sure. A new JMeter user can be disoriented if he don't see > his > >> >>>>>>> results. > >> >>>>>>> I've prefer a reliable software that a more performance software > >> >>>>>>> which > >> >>>>>>> can > >> >>>>>>> loose my results. > >> >>>>>> > >> >>>>>> With shutdown hook you won't loose results as quit signal will be > >> >>>>>> trapped > >> >>>>>> and we can flush the file, no ? > >> >>>>>> > >> >>>>>>> Perhaps, make this autoflush behavior more visible in JMeter UI. > >> For > >> >>>>>>> example, add a checkbox in Test Plan (or a checkbox-menu) to > >> >>>>>>> enable/disable > >> >>>>>>> this option. (and add a warning message when the option is > enabled > >> : > >> >>>>>>> "you > >> >>>>>>> can lost some results if you stop the test with Ctrl-C") > >> >>>>>>> > >> >>>>>>> > >> >>>>>>> Do you think it s still needed with what I described above ? > >> >>>>>> > >> >>>>>> What was the scenario that made you lost some resuts ? > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>> A simple scenario : > >> >>>>> > >> >>>>> Thread group with 1 / 1 / infinite loop > >> >>>>> |-- Java Sampler with 1000 ms delay > >> >>>>> > >> >>>>> > >> >>>>> Launch JMeter in non-gui mode, with 30 sec and Ctrl-C : > >> >>>>> > >> >>>>> milamber@ender:~/opt/apache-jmeter-2.10-SNAPSHOT/bin$ ./jmeter > -n -t > >> >>>>> ./lost-results.jmx -l myresults.csv > >> >>>>> Creating summariser <summary> > >> >>>>> Created the tree successfully using ./lost-results.jmx > >> >>>>> Starting the test @ Thu May 09 10:05:23 WEST 2013 (1368090323095) > >> >>>>> Waiting for possible shutdown message on port 4445 > >> >>>>> summary + 7 in 8s = 0.9/s Avg: 1076 Min: 1005 Max: > 1162 > >> >>>>> Err: > >> >>>>> 0 (0.00%) Active: 1 Started: 1 Finished: 0 > >> >>>>> summary + 27 in 30.2s = 0.9/s Avg: 1117 Min: 1023 Max: > 1251 > >> >>>>> Err: > >> >>>>> 0 (0.00%) Active: 1 Started: 1 Finished: 0 > >> >>>>> summary = 34 in 38s = 0.9/s Avg: 1108 Min: 1005 Max: > 1251 > >> >>>>> Err: > >> >>>>> 0 (0.00%) > >> >>>>> ^C > >> >>>>> milamber@ender:~/opt/apache-jmeter-2.10-SNAPSHOT/bin$ wc -l > >> >>>>> myresults.csv > >> >>>>> 0 myresults.csv > >> >>>>> milamber@ender:~/opt/apache-jmeter-2.10-SNAPSHOT/bin$ cat > >> myresults.csv > >> >>>>> <=== > >> >>>>> empty file > >> >>>>> milamber@ender:~/opt/apache-jmeter-2.10-SNAPSHOT/bin$ > >> >>>>> > >> >>>>> Not good in my opinion. > >> >>>>> > >> >>>>> Milamber > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>>> > >> >>>>>> > >> >>>>>>>> Ok for the rest let's keep the statu quo if you think all are > >> >>>>>>>> needed. > >> >>>>>>>> > >> >>>>>>>> I have doubts about those ones: > >> >>>>>>>>>> > >> >>>>>>>>>> # Netscape HTTP Cookie file > >> >>>>>>>>>> cookies=cookies => What does it do ? > >> >>>>>>>>>> > >> >>>>>>>>>> We could try to remove them and if users want them, we would > >> have > >> >>>>>>>>>> some > >> >>>>>>>>>> bugzilla request to get them back. > >> >>>>>>>>>> > >> >>>>>>>>>> If you remove these properties, you introduce a lot of > >> >>>>>>>>>> incompatibilty > >> >>>>>>>>> > >> >>>>>>>>> changes and (in my opinion) you remove some freedom of the > user's > >> >>>>>>>>> preferences. Please double check before remove. > >> >>>>>>>>> > >> >>>>>>>>> Milamber. > >> >>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>>> > >> > > >> > > > > > > > > -- > > Cordialement. > > Philippe Mouawad. > -- Cordialement. Philippe Mouawad.
