instacount/appengine-counter

numRetries doesn't get decremented in ShardedCounterServiceImpl.incrementMemcacheAtomic()

matjazs opened this issue · 1 comments

If memcacheService.putIfUntouched() returns false, numRetries should get decremented as in surrounding MemcacheServiceException handler, but it doesn't.

Nice catch - this isn't too big of a deal because memcacheService.putIfUntouched() will generally only return false if another request has updated the counter in memcache from underneath the "current" request/thread. This happening continuously for more than a few seconds is unlikely except under heavy load, and eventually the GAE request limit will be reached. If this is occuring, it's probably a good idea to increase the default shard-count to something larger so that contention on a single shard is reduced.

That said, this should still be fixed. After looking at the code, I'm thinking it might be preferable to switch this to a "for" loop so that the governor is adjusted in any case as part of the looping logic.