diff options
author | Eike Stepper | 2013-05-29 07:52:26 +0000 |
---|---|---|
committer | Eike Stepper | 2013-05-29 11:38:00 +0000 |
commit | 39ffa4d24715aeb4a36b2118a231fe9605ed02e7 (patch) | |
tree | af0fdee719b0765f77b6fe64d59fd42b1956728b | |
parent | 5ce7219182441088fbcdda476c58d3efd04aec03 (diff) | |
download | cdo-39ffa4d24715aeb4a36b2118a231fe9605ed02e7.tar.gz cdo-39ffa4d24715aeb4a36b2118a231fe9605ed02e7.tar.xz cdo-39ffa4d24715aeb4a36b2118a231fe9605ed02e7.zip |
[396743] [DB] List size column mismatching the row entries
https://bugs.eclipse.org/bugs/show_bug.cgi?id=396743
8 files changed, 128 insertions, 14 deletions
diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java index 6f6a064165..2475856f85 100644 --- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java +++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/internal/common/revision/CDORevisionImpl.java @@ -39,8 +39,7 @@ public class CDORevisionImpl extends BaseCDORevision protected CDORevisionImpl(CDORevisionImpl source) { super(source); - EStructuralFeature[] features = getClassInfo().getAllPersistentFeatures(); - initValues(features); + EStructuralFeature[] features = clearValues(); int length = features.length; for (int i = 0; i < length; i++) diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/BaseCDORevision.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/BaseCDORevision.java index e587c40337..1343941de4 100644 --- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/BaseCDORevision.java +++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/BaseCDORevision.java @@ -725,6 +725,16 @@ public abstract class BaseCDORevision extends AbstractCDORevision } /** + * @since 4.2 + */ + public EStructuralFeature[] clearValues() + { + EStructuralFeature[] features = getClassInfo().getAllPersistentFeatures(); + initValues(features); + return features; + } + + /** * @since 4.1 */ public CDOPermission getPermission() diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/DelegatingCDORevision.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/DelegatingCDORevision.java index f69d786ac4..61c6325f4d 100644 --- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/DelegatingCDORevision.java +++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/DelegatingCDORevision.java @@ -352,6 +352,14 @@ public abstract class DelegatingCDORevision implements InternalCDORevision } /** + * @since 4.2 + */ + public EStructuralFeature[] clearValues() + { + return getDelegate().clearValues(); + } + + /** * @since 4.1 */ public CDOPermission getPermission() diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/InternalCDORevision.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/InternalCDORevision.java index 92c1542140..4f7e4bc341 100644 --- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/InternalCDORevision.java +++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/InternalCDORevision.java @@ -141,6 +141,11 @@ public interface InternalCDORevision extends CDORevision, CDORevisionData, CDORe public InternalCDORevision copy(); /** + * @since 4.2 + */ + public EStructuralFeature[] clearValues(); + + /** * @since 4.1 */ public void setPermission(CDOPermission permission); diff --git a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/StubCDORevision.java b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/StubCDORevision.java index c0a9b669aa..8d864c8216 100644 --- a/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/StubCDORevision.java +++ b/plugins/org.eclipse.emf.cdo.common/src/org/eclipse/emf/cdo/spi/common/revision/StubCDORevision.java @@ -278,6 +278,14 @@ public class StubCDORevision extends AbstractCDORevision } /** + * @since 4.2 + */ + public EStructuralFeature[] clearValues() + { + throw new UnsupportedOperationException(getExceptionMessage()); + } + + /** * @since 4.1 */ public CDOPermission getPermission() diff --git a/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/mapping/horizontal/HorizontalNonAuditClassMapping.java b/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/mapping/horizontal/HorizontalNonAuditClassMapping.java index c2fc7bfd7a..7507749759 100644 --- a/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/mapping/horizontal/HorizontalNonAuditClassMapping.java +++ b/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/mapping/horizontal/HorizontalNonAuditClassMapping.java @@ -15,6 +15,7 @@ package org.eclipse.emf.cdo.server.internal.db.mapping.horizontal; import org.eclipse.emf.cdo.common.branch.CDOBranch; import org.eclipse.emf.cdo.common.branch.CDOBranchPoint; +import org.eclipse.emf.cdo.common.branch.CDOBranchVersion; import org.eclipse.emf.cdo.common.id.CDOID; import org.eclipse.emf.cdo.common.revision.CDOList; import org.eclipse.emf.cdo.common.revision.CDORevision; @@ -35,6 +36,7 @@ import org.eclipse.emf.cdo.server.db.IIDHandler; import org.eclipse.emf.cdo.server.db.mapping.IClassMappingDeltaSupport; import org.eclipse.emf.cdo.server.db.mapping.IListMappingDeltaSupport; import org.eclipse.emf.cdo.server.db.mapping.ITypeMapping; +import org.eclipse.emf.cdo.server.internal.db.DBStore; import org.eclipse.emf.cdo.server.internal.db.bundle.OM; import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevision; import org.eclipse.emf.cdo.spi.common.revision.InternalCDORevisionDelta; @@ -53,6 +55,7 @@ import org.eclipse.net4j.util.om.trace.ContextTracer; import org.eclipse.emf.ecore.EClass; import org.eclipse.emf.ecore.EStructuralFeature; +import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; import java.util.List; @@ -70,6 +73,8 @@ public class HorizontalNonAuditClassMapping extends AbstractHorizontalClassMappi private String sqlSelectCurrentAttributes; + private String sqlSelectCurrentVersion; + private String sqlInsertAttributes; private String sqlUpdateAffix; @@ -80,6 +85,8 @@ public class HorizontalNonAuditClassMapping extends AbstractHorizontalClassMappi private String sqlDelete; + private boolean hasLists; + private ThreadLocal<FeatureDeltaWriter> deltaWriter = new ThreadLocal<FeatureDeltaWriter>() { @Override @@ -93,6 +100,7 @@ public class HorizontalNonAuditClassMapping extends AbstractHorizontalClassMappi { super(mappingStrategy, eClass); initSQLStrings(); + hasLists = !getListMappings().isEmpty(); } private void initSQLStrings() @@ -121,6 +129,17 @@ public class HorizontalNonAuditClassMapping extends AbstractHorizontalClassMappi builder.append("=?"); //$NON-NLS-1$ sqlSelectCurrentAttributes = builder.toString(); + // ----------- Select Version --------------------------- + builder = new StringBuilder(); + builder.append("SELECT "); //$NON-NLS-1$ + builder.append(ATTRIBUTES_VERSION); + builder.append(" FROM "); //$NON-NLS-1$ + builder.append(getTable()); + builder.append(" WHERE "); //$NON-NLS-1$ + builder.append(ATTRIBUTES_ID); + builder.append("=?"); //$NON-NLS-1$ + sqlSelectCurrentVersion = builder.toString(); + // ----------- Insert Attributes ------------------------- builder = new StringBuilder(); builder.append("INSERT INTO "); //$NON-NLS-1$ @@ -336,24 +355,85 @@ public class HorizontalNonAuditClassMapping extends AbstractHorizontalClassMappi throw new UnsupportedOperationException("Mapping strategy does not support audits"); //$NON-NLS-1$ } - IIDHandler idHandler = getMappingStrategy().getStore().getIDHandler(); - IDBPreparedStatement stmt = accessor.getDBConnection().prepareStatement(sqlSelectCurrentAttributes, + CDOID id = revision.getID(); + + DBStore store = (DBStore)getMappingStrategy().getStore(); + IIDHandler idHandler = store.getIDHandler(); + IDBPreparedStatement stmtVersion = null; + IDBPreparedStatement stmtAttributes = accessor.getDBConnection().prepareStatement(sqlSelectCurrentAttributes, ReuseProbability.HIGH); try { - idHandler.setCDOID(stmt, 1, revision.getID()); + if (hasLists) + { + stmtVersion = accessor.getDBConnection().prepareStatement(sqlSelectCurrentVersion, ReuseProbability.HIGH); + stmtVersion.setMaxRows(1); // Optimization: only 1 row + idHandler.setCDOID(stmtVersion, 1, id); + } + + idHandler.setCDOID(stmtAttributes, 1, id); + + for (;;) + { + // Read singleval-attribute table always (even without modeled attributes!) + boolean success = readValuesFromStatement(stmtAttributes, revision, accessor); + + if (hasLists) + { + // Read multival tables only if revision exists + if (success) + { + int currentVersion; + + try + { + readLists(accessor, revision, listChunk); + currentVersion = readVersion(stmtVersion); + } + catch (IndexOutOfBoundsException ex) + { + // A commit has appended list rows after the list size has been read in readValuesFromStatement(). + // Trigger start from scratch below. + currentVersion = CDOBranchVersion.UNSPECIFIED_VERSION; + } + + if (currentVersion != revision.getVersion()) + { + // A commit has changed the revision while reading the lists. Start from scratch! + revision.clearValues(); // Make sure that lists are recreated + continue; + } + } + } + + return success; + } + } + catch (SQLException ex) + { + throw new DBException(ex); + } + finally + { + DBUtil.close(stmtAttributes); + DBUtil.close(stmtVersion); + } + } - // Read singleval-attribute table always (even without modeled attributes!) - boolean success = readValuesFromStatement(stmt, revision, accessor); + private int readVersion(IDBPreparedStatement stmt) + { + ResultSet resultSet = null; - // Read multival tables only if revision exists - if (success) + try + { + resultSet = stmt.executeQuery(); + if (resultSet.next()) { - readLists(accessor, revision, listChunk); + return resultSet.getInt(1); } - return success; + return CDOBranchVersion.UNSPECIFIED_VERSION; } catch (SQLException ex) { @@ -361,7 +441,7 @@ public class HorizontalNonAuditClassMapping extends AbstractHorizontalClassMappi } finally { - DBUtil.close(stmt); + DBUtil.close(resultSet); } } diff --git a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java index d654d79d16..68c178eb31 100644 --- a/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java +++ b/plugins/org.eclipse.emf.cdo.server/src/org/eclipse/emf/cdo/internal/server/TransactionCommitContext.java @@ -893,7 +893,7 @@ public class TransactionCommitContext implements InternalCommitContext { // First lock all objects (incl. possible ref targets). // This is a transient operation, it does not check for existance! - lockManager.lock2(LockType.WRITE, transaction, lockedObjects, 1000); + lockManager.lock2(LockType.WRITE, transaction, lockedObjects, 10000); // If all locks could be acquired, check if locked targets do still exist if (lockedTargets != null) @@ -1114,6 +1114,7 @@ public class TransactionCommitContext implements InternalCommitContext if (oldRevision == null) { + // If the object is logically locked (see lockObjects) but has a wrong (newer) version, someone else modified it throw new ConcurrentModificationException("Attempt by " + transaction + " to modify historical revision: " + delta); } @@ -1162,7 +1163,6 @@ public class TransactionCommitContext implements InternalCommitContext rollbackMessage = message; removePackageAdapters(); - unlockObjects(); if (accessor != null) { @@ -1179,6 +1179,8 @@ public class TransactionCommitContext implements InternalCommitContext repository.failCommit(timeStamp); } } + + unlockObjects(); } } diff --git a/plugins/org.eclipse.emf.cdo.tests.db/src/org/eclipse/emf/cdo/tests/db/Bugzilla_396743_Test.java b/plugins/org.eclipse.emf.cdo.tests.db/src/org/eclipse/emf/cdo/tests/db/Bugzilla_396743_Test.java index 49c116fa82..2c32ccd78d 100644 --- a/plugins/org.eclipse.emf.cdo.tests.db/src/org/eclipse/emf/cdo/tests/db/Bugzilla_396743_Test.java +++ b/plugins/org.eclipse.emf.cdo.tests.db/src/org/eclipse/emf/cdo/tests/db/Bugzilla_396743_Test.java @@ -44,6 +44,8 @@ import java.util.concurrent.TimeUnit; * @author Eike Stepper */ @Skips(IRepositoryConfig.CAPABILITY_AUDITING) +@org.eclipse.emf.cdo.tests.config.impl.ConfigTest.CleanRepositoriesBefore +@org.eclipse.emf.cdo.tests.config.impl.ConfigTest.CleanRepositoriesAfter public class Bugzilla_396743_Test extends AbstractCDOTest { private CountDownLatch readValueLatch; |