0035886: MobileServerController.getThisServerDefinition creates contention

Openbravo Issue Tracking System - Retail Modules

View Issue Details

Project

Category

View Status

Date Submitted

Last Update

0035886

Retail Modules

StoreServer

public

2017-04-11 15:18

2017-06-09 21:09

Reporter

alostale

Assigned To

mtaal

Priority

normal

Severity

major

Reproducibility

have not tried

Status

closed

Resolution

no change required

Platform

OS Version

Product Version

Target Version

RR17Q2.1

Fixed in Version

Merge Request Status

Review Assigned To

mtaal

OBNetwork customer

Support ticket

Regression level

Regression date

Regression introduced in release

Regression introduced by commit

Triggers an Emergency Pack

Summary

0035886: MobileServerController.getThisServerDefinition creates contention

Description

MobileServerController.getThisServerDefinition method is invoked all the time from many different places. It is synchronized on a singleton class, which in practice, means there is only thread per JVM that can execute it.

This method causes some contention at JVM level.

The situation can become worse in scenarios with overload, if number of DB connections is limited at pool level. Because in some cases, the connection for the thread is borrowed from pool inside this method, this can cause deadlock situations if:

-pool size=1
-thread 1 acquires the connection anywhere
-thread 2 inside this method tries to get a new connection, it cannot get it till thread 1 returns the one it previously borrowed
-thread 1 invokes MobileServerController.getThisServerDefinition
-> DEADLOCK: it won't be able to execute this method till thread 2 completes it, but it is waiting for thread 1 to release its connection

Steps To Reproduce

Can be artificially reproduced by

0. Configure pool as follows:
db.pool.maxActive=5
db.pool.maxWait=10000
1. clone https://bitbucket.org/gpscode/butmultiservertests [^]
2. execute wordDay.jmx setting 10 concurrent threads
3. check JVM contention (attached a screenshot from Yourkit)

Proposed Solution

* Does it make any sense this method to be synchronized?
- If it does not: remove syncrhonization
- If it does: what for? would it properly work in a clustered environment?, in any case look for an alternative

* Additionally, always it's invoked a DB query is triggered, to always return the same record. Could we improve it by some caching?

Additional Information