Another strange stacktrace in agent log

Thomas Segismont tsegismo at redhat.com
Tue Feb 11 08:57:40 UTC 2014


Le 10/02/2014 22:13, Jay Shaughnessy a écrit :
>
> I guess I still don't quite get the details here.   But first one basic
> up-front question:  would we want to consider just making
> Resource.childResources always be a CopyOnWriteArraySet?  Or is the
> thinking that it would incur unnecessary overhead server-side?

On the server side it's not really useful: Resource objects are JPA 
entities and should (almost) always not be updated concurrently.

>
> More questions inline:
>
> On 2/10/2014 12:12 PM, Thomas Segismont wrote:
>> Le 10/02/2014 17:34, Jay Shaughnessy a écrit :
>>> A couple of notes/questions on the current branch changes:
>>> - I'm not sure I quite understand the changes to Resource, with the
>>> customChildResourcesCollection flag. is this really necessary?
>>
>> It is necessary otherwise you can't guarantee that all threads are
>> working on the same, thread-safe collection.
>
> I guess I'm not sure I like the idea of the Resource entity behaving
> differently based on the constructor being used.  What exactly makes
> this necessary?  Why can't we, agent-side, just ensure the
> childResources Set is a CopyOnWriteArraySet, converting when the
> information comes from the server, or initializing when created on the
> agent?

I wasn't in a very creative mood so I just put the code back in the 
state it was before commit 5a63da5b :)

Jokes aside, the Resource class constructor was introduced by Ian in 
commit 3e149ee on 02/21/2012. Have a look at the commit:
https://git.fedorahosted.org/cgit/rhq/rhq.git/commit/?id=3e149ee
The purpose was exactly what we're talking about: "Use a 
ConcurrentHashMap-based Set for childResources to allow the field to be 
concurrently accessed safely"

The only difference I added is the use of CopyOnWriteArraySet instead of 
the default-initialized ConcurrentHashMap-based Set. Default-initialized 
CHM are very bad from a memory consumption perspective when you expect a 
few or no elements in the collection and a low number of concurrent 
threads updating the collection:
http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ConcurrentHashMap.html#ConcurrentHashMap%28%29
"default initial capacity (16), load factor (0.75) and concurrencyLevel 
(16)"

>
> Or, as asked above, maybe just settle on the thread-safe type and be
> done with it.
>

That would be a step backwards considering the job done in commit 
5a63da5b. But if everyone is happy with a simpler version and we still 
meet the performance expectations, I'm fine.

Thomas


More information about the rhq-devel mailing list