Bug 54230 - TCP Sampler, additions of "Close Connection", "SO_LINGER" and "End of line(EOL) byte value" options
TCP Sampler, additions of "Close Connection", "SO_LINGER" and "End of line(EO...
Status: RESOLVED FIXED
Product: JMeter
Classification: Unclassified
Component: Main
Nightly (Please specify date)
All All
: P2 enhancement (vote)
: ---
Assigned To: JMeter issues mailing list
: PatchAvailable
: 40499 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2012-12-01 13:31 UTC by Kurt Hong
Modified: 2012-12-20 22:33 UTC (History)
2 users (show)



Attachments
TCP Sampler patch file and images (28.40 KB, patch)
2012-12-01 13:31 UTC, Kurt Hong
Details | Diff
TCP Sampler patch file (17.12 KB, patch)
2012-12-01 13:38 UTC, Kurt Hong
Details | Diff
TCP Sampler Config screenshot for xdoc (14.61 KB, image/png)
2012-12-01 13:39 UTC, Kurt Hong
Details
TCP Sampler screenshot for xdoc (16.66 KB, image/png)
2012-12-01 13:39 UTC, Kurt Hong
Details
Changed SO_LINGER checkbox to text input box and minor changes. (16.80 KB, patch)
2012-12-03 12:48 UTC, Kurt Hong
Details | Diff
TCP Sampler screenshot for xdoc (15.57 KB, image/png)
2012-12-03 12:49 UTC, Kurt Hong
Details
TCP Sampler Config screenshot for xdoc (13.07 KB, image/png)
2012-12-03 12:50 UTC, Kurt Hong
Details
applied what Sebb said (15.74 KB, patch)
2012-12-04 03:55 UTC, Kurt Hong
Details | Diff
to reflect above change... (17.21 KB, image/png)
2012-12-04 03:56 UTC, Kurt Hong
Details
TCP Sampler Config screenshot for xdoc (14.49 KB, image/png)
2012-12-04 03:57 UTC, Kurt Hong
Details
TCP Sampler screenshot for xdoc (17.21 KB, image/png)
2012-12-04 03:58 UTC, Kurt Hong
Details
applied what Sebb said with some GUI adjustments (15.74 KB, patch)
2012-12-04 05:03 UTC, Kurt Hong
Details | Diff
Cleanning... (13.43 KB, application/octet-stream)
2012-12-04 06:13 UTC, Kurt Hong
Details
TCP Sampler patch file (13.43 KB, patch)
2012-12-04 06:14 UTC, Kurt Hong
Details | Diff
TCP Sampler Config screenshot for xdoc (13.89 KB, image/png)
2012-12-04 06:17 UTC, Kurt Hong
Details
TCP Sampler screenshot for xdoc (16.33 KB, image/png)
2012-12-04 06:18 UTC, Kurt Hong
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kurt Hong 2012-12-01 13:31:02 UTC
Created attachment 29661 [details]
TCP Sampler patch file and images

Please find the attached diff file for source code and screen capture images for xdoc.
I made a patch for TCP Sampler and TCP Sampler Config. This patch was originally made for our company’s testing.

Things changed
- On TCP Sampler Config
Add “SO_LINGER” checkbox and “LINGER Timeout” text input box to prevent large numbers of sockets sitting around with a TIME_WAIT status. It’s related to https://issues.apache.org/bugzilla/show_bug.cgi?id=40499
Add “End of line(EOL) byte value” text input box to set eolByte value on GUI. Setting on GUI will overwrite the setting in properties file. If user doesn’t set the value on GUI, It will use the setting on properties file.
Remove “Re-Use Connection” checkbox, and it will be shown only on TCP Sampler as in case of checkbox, if user utilizes TCP Sampler and TCP Sampler Config together, the settings on the TCP Sampler Config are meaningless.
Remove “TCP Nodelay” checkbox, and it will be shown only on TCP Sampler.

- On TCP Sampler
Add “Close Connection” checkbox to be able to close connection even if user checks “Re-Use Connection” option. In our company’s test, we needed to close the connection at the end of each thread.
           
Also the diff contains a little GUI adjustment for TCP Sampler and Config.

Attached two images’ location is /jmeter/xdocs/images/screenshots.

The issue - https://issues.apache.org/bugzilla/show_bug.cgi?id=40499 – is related to this patch but, he said too generally, not particularly about TCP Sampler. So I will not change the status of the issue.

TODO(Next time)
- Change check boxes to select boxes for those options.
Comment 1 Kurt Hong 2012-12-01 13:38:10 UTC
Created attachment 29662 [details]
TCP Sampler patch file

Please find this file...
Comment 2 Kurt Hong 2012-12-01 13:39:12 UTC
Created attachment 29663 [details]
TCP Sampler Config screenshot for xdoc
Comment 3 Kurt Hong 2012-12-01 13:39:55 UTC
Created attachment 29664 [details]
TCP Sampler screenshot for xdoc
Comment 4 Sebb 2012-12-01 21:04:15 UTC
I'm not sure how the new "Close Connection" option  helps.

Surely, just deselecting "Re-use connection" has the same effect?

The method getSoLingerTimeut() is misspelt, but that could be easily fixed.

Also, I don't think one needs both the boolean SO linger and the Linger timeout fields on the GUI.

If the timeout field is non-empty, only then set SO linger. The documentation needs to make it clear that the linger timeout is only applied when the socket is created.
Comment 5 Kurt Hong 2012-12-02 03:12:24 UTC
(In reply to comment #4)
> I'm not sure how the new "Close Connection" option helps.
At the moment, there is no way to control connections in TCP Sampler on behalf of client. 

Even if the server close the connection, the connection will not be closed immediately. There could effect next loop of the thread. 

In our case, sending "quit" message to server, the server ideally responds "bye" and close the connection. But the response also could be an event message which is made by another sampler of another thread, asynchronously. Then the first sampler of next loop could try to send message to the closed socket. 

So I thought it would be better if we can control the connection on client behalf. 

> Surely, just deselecting "Re-use connection" has the same effect? 
Yes, if you deselect "Re-Use Connection", the "Close connection" checkbox will be disabled, set as "checked" on GUI.

> The method getSoLingerTimeut() is misspelt, but that could be easily fixed.
Sorry for that. I will fix that.

> Also, I don't think one needs both the boolean SO linger and the Linger
> timeout fields on the GUI.
> 
> If the timeout field is non-empty, only then set SO linger. The
> documentation needs to make it clear that the linger timeout is only applied
> when the socket is created.
Yes, I think that's good idea. I'll apply that.

(In reply to comment #4)
> I'm not sure how the new "Close Connection" option  helps.
> 
> Surely, just deselecting "Re-use connection" has the same effect?
> 
> The method getSoLingerTimeut() is misspelt, but that could be easily fixed.
> 
> Also, I don't think one needs both the boolean SO linger and the Linger
> timeout fields on the GUI.
> 
> If the timeout field is non-empty, only then set SO linger. The
> documentation needs to make it clear that the linger timeout is only applied
> when the socket is created.

(In reply to comment #4)
> I'm not sure how the new "Close Connection" option  helps.
> 
> Surely, just deselecting "Re-use connection" has the same effect?
> 
> The method getSoLingerTimeut() is misspelt, but that could be easily fixed.
> 
> Also, I don't think one needs both the boolean SO linger and the Linger
> timeout fields on the GUI.
> 
> If the timeout field is non-empty, only then set SO linger. The
> documentation needs to make it clear that the linger timeout is only applied
> when the socket is created.
Comment 6 Kurt Hong 2012-12-02 03:25:30 UTC
(In reply to comment #4)
> Surely, just deselecting "Re-use connection" has the same effect?
Actually no,
deselecting "Re-Use Connection" will not re-use connection and creat new connection but selecting both of them will try re-use connection and after running the sampler, close the connection.
Comment 7 Sebb 2012-12-02 12:12:03 UTC
(In reply to comment #6)
> (In reply to comment #4)
> > Surely, just deselecting "Re-use connection" has the same effect?
> Actually no,
> deselecting "Re-Use Connection" will not re-use connection and creat new
> connection but selecting both of them will try re-use connection and after
> running the sampler, close the connection.

OK< I see now.
Comment 8 Kurt Hong 2012-12-03 12:48:17 UTC
Created attachment 29670 [details]
Changed  SO_LINGER checkbox to text input box and minor changes.

- Changed  SO_LINGER checkbox to text input box
- Fixed misspelled one
- Modified component_reference.xml to reflect above change.
Comment 9 Kurt Hong 2012-12-03 12:49:37 UTC
Created attachment 29671 [details]
TCP Sampler screenshot for xdoc

to reflect above change....
Comment 10 Kurt Hong 2012-12-03 12:50:08 UTC
Created attachment 29672 [details]
TCP Sampler Config screenshot for xdoc

to reflect above change....
Comment 11 Sebb 2012-12-03 21:48:54 UTC
The existing TCP Config and Sampler GUIs differ only in that the login section is only present on the sampler.

However, your screen shot shows the additional Close Connection box on the Sampler. The TCP config screen shot shows SO_LINGER and EOL only - the Reuse connection and Set NoDelay check-boxes are missing, and there is no Close Connection.

This seems wrong. Existing test plans that use the config element check-boxes will no longer work.

Generally, the config screen is the same as the sampler screen, except for any fields that just don't make sense for a sampler. I'm not sure that applies to any of the new settings. For example if Re-use connection is not selected, then all the settings would apply to a TCP sampler.
Comment 12 Kurt Hong 2012-12-03 23:24:03 UTC
(In reply to comment #11)
> The existing TCP Config and Sampler GUIs differ only in that the login
> section is only present on the sampler.
> 
> However, your screen shot shows the additional Close Connection box on the
> Sampler. The TCP config screen shot shows SO_LINGER and EOL only - the Reuse
> connection and Set NoDelay check-boxes are missing, and there is no Close
> Connection.
> 
> This seems wrong. Existing test plans that use the config element
> check-boxes will no longer work.
> 
> Generally, the config screen is the same as the sampler screen, except for
> any fields that just don't make sense for a sampler. I'm not sure that
> applies to any of the new settings. For example if Re-use connection is not
> selected, then all the settings would apply to a TCP sampler.

But the problem is the checkbox, if a checkbox option is placed on both of sampler and config, the value of the checkbox on the config is useless. So in case of checkbox option, those should be placed only on one of them. So I arranged like that.

And furthermore, Philippe Mouawad suggested replacing checkbox by selectbox like several options on HTTP Sampler. And I agreed with that. I will use select boxes and place them on both of TCP Sampler and TCP Sampler Config on my next patch.

So how do you think I would rearrange those options for now?

Thank you!

(In reply to comment #11)
> The existing TCP Config and Sampler GUIs differ only in that the login
> section is only present on the sampler.
> 
> However, your screen shot shows the additional Close Connection box on the
> Sampler. The TCP config screen shot shows SO_LINGER and EOL only - the Reuse
> connection and Set NoDelay check-boxes are missing, and there is no Close
> Connection.
> 
> This seems wrong. Existing test plans that use the config element
> check-boxes will no longer work.
> 
> Generally, the config screen is the same as the sampler screen, except for
> any fields that just don't make sense for a sampler. I'm not sure that
> applies to any of the new settings. For example if Re-use connection is not
> selected, then all the settings would apply to a TCP sampler.
Comment 13 Sebb 2012-12-04 00:40:36 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > The existing TCP Config and Sampler GUIs differ only in that the login
> > section is only present on the sampler.
> > 
> > However, your screen shot shows the additional Close Connection box on the
> > Sampler. The TCP config screen shot shows SO_LINGER and EOL only - the Reuse
> > connection and Set NoDelay check-boxes are missing, and there is no Close
> > Connection.
> > 
> > This seems wrong. Existing test plans that use the config element
> > check-boxes will no longer work.
> > 
> > Generally, the config screen is the same as the sampler screen, except for
> > any fields that just don't make sense for a sampler. I'm not sure that
> > applies to any of the new settings. For example if Re-use connection is not
> > selected, then all the settings would apply to a TCP sampler.
> 
> But the problem is the checkbox, if a checkbox option is placed on both of
> sampler and config, the value of the checkbox on the config is useless. So
> in case of checkbox option, those should be placed only on one of them. So I
> arranged like that.

Regardless, this Bugzilla is an enhancement request, so should not change the existing behaviour unnecessarily.
 
Also, the text fields should appear on both screens.

> And furthermore, Philippe Mouawad suggested replacing checkbox by selectbox
> like several options on HTTP Sampler. And I agreed with that. I will use
> select boxes and place them on both of TCP Sampler and TCP Sampler Config on
> my next patch.

That is one possible option.

It would be nicer to implement 3-state checkboxes, as that would not require extra screen-space. The indeterminate state would mean to use the default.

So it's not appropriate to fix this as part of this enhancement.

> So how do you think I would rearrange those options for now?

The new fields should be added to the existing GUI, to both config and sampler.

However, the Close Connection makes slightly less sense for the config screen, so could perhaps be omitted from that.
Comment 14 Kurt Hong 2012-12-04 03:55:27 UTC
Created attachment 29675 [details]
applied what Sebb said

all options will be shown on both of TCP Sampler and TCP Sampler Config.
Comment 15 Kurt Hong 2012-12-04 03:56:35 UTC
Created attachment 29676 [details]
to reflect above change...
Comment 16 Kurt Hong 2012-12-04 03:57:29 UTC
Created attachment 29677 [details]
TCP Sampler Config screenshot for xdoc

to reflect above change...
Comment 17 Kurt Hong 2012-12-04 03:58:16 UTC
Created attachment 29678 [details]
TCP Sampler screenshot for xdoc

to reflect above change....
Comment 18 Kurt Hong 2012-12-04 05:03:25 UTC
Created attachment 29679 [details]
applied what Sebb said with some GUI adjustments

I'm not sure if you like this or not, so upload both diff files. 
In case of the first one, align of option components looks ugly. So I tried a little adjustments. You may choose whatever you want.

Thank you.
Comment 19 Kurt Hong 2012-12-04 06:13:15 UTC
Created attachment 29680 [details]
Cleanning...

just cleanning...
Comment 20 Kurt Hong 2012-12-04 06:14:25 UTC
Created attachment 29681 [details]
TCP Sampler patch file
Comment 21 Kurt Hong 2012-12-04 06:17:39 UTC
Created attachment 29682 [details]
TCP Sampler Config screenshot for xdoc

reflect above change
Comment 22 Kurt Hong 2012-12-04 06:18:40 UTC
Created attachment 29683 [details]
TCP Sampler screenshot for xdoc

reflect above change
Comment 23 Sebb 2012-12-04 10:27:32 UTC
Thanks, that looks much better.
Comment 24 Sebb 2012-12-07 16:00:40 UTC
Thanks for the patches and your patience.

Code patch was applied with minor changes - had to change the re-use connection item listener, as that was setting the connection close checkbox. Ok when disabling re-use connection, but when re-use connection is re-enable, the original close connection state was not restored.

URL: http://svn.apache.org/viewvc?rev=1418365&view=rev
Log:
TCP Sampler, additions of "Close Connection", "SO_LINGER" and "End of line(EOL) byte value" options
Bugzilla Id: 54230

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/resources/messages.properties
    jmeter/trunk/src/protocol/tcp/org/apache/jmeter/protocol/tcp/config/gui/TCPConfigGui.java
    jmeter/trunk/src/protocol/tcp/org/apache/jmeter/protocol/tcp/sampler/TCPSampler.java
    jmeter/trunk/xdocs/changes.xml
    jmeter/trunk/xdocs/usermanual/component_reference.xml

URL: http://svn.apache.org/viewvc?rev=1418368&view=rev
Log:
Updated screenshots
Bugzilla Id: 54230

Modified:
    jmeter/trunk/docs/images/screenshots/tcpsampler.png
    jmeter/trunk/docs/images/screenshots/tcpsamplerconfig.png
    jmeter/trunk/xdocs/images/screenshots/tcpsampler.png
    jmeter/trunk/xdocs/images/screenshots/tcpsamplerconfig.png
    jmeter/trunk/xdocs/usermanual/component_reference.xml
Comment 25 Philippe Mouawad 2012-12-20 22:33:35 UTC
*** Bug 40499 has been marked as a duplicate of this bug. ***