Does InProcessLockingManager implement a locking anti-pattern?
--------------------------------------------------------------
Key: GEOT-3930
URL: https://jira.codehaus.org/browse/GEOT-3930
Project: GeoTools
Issue Type: Bug
Components: main
Reporter: Kenneth Gulbrandsøy
Assignee: Jody Garnett
I've marked this issue as a critical bug, although it might not be. It depends
on what kind of transactional feature locking GeoTools is ment to officially
support.
After a thorough inspection of code documentation, comments and the the
implementation,
[InProcessLockingManagerInProcessLockingManager|http://svn.osgeo.org/geotools/trunk/modules/library/main/src/main/java/org/geotools/data/InProcessLockingManager.java]
seems to offers both [optimistic and pessimistic transaction
locking|http://docs.jboss.org/jbossas/docs/Server_Configuration_Guide/4/html/TransactionJTA_Overview-Pessimistic_and_optimistic_locking.html].
The optimistic transaction locking implemented by InProcessLockingManager is
aligned with the [WFS
specification|http://portal.opengeospatial.org/files/?artifact_id=39967].
Feature locking described in the WFS specification is a transaction locking
model based on a optimistic locking pattern. This pattern dictates that if a
feature lock is already acquired by a transaction, an exception should be
thrown when another tries to do the same. InProcessLockingManager do throw an
FeatureLockException iff a lock is already acquired for given feature and the
acquired lock type is not a TransactionLock type. InProcessLockingManager pass
all the WFS tests, so this part of the implementation must be correct.
However, InProcessLockingManager also offers pessimistic transaction locking if
the feature lock requested is of type FeatureLock.TRANSACTION (see the
protected factory method [protected synchronized Lock
createLock(Transaction,FeatureLock)|http://svn.osgeo.org/geotools/trunk/modules/library/main/src/main/java/org/geotools/data/InProcessLockingManager.java].
Pessimistic locking is very different from the optimistic locking pattern and
requires much more considerations and careful implementation. Instead of
throwing exceptions, a pessimistic locking pattern involves the use of locking
primitives which block concurrent access to already locked features until the
lock is released by the thread that acquired it. A very important difference
between the optimistic and pessimistic locking pattern is that opposed to the
optimistic locking pattern which only have to _check and fail_ if features are
locked for mutually exclusive _write access_, pessimistic locking have to
_check and block_ on both _read access_ and _write access_ to ensure that
readers on different threads read the same attribute values from each feature.
I'm omitting [transaction
isolation|http://en.wikipedia.org/wiki/Isolation_(database_systems)] levels
from the discussion for the sake of simplicity, but pessimistic locking does
allow less restrictive locking then this. The isolation level described here is
[SERIALIZABLE|http://en.wikipedia.org/wiki/Isolation_(database_systems)#SERIALIZABLE],
which InProcessLockingManager implements partly.
IMHO, this is a critical bug.
To my knowledge, the framework only assert if access to features are locked in
_feature writers_. Feature readers never assert if access to features are
locked. This make sense since WFS feature locking is optimistic. However, it
becomes terrible wrong when pessimistic locking is also offered by default
implementations like InProcessLockingManager. Every datastore implementation
which depend on InProcessLockingManager implementing a pessimistic feature
locking model, along with any client code depending on these to return
transactional readers and writers that are thread-safe, are at risk of dirty
reads and data corruption.
I believe GeoTools have two choices. Rollback the support for pessimistic
feature locking which is broken, or implement any necessary changes to the API,
boilerplate code and default implementations to properly support pessimistic
locking. Regardless of which decision the community makes, transaction support
together with persistence level (not covered here) and feature level locking
support should be thoroughly described in both user and developer documentation
(I am of course feeling obliged to contribute to this work).
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
------------------------------------------------------------------------------
The demand for IT networking professionals continues to grow, and the
demand for specialized networking skills is growing even more rapidly.
Take a complimentary Learning@Cisco Self-Assessment and learn
about Cisco certifications, training, and career opportunities.
http://p.sf.net/sfu/cisco-dev2dev
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
