moodlehq / moodle-performance-comparison

Set of shell scripts to run Moodle performance tests using different hardware and configurations and compare results.
GNU General Public License v2.0
75 stars 39 forks source link

recorder.bsf is writing the header multiple times #3

Closed dmonllao closed 10 years ago

dmonllao commented 11 years ago

Depending on the ramp up and throughput values the results recorder writes multiple php headers which breaks the PHP syntax and makes the runs unusable

dmonllao commented 10 years ago

I have spend a couple of hours dealing with TestListener and ThreadListener to avoid using the same BeanShell script but I could not manage to create the file and populate it from them; for what I have seen the methods are not opened to read properties sent from the recorder. The problem was random, but since I've integrated this solution I had no more broken files.

PD: Hopefully tomorrow I will being working on other issues

dmonllao commented 10 years ago

It can be integrated in master and 25

dmonllao commented 10 years ago

Well, I don't know how to convert this issue to a PR...

dmonllao commented 10 years ago

Updating patch finally using listeners as we can not rely on any other option. This is the commit (https://github.com/dmonllao/moodle-performance-comparison/commit/8dc38a0c5a32651277d98967d2e01adecc75a01a) can be merged into 2.5, 2.6 and master

dmonllao commented 10 years ago

This patch has been tested successfully in the CI server where the resulting runs files were broken when comparing master stable and master integration

stronk7 commented 10 years ago

LOL, David, I'm a bit lost reading this... are all those commits different, or versions of the same? I've looked to your branches and the issue_3 seems to be the related one, but it's 1 commit ahead and 1 commit behind master.

So... or you clarify it and I can know what to look for... or if you know it... go ahead and push the fix (as far as I don't know much about those internals).

Ciao :-)

dmonllao commented 10 years ago

Hi Eloy, it is the same commit, all our branches are the same but changing the default basecommit and branches (webserver_config.properties)

What happened with my repository's issue_3 and master is that I already cherry-picked it to my master branch (it was not set as pull branch)

I'll merge the changes so I can begin with http://docs.moodle.org/dev/Release_process_(Combined)#Package_day #6 now that we have the 2.6 hash

dmonllao commented 10 years ago

Changes merged in the 3 maintained branches. The recorder scripts runs for every thread, the previous code was checking if the generated php file with all the test plan run results was created, and if it was not it creates it. The issue was that sometimes the threads ran too fast and the second thread result runs when the file is being created by the first thread result and the run info (included in all the threads) were added (not deleting the previous one as it was appending the file) and it should only be added once, when the first thread starts; it was a big issue because this run info began with <?php, so there was a random php syntax error.

With this patch we use listeners to create the file only once at testStarted and we rename it to a proper name (the run timestamp) when testEnded.