Fix race condition which causes layout generation to be lost.

3 files changed, 41 insertions, 12 deletions

diff --git a/src/main/java/com/amazon/carbonado/layout/Layout.java b/src/main/java/com/amazon/carbonado/layout/Layout.java
index 1ea9a36..0c7ab83 100644
--- a/src/main/java/com/amazon/carbonado/layout/Layout.java
+++ b/src/main/java/com/amazon/carbonado/layout/Layout.java
@@ -25,6 +25,7 @@ import java.net.InetAddress;
 import java.net.UnknownHostException;

 

 import java.util.ArrayList;

+import java.util.Arrays;

 import java.util.Collection;

 import java.util.Collections;

 import java.util.List;

@@ -45,6 +46,7 @@ import com.amazon.carbonado.Query;
 import com.amazon.carbonado.RepositoryException;

 import com.amazon.carbonado.Storable;

 import com.amazon.carbonado.SupportException;

+import com.amazon.carbonado.UniqueConstraintException;

 import com.amazon.carbonado.info.StorableInfo;

 import com.amazon.carbonado.info.StorableProperty;

 import com.amazon.carbonado.synthetic.SyntheticKey;

@@ -484,7 +486,8 @@ public class Layout {
             return true;

         }

         return getStorableTypeName().equals(layout.getStorableTypeName())

-            && getAllProperties().equals(layout.getAllProperties());

+            && getAllProperties().equals(layout.getAllProperties())

+            && Arrays.equals(mStoredLayout.getExtraData(), layout.mStoredLayout.getExtraData());

     }

 

     /**

@@ -519,20 +522,32 @@ public class Layout {
         if (mAllProperties == null) {

             throw new IllegalStateException();

         }

+

         mStoredLayout.setGeneration(generation);

-        if (!mStoredLayout.tryInsert()) {

-            StoredLayout existing = mLayoutFactory.mLayoutStorage.prepare();

+

+        try {

+            mStoredLayout.insert();

+        } catch (UniqueConstraintException e) {

+            // If existing record logically matches, update to allow replication.

+            StoredLayout existing = mStoredLayout.prepare();

             mStoredLayout.copyPrimaryKeyProperties(existing);

             try {

                 existing.load();

-            } catch (FetchException e) {

-                throw e.toPersistException();

+            } catch (FetchException e2) {

+                throw e2.toPersistException();

+            }

+            if (existing.getGeneration() != generation ||

+                !existing.getStorableTypeName().equals(getStorableTypeName()) ||

+                !Arrays.equals(existing.getExtraData(), mStoredLayout.getExtraData()))

+            {

+                throw e;

             }

             mStoredLayout.setVersionNumber(existing.getVersionNumber());

             mStoredLayout.update();

         }

+

         for (LayoutProperty property : mAllProperties) {

-            property.store();

+            property.insert();

         }

     }

 }

diff --git a/src/main/java/com/amazon/carbonado/layout/LayoutFactory.java b/src/main/java/com/amazon/carbonado/layout/LayoutFactory.java
index c82e3b4..92099dd 100644
--- a/src/main/java/com/amazon/carbonado/layout/LayoutFactory.java
+++ b/src/main/java/com/amazon/carbonado/layout/LayoutFactory.java
@@ -184,6 +184,12 @@ public class LayoutFactory implements LayoutCapability {
                     // If this point is reached, then type represents a new

                     // generation. Calculate next generation value and insert.

 

+                    // Note: The following query might find a record that

+                    // didn't exist just a moment ago. This will cause a new

+                    // generation value to be calculated, which is incorrect.

+                    // Inserting the layout causes a unique constraint

+                    // exception, which prevents the mistake from persisting.

+

                     assert(newLayout != null);

                     int generation = 0;

 

diff --git a/src/main/java/com/amazon/carbonado/layout/LayoutProperty.java b/src/main/java/com/amazon/carbonado/layout/LayoutProperty.java
index f32cf3f..667e283 100644
--- a/src/main/java/com/amazon/carbonado/layout/LayoutProperty.java
+++ b/src/main/java/com/amazon/carbonado/layout/LayoutProperty.java
@@ -23,6 +23,7 @@ import org.cojen.classfile.TypeDesc;
 import com.amazon.carbonado.FetchException;

 import com.amazon.carbonado.PersistException;

 import com.amazon.carbonado.SupportException;

+import com.amazon.carbonado.UniqueConstraintException;

 

 import com.amazon.carbonado.info.StorableProperty;

 import com.amazon.carbonado.info.StorablePropertyAdapter;

@@ -186,15 +187,22 @@ public class LayoutProperty {
         return mStoredLayoutProperty.toString();

     }

 

-    void store() throws PersistException {

-        if (!mStoredLayoutProperty.tryInsert()) {

-            StoredLayoutProperty existing = mStoredLayoutProperty.copy();

+    void insert() throws PersistException {

+        try {

+            mStoredLayoutProperty.insert();

+        } catch (UniqueConstraintException e) {

+            // If existing record logically matches, update to allow replication.

+            StoredLayoutProperty existing = mStoredLayoutProperty.prepare();

+            mStoredLayoutProperty.copyPrimaryKeyProperties(existing);

             try {

                 existing.load();

-                existing.copyVersionProperty(mStoredLayoutProperty);

-            } catch (FetchException e) {

-                throw e.toPersistException();

+            } catch (FetchException e2) {

+                throw e2.toPersistException();

             }

+            if (!equals(new LayoutProperty(existing))) {

+                throw e;

+            }

+            mStoredLayoutProperty.setVersionNumber(existing.getVersionNumber());

             mStoredLayoutProperty.update();

         }

     }