On 9 May 2013 23:17, 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. > > And since the files field is static, the flushFileOutput method can be > static, so no need to pass the instance to the thread. > > I'll make those changes.
The end of an XML file is not written by this code. I think that is OK - it's easy enough to add it manually, and the missing trailer indicates that there was a problem with the test. For CSV files of course one cannot tell. It would be possible for the hook to call finalizeFileOutput() instead which would produce a tidier XML file. But I'm not sure that the best idea, as explained above. >> 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.
