Bug 57025 - SaveService : Better defaults, save thread counts by default
Summary: SaveService : Better defaults, save thread counts by default
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: unspecified
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-27 11:47 UTC by Andrey Pokhilko
Modified: 2014-09-29 18:59 UTC (History)
1 user (show)



Attachments
proposed patch (31.78 KB, patch)
2014-09-27 12:20 UTC, Andrey Pokhilko
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Andrey Pokhilko 2014-09-27 12:20:07 UTC
Created attachment 32063 [details]
proposed patch

Added patch with the change for code and tests.
Comment 2 Sebb 2014-09-27 13:00:01 UTC
Sorry, but that patch is wrong.

The test cases don't need to be changed.
As per the mailing list discussion only the test properties file needs to updated to ensure that the active threads are not saved.

I'm not sure that the code needs to be changed either; the property can just be enabled as true in jmeter.properties. This would be the simplest way to implement the change, and would not affect tests.

Also, this change may break some user tests or analysis of result files, so it needs to be carefully documented in the release notes.
Comment 3 Philippe Mouawad 2014-09-27 13:08:19 UTC
@sebb, in my opinion we should not disabled thread saving in test properties, as this setting change will be the new default, if it is disabled in test we may miss some issue introduced.

Regarding Test Changes, it seems to me fine as per previous remark. I also think we need to change default also in code to be inline with how we manage other properties.

Finally I agree with you we need to document it clearly in changes and I can do it mentioning how to get back to old settings.

Frankly I wonder if there are many users who don't set jmeter.save.saveservice.thread_counts to true.

You absolutely need it to know how much threads were active for some response time.


So in summary I would like to commit patch and update changes to mention how to revert settings.
Comment 4 Sebb 2014-09-27 23:33:47 UTC
(In reply to Philippe Mouawad from comment #3)
> @sebb, in my opinion we should not disabled thread saving in test
> properties, as this setting change will be the new default, if it is
> disabled in test we may miss some issue introduced.

It's important to run tests without the thread count as well, so please leave the existing tests alone.

You can define more tests if you want.

> Regarding Test Changes, it seems to me fine as per previous remark. I also
> think we need to change default also in code to be inline with how we manage
> other properties.

The minimal fix is to define the property as true.
 
> Finally I agree with you we need to document it clearly in changes and I can
> do it mentioning how to get back to old settings.
> 
> Frankly I wonder if there are many users who don't set
> jmeter.save.saveservice.thread_counts to true.

So let's just do that.

> 
> So in summary I would like to commit patch and update changes to mention how
> to revert settings.

I think we should just change the property and update the docs.
Job done.
Comment 5 Andrey Pokhilko 2014-09-28 09:00:46 UTC
If I can't change test files then I don't know how to fix failing batch_scripts if I define thread count saving property... Should I change failing test JMX and disable saving thread counts there?
Comment 6 Sebb 2014-09-28 12:53:54 UTC
(In reply to Andrey Pokhilko from comment #5)
> If I can't change test files then I don't know how to fix failing
> batch_scripts if I define thread count saving property... Should I change
> failing test JMX and disable saving thread counts there?

If you just change jmeter.properties it won't affect the tests, because they use a different properties file. Try it and see.
Comment 7 Andrey Pokhilko 2014-09-28 12:57:01 UTC
Ok. So if just changing the property not affect any tests, the commit becomes trivial change of single line. Do I need to provide a patch for trivial change or somebody from maintainers will just commit it?
Comment 8 Sebb 2014-09-28 12:59:29 UTC
(In reply to Andrey Pokhilko from comment #7)
> Ok. So if just changing the property not affect any tests, the commit
> becomes trivial change of single line. Do I need to provide a patch for
> trivial change or somebody from maintainers will just commit it?

As already mentioned, the documentation also needs to be updated.
Comment 9 Philippe Mouawad 2014-09-29 18:59:26 UTC
Date: Mon Sep 29 18:47:05 2014
New Revision: 1628255

URL: http://svn.apache.org/r1628255
Log:
Bug 57025 - SaveService : Better defaults, save thread counts by default
Bugzilla Id: 57025

Modified:
    jmeter/trunk/bin/jmeter.properties
    jmeter/trunk/bin/testfiles/jmeter-batch.properties
    jmeter/trunk/bin/testfiles/jmetertest.properties
    jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
    jmeter/trunk/xdocs/changes.xml
    jmeter/trunk/xdocs/usermanual/listeners.xml

Date: Mon Sep 29 18:58:47 2014
New Revision: 1628258

URL: http://svn.apache.org/r1628258
Log:
Bug 57025 - SaveService : Better defaults, save thread counts by default
Bugzilla Id: 57025

Modified:
    jmeter/trunk/xdocs/changes.xml