You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jmeter.apache.org by bu...@apache.org on 2013/03/18 11:59:01 UTC

[Bug 54717] New: BatchSampleSender/StatisticalSampleSender slows thread handling down

https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

            Bug ID: 54717
           Summary: BatchSampleSender/StatisticalSampleSender slows thread
                    handling down
           Product: JMeter
           Version: 2.9
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Main
          Assignee: issues@jmeter.apache.org
          Reporter: dlade@web.de
    Classification: Unclassified

Because the BatchSampleSender/StatisticalSampleSender synchronizes the whole
sampleOccurred() method it blocks the threads of each thread group. 

The differences of the generate load can be seen using the
DiskStoreSampleSender. In our case it was a factor of 3 faster. But this one
could not be used striped therefore the request body is stored and will be send
at the end (in our case several GB of data).

Although the AsynchSampleSender blocks several times on queue.offer() if the
traffic could not be send fast enough to the client - which might also be a
problem of sending the big request body too.

I suggest to decouple all sample senders from working threads, using a rather
simple idea:
Store the (stripped) sample in a ConcurrentHashMap grouped by the current
thread name and send it by one (ore more) worker thread(s) to the client.

There is no need to block the load generating threads doing their job.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #13 from Danny Lade <dl...@web.de> ---
> In comment #9 you wrote:
> 
> > But the queue is only accessed two times 1) at the initializing for each
> > thread (see statisticalMapSelector) and 2) at the testEnded() method.
>  
> One thread initialises the queue, another processes it.
> 
No, the same thread does this. 

Main: create SampleSender, therefore create queue
Main: start threads
Thread 1: append map to queue
Thread 1: add sample data to own map
...
Thread N: append map to queue
Thread 1: add sample data to own map
...
Thread N: add sample data to own map
Main: all threads ended
Main: call testEnded(), therefore read queue and send sample data

> > As I already mentioned, the solution we've made works quite well in our high
> > load scenarios - believe me.
> 
> Unfortunately that does not prove anything.
>
No it doesn't, but it also sounds like 'Don't trust anyone' to me. 

So, how to prove? What do you have in mind?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #11 from Danny Lade <dl...@web.de> ---
That's all true and well known, but where is the connection to the
SampleSender?

Each Thread (which produces load) can store and send the sample data by it's
own using the SampleSender. The only catch is the end of the tests where the
main thread calls testEnded() method. 
When does one thread need access to the sample data of another thread,
especially inside the SampleSender???

There might be several scenarios where one thread need access to the sample
events of another thread. But IMO this is bad practise for a high load test
because one would provoke synchronization/blocking.
(we never use these technics, each thread has it's own data, nothing shared)

However, if the data is already given to the SampleSender, to send it to the
jmeter (remote) client, no thread needs to share the data stored INSIDE the
SampleSender (fire and forget).

As I already mentioned, the solution we've made works quite well in our high
load scenarios - believe me.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #12 from Sebb <se...@apache.org> ---
(In reply to Danny Lade from comment #11)
> That's all true and well known, but where is the connection to the
> SampleSender?

In comment #9 you wrote:

> But the queue is only accessed two times 1) at the initializing for each
> thread (see statisticalMapSelector) and 2) at the testEnded() method.

One thread initialises the queue, another processes it.

> As I already mentioned, the solution we've made works quite well in our high
> load scenarios - believe me.

Unfortunately that does not prove anything.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #2 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Created attachment 30724
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30724&action=edit
Patch to send SampleEvents outside of synchronized block

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #3 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Hello,
Could you give the attached patch a try ?
It does not yet do what you propose but it should improve the blocking time.

Thanks

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #16 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Looking at your code, I think it is fine regarding multi-threading.
There is something misleading in the comments of this Bugzilla as you spoke
about BatchSampleSender at first.

Thanks for the patch, although not in expected format and with too many
dependencies that could be avoided.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #6 from Sebb <se...@apache.org> ---
(In reply to Sebb from comment #5)
> (In reply to Philippe Mouawad from comment #3)
> > It does not yet do what you propose but it should improve the blocking time.
> 
> The sampleStore needs to be synchronised for both the clone() and clear()
> methods otherwise a sample can be added between the clone() and the clear().
> Or possibly the store can be updated during the clone(). In either case,
> samples can be lost if the code is not synchronised.
> 
> That's partly why the existing code is synchronised - to make sure samples
> are not lost.

Sorry, I see now that the code is still synchronised - the idea of the clone()
is presumably to decouple the sending from the extracting of the samples.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #17 from Danny Lade <dl...@web.de> ---
(In reply to Philippe Mouawad from comment #16)
> Looking at your code, I think it is fine regarding multi-threading.
> There is something misleading in the comments of this Bugzilla as you spoke
> about BatchSampleSender at first.
> 
The BatchSampleSender has exactly the same problem like all samplers which are
synchronizing in sampleOccurred() method. We just wrote the 
StatisticalHoldSampleSender because it is much easier to implement.

> Thanks for the patch, although not in expected format and with too many
> dependencies that could be avoided.

Sorry for that, but we just use the implementation above as it is by patching
the jmeter.properties for
"mode=net.bigpoint.jmeter.samplers.StatisticalHoldSampleSender". It should have
been just an example how one may solve the problem described first.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #10 from Sebb <se...@apache.org> ---
(In reply to Danny Lade from comment #9)
> 
> IMHO this is not necessary to synchronize the sample store if you're using
> an own  store for each thread (see my attachment). 

Only true if there is no access from other threads.

> The only catch is, that testEnded() runs in the main thread. This is why I
> use the queue of maps to store all maps of the threads on one central point.
> But the queue is only accessed two times 1) at the initializing for each
> thread (see statisticalMapSelector) and 2) at the testEnded() method.
> 
> This is why I don't need to synchronize or lock at all.

Synchronisation is not only needed to guarantee single-threaded access to
critical points in the code. It is also needed to ensure safe publication
across threads. The Java memory model allows threads cache values locally;
threads only have to update/fetch main memory at a memory synch. point. If one
thread updates a field, another thread may not see the new value unless both
the threads synch. on the same lock (volatile can also be used in some cases).

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

Sebb <se...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 OS|                            |All
           Severity|normal                      |enhancement

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #5 from Sebb <se...@apache.org> ---
(In reply to Philippe Mouawad from comment #3)
> It does not yet do what you propose but it should improve the blocking time.

The sampleStore needs to be synchronised for both the clone() and clear()
methods otherwise a sample can be added between the clone() and the clear(). Or
possibly the store can be updated during the clone(). In either case, samples
can be lost if the code is not synchronised.

That's partly why the existing code is synchronised - to make sure samples are
not lost.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

Danny Lade <dl...@web.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #30065|0                           |1
        is obsolete|                            |

--- Comment #7 from Danny Lade <dl...@web.de> ---
Created attachment 30729
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30729&action=edit
example code for a totally decoupled statistical sample sender

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #14 from Sebb <se...@apache.org> ---
(In reply to Danny Lade from comment #13)
> > In comment #9 you wrote:
> > 
> > > But the queue is only accessed two times 1) at the initializing for each
> > > thread (see statisticalMapSelector) and 2) at the testEnded() method.
> >  
> > One thread initialises the queue, another processes it.
> > 
> No, the same thread does this. 
> 
> Main: create SampleSender, therefore create queue
> Main: start threads
> Thread 1: append map to queue

The queue entry is set up (i.e. initialised) by a different thread.

Also Thread 1 adds items to the queue, which are later accessed by a different
thread in testEnded(). This also needs to be done in a way that ensures safe
publication, e.g. by using a thread-safe queue (which seems to be the case).

> Thread 1: add sample data to own map
> ...
> Thread N: append map to queue
> Thread 1: add sample data to own map
> ...
> Thread N: add sample data to own map
> Main: all threads ended
> Main: call testEnded(), therefore read queue and send sample data
> 
> > > As I already mentioned, the solution we've made works quite well in our high
> > > load scenarios - believe me.
> > 
> > Unfortunately that does not prove anything.
> >
> No it doesn't, but it also sounds like 'Don't trust anyone' to me. 

I'm just saying it proves nothing.

> So, how to prove? What do you have in mind?

Code inspection should be used to show that objects shared between threads are
protected by a common lock.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #4 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
we could also create a StrippedAsyncBatchMode

Finally I think the Disruptor Framework would be a good answer for the async
sample sender.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #8 from Danny Lade <dl...@web.de> ---
I've uploaded a new version of our implementation, because we've made several
bugfixes. Now it works fine and completely without synchronization at all.

In my opinion it will be much harder to find a solution for batched samplers,
but for hold samplers this solution seems to be the best.

note: 
Ignore the "marker"-stuff in the code. We're using it to measure different
phases of the load test.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |p.mouawad@ubik-ingenierie.c
                   |                            |om
           Hardware|PC                          |All

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #15 from Danny Lade <dl...@web.de> ---
> > Main: create SampleSender, therefore create queue
> > Main: start threads
> > Thread 1: append map to queue
> 
> The queue entry is set up (i.e. initialised) by a different thread.
> 
> Also Thread 1 adds items to the queue, which are later accessed by a
> different thread in testEnded(). This also needs to be done in a way that
> ensures safe publication, e.g. by using a thread-safe queue (which seems to
> be the case).
> 
I see, yes it is a ConcurrentLinkedQueue.

> > So, how to prove? What do you have in mind?
> 
> Code inspection should be used to show that objects shared between threads
> are protected by a common lock.

As you already found out the lock is only necessary at the beginning (init).
Feel free to inspect the ConcurrentLinkedQueue (java.util.concurrent) about it.

However, this code works only for a HoldSampleSender, which sends all data at
the end at once. 
I was also thinking about a BatchSampleSender using the same principles in the
past. But my project does not needed one, therefore I got no time to implement
it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #9 from Danny Lade <dl...@web.de> ---
About your patch:
It still synchronizes the sample store and this is the most time consuming
operation. 

IMHO this is not necessary to synchronize the sample store if you're using an
own  store for each thread (see my attachment). 
The only catch is, that testEnded() runs in the main thread. This is why I use
the queue of maps to store all maps of the threads on one central point. But
the queue is only accessed two times 1) at the initializing for each thread
(see statisticalMapSelector) and 2) at the testEnded() method.

This is why I don't need to synchronize or lock at all.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 54717] BatchSampleSender/StatisticalSampleSender slows thread handling down

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=54717

--- Comment #1 from Danny Lade <dl...@web.de> ---
Created attachment 30065
  --> https://issues.apache.org/bugzilla/attachment.cgi?id=30065&action=edit
example code for a totally decoupled statistical sample sender

-- 
You are receiving this mail because:
You are the assignee for the bug.