RE: [coldbox:8831] Re: Element cbox_handler-app.handlers.products is undefined in a Java object of type class java.util.concurrent.ConcurrentHashMap

bdw429s · March 21, 2011, 5:19pm

We have RC1 on production and occasionally see errors like this. We are about to roll out RC2, but it’s still in testing on stage.

Here is an example:

Mark_Mandel · March 21, 2011, 8:32pm

Yeah, it’s very occasional.

Seems like it only happens when there hasn’t been access on the site for a bit.

If traffic is constant, it never shows up.

Mark

adk · March 22, 2011, 6:04am

I got this consistently (daily, but usually only once per day...) on a
site. See my previous post here:

http://groups.google.com/group/coldbox/browse_thread/thread/83cbc8d3852f08a5/b19d6105fbbfc7a8?lnk=gst&q=adk#b19d6105fbbfc7a8

I have also gotten several people who replied to my personal email
address - rather than to the group - saying they were experiencing the
same issue. I asked each one to post about it on the above thread in
hopes of getting a group discussion going, but none did so.

Bottom line is that I do not know what specifically to do to recreate
it, but I did receive the error EVERY DAY. The site in question is an
NFL football pool so it is only up seasonally... so currently is on
hiatus! but I would love to figure out what's going on? A
simple hit of the browser refresh button fixed the issue and
unfortunately that's what we had to do...

Andy

Mark_Mandel · March 22, 2011, 11:36pm

Well, it’s good to know it’s not just me.

Sounds like a threading issue. I had a quick peek through, but couldn’t see anything.

Looks like something is clear()'ing it just at the same time as it is being accessed. Is that possible?

Mark

lmajano · March 23, 2011, 12:06am

yea, not sure, what this might be, need more data on this one to alter stuff.
Luis F. Majano
President
Ortus Solutions, Corp

ColdBox Platform: http://www.coldbox.org
Linked In: http://www.linkedin.com/pub/3/731/483
Blog: http://www.luismajano.com
IECFUG Manager: http://www.iecfug.com

Mark_Mandel · March 23, 2011, 12:23am

What do you need? It’s very hard to reproduce (as threading bugs tend to be)

Mark

lmajano · March 23, 2011, 12:35am

I know, that is why it is so hard, maybe looking at the concurrent object store, it has the appropriate locks on the get/set/clear methods, according to objects requested, so they don’t overlap.

Is this on the latest in github?

If you want, I can try an alternative locking strategy for the metadata and see how that works out?

Luis F. Majano
President
Ortus Solutions, Corp

ColdBox Platform: http://www.coldbox.org
Linked In: http://www.linkedin.com/pub/3/731/483
Blog: http://www.luismajano.com
IECFUG Manager: http://www.iecfug.com

Mark_Mandel · March 23, 2011, 12:49am

The code I have is from the 2011-02-22

Looking at the ConcurrentStore, that was exactly the sort of locking I was thinking should be in place.

Mark

lmajano · March 23, 2011, 1:02am

Yea, the only thing I can think of is maybe the indexer locking strategy can be reworked and let the store do the locking instead. Just a hunch.

Luis F. Majano
President
Ortus Solutions, Corp

ColdBox Platform: http://www.coldbox.org
Linked In: http://www.linkedin.com/pub/3/731/483
Blog: http://www.luismajano.com
IECFUG Manager: http://www.iecfug.com

bdw429s · March 23, 2011, 6:08am

A few thoughts on this:

I would suggest using some load tools to try and reproduce the bug. Andrew reported that his codebase would error daily when receiving traffic. Perhaps he would let you toy with his app and some load testing programs.
This is sort of off-the-wall and probably unrelated, but why do you create a java.util.concurrent.ConcurrentHashMap for the poolMetadata and not just use a regular ColdFusion struct? Are we killing ants with hand grenades?
The ConcurrentStore.get() method employs a read only lock, yet it modifies that item’s metadata (And the MetadataIndexer employs no locking of its own) If 20 concurrent request all called get for the same item at once, can you really be sure the “hits” metadata property is going to be correct?
Both lookup() and get() both employ locks, but that only ensures consistency within those atomic operations. If you are going to call lookup() and then get() in that order (like CacheBoxProvider.getQuiet() does in Mark’s stack trace) shouldn’t you lock that entire operation since you are not guaranteed that the item won’t be removed from the cache after the lookup() is run but prior to the get() being run. This issue has actually bugged me for a while since it is common to check for existence and then get something from the cache. Unless I am going to employ my own application-level locking (which would need to use the same lock names as the cache does internally to be effective) I can’t enforce repeatable reads. To use the DB analogy, you should be selecting with(holdlock) inside a transaction to guarantee you can read the same record later but you’re not.
Thanks!

~Brad

lmajano · March 23, 2011, 4:36pm

I would suggest using some load tools to try and reproduce the bug. Andrew reported that his codebase would error daily when receiving traffic. Perhaps he would let you toy with his app and some load testing programs.

Yes, I have some load tests here, but can’t reproduce

This is sort of off-the-wall and probably unrelated, but why do you create a java.util.concurrent.ConcurrentHashMap for the poolMetadata and not just use a regular ColdFusion struct? Are we killing ants with hand grenades?

Not really, because I don’t want the structures to lock the entire map when doing operations on the cache, they go hand in hand. The concurrent locks enable atomic operations on the indexer as well, which enables performance and also less threading issues as we only need to do row locking.

The ConcurrentStore.get() method employs a read only lock, yet it modifies that item’s metadata (And the MetadataIndexer employs no locking of its own) If 20 concurrent request all called get for the same item at once, can you really be sure the “hits” metadata property is going to be correct?

Yes, I agree on this one, I will revisit the locking strategy on this too.

Both lookup() and get() both employ locks, but that only ensures consistency within those atomic operations. If you are going to call lookup() and then get() in that order (like CacheBoxProvider.getQuiet() does in Mark’s stack trace) shouldn’t you lock that entire operation since you are not guaranteed that the item won’t be removed from the cache after the lookup() is run but prior to the get() being run. This issue has actually bugged me for a while since it is common to check for existence and then get something from the cache. Unless I am going to employ my own application-level locking (which would need to use the same lock names as the cache does internally to be effective) I can’t enforce repeatable reads. To use the DB analogy, you should be selecting with(holdlock) inside a transaction to guarantee you can read the same record later but you’re not.

I agree. I think the indexer is the issue here.

lmajano · March 23, 2011, 5:55pm

Ok guys, thanks to y'alls feedback, I reworked the locking strategies
and created a new branch you can test on.

https://github.com/coldbox/coldbox-platform/tree/cache-concurrency

Please give it a shot. If all is good, I will re-integrate just in
time for 3.0 release and cachebox 1.2 release. ON my tests, it works
pretty sweet, and actually a little faster on indexing.

Thanks

Mark_Mandel · March 29, 2011, 11:04pm

Just deployed this last night. This error hasn’t shown up in our error logs in the past 12 hours, so looks like it should be fixed…

Mark

lmajano · March 30, 2011, 1:02am

Hurray!! Just in time for 3.0.0 goodness!! Thanks Mark,you rock as always!
Luis F. Majano
President
Ortus Solutions, Corp

ColdBox Platform: http://www.coldbox.org
Linked In: http://www.linkedin.com/pub/3/731/483
Blog: http://www.luismajano.com
IECFUG Manager: http://www.iecfug.com

RE: [coldbox:8831] Re: Element cbox_handler-app.handlers.products is undefined in a Java object of type class java.util.concurrent.ConcurrentHashMap

Latest Ortus News

GIVING BACK

HARVESTING IN SPANISH

WOODSEDGE