JDO
  1. JDO
  2. JDO-619

API required for enabling/disabling FOR UPDATE locking for SELECTs

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: JDO 3 (3.0)
    • Component/s: api, specification, tck
    • Labels:
      None

      Description

      We - http://www.jfire.org - have some code where it is essential that objects read from the datastore are not manipulated by another transaction before they are modified and written to the datastore. In SQL, you use "SELECT ... FOR UPDATE" for this purpose, which locks the records included in the query result till the end of the transaction just like a write operation does.

      In JDO, it is currently not yet possible to control whether reading causes read-locks (simple SELECT) or write-locks (SELECT ... FOR UPDATE). There are, however, vendor-specific solutions already. Thus, I'd like to first point out how DataNucleus solves this problem:

      1) The page RDBMS persistence properties<http://www.datanucleus.org/products/accessplatform/rdbms/persistence_properties.html>
      describes the property "datanucleus.rdbms.useUpdateLock" which applies to all queries. This would leave our code pure-JDO (not
      DataNucleus-dependent), but it's unfortunately not what we need: Most of the time a lock is not required and this option would therefore
      unnecessarily slow down our application.

      2) The page JDOQL<http://www.datanucleus.org/products/accessplatform/rdbms/jdoql.html> shows this code snippet:

      ((org.datanucleus.jdo.JDOTransaction)pm.currentTransaction()).setOption(
      "transaction.serializeReadObjects", "true"
      );

      This applies to all subsequent queries of one transaction. It works fine to enable/disable the option back and forth during the same transaction. Obviously, this is the most useful way to control the use of write-locks during read operations.

      3) Additionally, the same page mentions that you can set "datanucleus.rdbms.query.useUpdateLock" as a JDOQL extension. I assume
      that's simply code like this:

      query.addExtension("datanucleus.rdbms.query.useUpdateLock", "true");

      In contrast to solution (2), this only affects the single explicit query and no implicit queries which are used when accessing fields of the returned object(s).

      Having explained all this, I'd like to request the following feature for the next JDO release (2.3):

      Please extend javax.jdo.Transaction and add 2 methods:

      void setSerializeReadObjects(boolean)
      boolean isSerializeReadObjects()

      This would make DataNucleus' solution (2) - see above - available via the JDO API.

      Additionally, please extend javax.jdo.Query and add 2 new method:

      void setSerializeReadObjects(boolean)
      boolean isSerializeReadObjects()

      This represents JDO-API for DataNucleus' solution (3) - see above.

      1. jdo619.patch
        133 kB
        Andy Jefferson

        Activity

        Hide
        Michael Bouschen added a comment -

        I think enabling/disabling FOR UPDATE locking for SELECTs is a useful feature and we should discuss how an API should look like to support this.

        One way is to specify this statically in the JDO metadata for a persistence capable class: e.g. add a boolean property useUpdateLock to the class metadata. This might help in most cases, but is not flexible enough, because it cannot be changed at runtime. Maybe we can add the same property useUpdateLock to the fetchplan and/or fetchgroup. This would allow FOR UPDATE locking being enabled and disabled dynamically at runtime.

        Show
        Michael Bouschen added a comment - I think enabling/disabling FOR UPDATE locking for SELECTs is a useful feature and we should discuss how an API should look like to support this. One way is to specify this statically in the JDO metadata for a persistence capable class: e.g. add a boolean property useUpdateLock to the class metadata. This might help in most cases, but is not flexible enough, because it cannot be changed at runtime. Maybe we can add the same property useUpdateLock to the fetchplan and/or fetchgroup. This would allow FOR UPDATE locking being enabled and disabled dynamically at runtime.
        Hide
        Craig L Russell added a comment -

        > One way is to specify this statically in the JDO metadata for a persistence capable class: e.g. add a boolean property useUpdateLock to the class metadata.

        I agree this is a useful feature and it's good to have a standard name for the concept. SerializeRead is a good crisp concept name, and we could use this for metadata as well as the API:

        element class, element interface attribute serialize-read for metadata
        Transaction and Query: setSerializeRead and getSerializeRead for API

        One issue that I see is when using fetch plans. Which objects should be read with FOR UPDATE? Only root objects, or the entire fetch plan object graph?

        Another issue is whether you can override the setting in metadata. If you have a class Foo that in metadata has serialize-read, and your Transaction has SerializeRead false (presumably the default) do you use serialize-read for Foo or not? I'd say yes. How do you describe this behavior?

        Show
        Craig L Russell added a comment - > One way is to specify this statically in the JDO metadata for a persistence capable class: e.g. add a boolean property useUpdateLock to the class metadata. I agree this is a useful feature and it's good to have a standard name for the concept. SerializeRead is a good crisp concept name, and we could use this for metadata as well as the API: element class, element interface attribute serialize-read for metadata Transaction and Query: setSerializeRead and getSerializeRead for API One issue that I see is when using fetch plans. Which objects should be read with FOR UPDATE? Only root objects, or the entire fetch plan object graph? Another issue is whether you can override the setting in metadata. If you have a class Foo that in metadata has serialize-read, and your Transaction has SerializeRead false (presumably the default) do you use serialize-read for Foo or not? I'd say yes. How do you describe this behavior?
        Hide
        Andy Jefferson added a comment -

        Patch for review as per the comments in this issue

        Show
        Andy Jefferson added a comment - Patch for review as per the comments in this issue
        Hide
        Andy Jefferson added a comment -

        Patch applied to SVN trunk

        Show
        Andy Jefferson added a comment - Patch applied to SVN trunk
        Hide
        Craig L Russell added a comment -

        We haven't discussed comments from 12-Dec-2008. The patch doesn't answer these comments.

        Until we discuss and agree to the change, please revert the patch.

        Show
        Craig L Russell added a comment - We haven't discussed comments from 12-Dec-2008. The patch doesn't answer these comments. Until we discuss and agree to the change, please revert the patch.
        Hide
        Andy Jefferson added a comment -

        I took it as accepted that the methods were needed (since 2 were in favour and none against), and in the absence of comments on the patch applied it. Ooops

        Re: Dec 12 comments,
        MetaData change makes sense, needing to go into JDO DTD/XSD (but not ORM DTD/XSD), as well as adding to @Persistable annotation, as well as ClassMetadata/InterfaceMetadata API objects. should be Boolean type defaulting to unspecified.

        Fetch Plans : What is read FOR UPDATE ? I'd assume all objects read in the query itself would take the serialize setting as per the priority order (below). So if the query fires off 2 SELECTs one for candidate and one for related objects then

        • the candidate will use FOR UPDATE as per query, and if not set then txn, and if not set then metadata
        • the related will use FOR UPDATE as per query, and if not set then txn, and if not set then metadata.

        Priority order for serializeRead settings ? I'd look at Query value having top priority, and if not set then use txn value, and if not set use metadata setting. Why should metadata have priority ? it is statically defined in annotations/XML yet the txn/query values are at runtime (dynamic).

        Show
        Andy Jefferson added a comment - I took it as accepted that the methods were needed (since 2 were in favour and none against), and in the absence of comments on the patch applied it. Ooops Re: Dec 12 comments, MetaData change makes sense, needing to go into JDO DTD/XSD (but not ORM DTD/XSD), as well as adding to @Persistable annotation, as well as ClassMetadata/InterfaceMetadata API objects. should be Boolean type defaulting to unspecified. Fetch Plans : What is read FOR UPDATE ? I'd assume all objects read in the query itself would take the serialize setting as per the priority order (below). So if the query fires off 2 SELECTs one for candidate and one for related objects then the candidate will use FOR UPDATE as per query, and if not set then txn, and if not set then metadata the related will use FOR UPDATE as per query, and if not set then txn, and if not set then metadata. Priority order for serializeRead settings ? I'd look at Query value having top priority, and if not set then use txn value, and if not set use metadata setting. Why should metadata have priority ? it is statically defined in annotations/XML yet the txn/query values are at runtime (dynamic).
        Hide
        Craig L Russell added a comment -

        > I took it as accepted that the methods were needed (since 2 were in favour and none against), and in the absence of comments on the patch applied it. Ooops

        No problem. It's just that the patch as applied was incomplete from my perspective.

        > MetaData change makes sense, needing to go into JDO DTD/XSD (but not ORM DTD/XSD), as well as adding to @Persistable annotation, as well as ClassMetadata/InterfaceMetadata API objects. should be Boolean type defaulting to unspecified.

        I agree.

        > * the related will use FOR UPDATE as per query, and if not set then txn, and if not set then metadata.

        The implication is that you cannot via API read FOR UPDATE only the root instances of the query. The most common use case I have seen is where you use FOR UPDATE as a guard for queried objects. For example, you use FOR UPDATE on the Order instances and then read the related OrderLine instances without a lock because they are guarded by the lock on the Order. But if we have the metadata option, that might be ok.

        Still, we could consider setSerializeRelatedRead(boolean related) as a separate flag. I'd like to get others' input on this.

        > Priority order for serializeRead settings ? I'd look at Query value having top priority, and if not set then use txn value, and if not set use metadata setting. Why should metadata have priority ? it is statically defined in annotations/XML yet the txn/query values are at runtime (dynamic).

        I agree.

        Show
        Craig L Russell added a comment - > I took it as accepted that the methods were needed (since 2 were in favour and none against), and in the absence of comments on the patch applied it. Ooops No problem. It's just that the patch as applied was incomplete from my perspective. > MetaData change makes sense, needing to go into JDO DTD/XSD (but not ORM DTD/XSD), as well as adding to @Persistable annotation, as well as ClassMetadata/InterfaceMetadata API objects. should be Boolean type defaulting to unspecified. I agree. > * the related will use FOR UPDATE as per query, and if not set then txn, and if not set then metadata. The implication is that you cannot via API read FOR UPDATE only the root instances of the query. The most common use case I have seen is where you use FOR UPDATE as a guard for queried objects. For example, you use FOR UPDATE on the Order instances and then read the related OrderLine instances without a lock because they are guarded by the lock on the Order. But if we have the metadata option, that might be ok. Still, we could consider setSerializeRelatedRead(boolean related) as a separate flag. I'd like to get others' input on this. > Priority order for serializeRead settings ? I'd look at Query value having top priority, and if not set then use txn value, and if not set use metadata setting. Why should metadata have priority ? it is statically defined in annotations/XML yet the txn/query values are at runtime (dynamic). I agree.
        Hide
        Andy Jefferson added a comment -

        Updated patch. This time it includes
        1). Update to @PersistenceCapable to include "serializeRead" defaulting to false.
        2). Update to ComponentMetadata to include serializeRead property, using boolean
        3). Addition of get/setSerializeRead on Query taking Boolean.
        4). Addition of get/setSerializeRead on Transaction taking Boolean
        5). Addition of DTD/XSD for all XML components of JDO2.3 for completeness (these include "serialize-read" on <class>/<interface> with default of "false". The DTD/XSD for 'orm'/'jdoquery'/'jdoconfig' aren't changed from 2.2, just copied for completeness since we need to update those for 'jdo'.

        Thought it was best to have a default on the metadata (XML/annotations) since that is the ultimate fallback so we get defined behaviour when nothing is specified. Query and Transaction, since the override the metadata, use Boolean. Also thought that "false" was the best default since it likely wouldn't be a good policy to just lock everything. Opinions may differ

        Show
        Andy Jefferson added a comment - Updated patch. This time it includes 1). Update to @PersistenceCapable to include "serializeRead" defaulting to false. 2). Update to ComponentMetadata to include serializeRead property, using boolean 3). Addition of get/setSerializeRead on Query taking Boolean. 4). Addition of get/setSerializeRead on Transaction taking Boolean 5). Addition of DTD/XSD for all XML components of JDO2.3 for completeness (these include "serialize-read" on <class>/<interface> with default of "false". The DTD/XSD for 'orm'/'jdoquery'/'jdoconfig' aren't changed from 2.2, just copied for completeness since we need to update those for 'jdo'. Thought it was best to have a default on the metadata (XML/annotations) since that is the ultimate fallback so we get defined behaviour when nothing is specified. Query and Transaction, since the override the metadata, use Boolean. Also thought that "false" was the best default since it likely wouldn't be a good policy to just lock everything. Opinions may differ
        Hide
        Michael Bouschen added a comment -

        The patch looks good!

        Show
        Michael Bouschen added a comment - The patch looks good!
        Hide
        Andy Jefferson added a comment -

        Patch applied to SVN trunk. Remaining work is in spec and tests

        Show
        Andy Jefferson added a comment - Patch applied to SVN trunk. Remaining work is in spec and tests
        Hide
        Guido Anzuoni added a comment -

        Just my 2 cents.
        How would be hard to provide the serializeRead feature at PersistenceCapable instance level ?
        There could be a variant in PM.getObjectById with a boolean serializeRead parameter and an explicit
        PM.serializeRead(Object o)

        Show
        Guido Anzuoni added a comment - Just my 2 cents. How would be hard to provide the serializeRead feature at PersistenceCapable instance level ? There could be a variant in PM.getObjectById with a boolean serializeRead parameter and an explicit PM.serializeRead(Object o)
        Hide
        Marco added a comment -

        Adding an optional boolean parameter to pm.getObjectById(...) IMHO has the downside that already quite a few overloaded getObjectById(...) methods exist, where one of them already has a boolean parameter:

        a) getObjectById(java.lang.Class cls, java.lang.Object key)
        b) getObjectById(java.lang.Object oid)
        c) getObjectById(java.lang.Object oid, boolean validate)

        There's the question, to which of these getObjectById-methods do you want to add it? Adding it as last parameter is obviously only possible for (a) and (c) leading to 2 new methods. Alternatively you could add it at as first parameter and then have 3 new methods.

        Is that helpful? Or confusing?

        Any other opinions?

        The PM.serializeRead(Object) is IMHO a really good idea. If I understand you correctly, that's more or less the same as writing:

        myTransaction.setSerializeRead(true);
        pm.refresh(myPersistenceCapable);
        myTransaction.setSerializeRead(null);

        Right? Probably the only difference is that the above code might overwrite any previously set value (that's why we encapsulate the Transaction.setSerializeRead(...) calls and employ reference counting - we're using code like the above quite often).

        When you add pm.serializeRead(Object), you'd need additionally pm.serializeReadAll(Collection) to be somehow symmetric with all other methods (detach/detachAll, refresh/refreshAll etc.) and to give the JDO implementation the ability for optimization.

        Show
        Marco added a comment - Adding an optional boolean parameter to pm.getObjectById(...) IMHO has the downside that already quite a few overloaded getObjectById(...) methods exist, where one of them already has a boolean parameter: a) getObjectById(java.lang.Class cls, java.lang.Object key) b) getObjectById(java.lang.Object oid) c) getObjectById(java.lang.Object oid, boolean validate) There's the question, to which of these getObjectById-methods do you want to add it? Adding it as last parameter is obviously only possible for (a) and (c) leading to 2 new methods. Alternatively you could add it at as first parameter and then have 3 new methods. Is that helpful? Or confusing? Any other opinions? The PM.serializeRead(Object) is IMHO a really good idea. If I understand you correctly, that's more or less the same as writing: myTransaction.setSerializeRead(true); pm.refresh(myPersistenceCapable); myTransaction.setSerializeRead(null); Right? Probably the only difference is that the above code might overwrite any previously set value (that's why we encapsulate the Transaction.setSerializeRead(...) calls and employ reference counting - we're using code like the above quite often). When you add pm.serializeRead(Object), you'd need additionally pm.serializeReadAll(Collection) to be somehow symmetric with all other methods (detach/detachAll, refresh/refreshAll etc.) and to give the JDO implementation the ability for optimization.
        Hide
        Guido Anzuoni added a comment -

        I agree about your concerns on getObjectById, even if it would avoid a backend access.
        The PM.serializeRead(Object) is not exactly the sequence you propose, because the purpose is to lock the object, not to resync with the
        backend
        It could be implemented with a
        SELECT <any column - it doesn't matter since result is discarded> from <table> where <pk condition> FOR UPDATE

        Not sure if pm.serializeReadAll(Collection) is a good thing (what if your connection is in nowait mode and one of the rows is already locked)

        Show
        Guido Anzuoni added a comment - I agree about your concerns on getObjectById, even if it would avoid a backend access. The PM.serializeRead(Object) is not exactly the sequence you propose, because the purpose is to lock the object, not to resync with the backend It could be implemented with a SELECT <any column - it doesn't matter since result is discarded> from <table> where <pk condition> FOR UPDATE Not sure if pm.serializeReadAll(Collection) is a good thing (what if your connection is in nowait mode and one of the rows is already locked)
        Hide
        Craig L Russell added a comment -

        Would this requirement be satisfied by adding a method serializeRead(Object oid)? Then there is no need to go to the datastore twice and no need to modify the getObjectById methods.

        Show
        Craig L Russell added a comment - Would this requirement be satisfied by adding a method serializeRead(Object oid)? Then there is no need to go to the datastore twice and no need to modify the getObjectById methods.
        Hide
        Andy Jefferson added a comment -

        The vast majority of this is already done, embodied in the API, and also in the spec.

        Any further requirements (like the final item - getObjectById etc) could be raised as a separate JIRA and this issue closed.

        Show
        Andy Jefferson added a comment - The vast majority of this is already done, embodied in the API, and also in the spec. Any further requirements (like the final item - getObjectById etc) could be raised as a separate JIRA and this issue closed.
        Hide
        Craig L Russell added a comment -

        Most of these issues have been resolved with the JDO 3.0 release. If any issues remain, please open another JIRA.

        Show
        Craig L Russell added a comment - Most of these issues have been resolved with the JDO 3.0 release. If any issues remain, please open another JIRA.

          People

          • Assignee:
            Craig L Russell
            Reporter:
            Marco
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development