From ce4fecd9d313d61d9468954c675e735d468a4a5e Mon Sep 17 00:00:00 2001 From: "Brian S. O'Neill" Date: Mon, 3 Sep 2007 17:13:02 +0000 Subject: Calling Storable.update when there are no dirty properties actually does an update instead of being "smart" and ignoring the update request. The old behavior was non-intuitive and interfered with expected trigger behavior. --- .../amazon/carbonado/gen/StorableGenerator.java | 46 ------------ .../repo/replicated/ReplicationTrigger.java | 83 +++++++--------------- 2 files changed, 25 insertions(+), 104 deletions(-) (limited to 'src/main/java/com/amazon/carbonado') diff --git a/src/main/java/com/amazon/carbonado/gen/StorableGenerator.java b/src/main/java/com/amazon/carbonado/gen/StorableGenerator.java index ea61306..6a66528 100644 --- a/src/main/java/com/amazon/carbonado/gen/StorableGenerator.java +++ b/src/main/java/com/amazon/carbonado/gen/StorableGenerator.java @@ -1314,52 +1314,6 @@ public final class StorableGenerator { Label tryStart = addGetTriggerAndEnterTxn (b, UPDATE_OP, forTryVar, false, triggerVar, txnVar, stateVar); - // If no properties are dirty, then don't update. - Label doUpdate = b.createLabel(); - branchIfDirty(b, true, doUpdate); - - // Even though there was no update, still need tryLoad side-effect. - { - // if (txn != null) { - // txn.exit(); - // } - b.loadLocal(txnVar); - Label noTxn = b.createLabel(); - b.ifNullBranch(noTxn, true); - b.loadLocal(txnVar); - b.invokeInterface(transactionType, EXIT_METHOD_NAME, null, null); - noTxn.setLocation(); - - Label tryStart2 = b.createLabel().setLocation(); - b.loadThis(); - b.invokeVirtual(DO_TRY_LOAD_METHOD_NAME, TypeDesc.BOOLEAN, null); - - Label notUpdated = b.createLabel(); - b.ifZeroComparisonBranch(notUpdated, "=="); - - // Only mark properties clean if doTryLoad returned true. - b.loadThis(); - b.invokeVirtual(MARK_ALL_PROPERTIES_CLEAN, null, null); - b.loadConstant(true); - b.returnValue(TypeDesc.BOOLEAN); - - notUpdated.setLocation(); - - // Mark properties dirty, to be consistent with a delete side-effect. - b.loadThis(); - b.invokeVirtual(MARK_PROPERTIES_DIRTY, null, null); - b.loadConstant(false); - b.returnValue(TypeDesc.BOOLEAN); - - Label tryEnd = b.createLabel().setLocation(); - b.exceptionHandler(tryStart2, tryEnd, FetchException.class.getName()); - b.invokeVirtual(FetchException.class.getName(), "toPersistException", - TypeDesc.forClass(PersistException.class), null); - b.throwObject(); - } - - doUpdate.setLocation(); - // Call doTryUpdate. b.loadThis(); b.invokeVirtual(DO_TRY_UPDATE_METHOD_NAME, TypeDesc.BOOLEAN, null); diff --git a/src/main/java/com/amazon/carbonado/repo/replicated/ReplicationTrigger.java b/src/main/java/com/amazon/carbonado/repo/replicated/ReplicationTrigger.java index 7b9804d..c3adeed 100644 --- a/src/main/java/com/amazon/carbonado/repo/replicated/ReplicationTrigger.java +++ b/src/main/java/com/amazon/carbonado/repo/replicated/ReplicationTrigger.java @@ -131,69 +131,36 @@ class ReplicationTrigger extends Trigger { private Object beforeUpdate(S replica, boolean forTry) throws PersistException { final S master = mMasterStorage.prepare(); replica.copyPrimaryKeyProperties(master); + replica.copyVersionProperty(master); + replica.copyDirtyProperties(master); - if (!replica.hasDirtyProperties()) { - // Nothing to update, but must load from master anyhow, since - // update must always perform a fresh load as a side-effect. We - // cannot simply call update on the master, since it may need a - // version property to be set. Setting the version has the - // side-effect of making the storable look dirty, so the master - // will perform an update. This in turn causes the version to - // increase for no reason. - try { - if (forTry) { - if (!master.tryLoad()) { - // Master record does not exist. To ensure consistency, - // delete record from replica. - tryDeleteReplica(replica); - throw abortTry(); - } - } else { - try { - master.load(); - } catch (FetchNoneException e) { - // Master record does not exist. To ensure consistency, - // delete record from replica. - tryDeleteReplica(replica); - throw e; - } + try { + if (forTry) { + if (!master.tryUpdate()) { + // Master record does not exist. To ensure consistency, + // delete record from replica. + tryDeleteReplica(replica); + throw abortTry(); } - } catch (FetchException e) { - throw e.toPersistException - ("Could not load master object for update: " + master.toStringKeyOnly()); - } - } else { - replica.copyVersionProperty(master); - replica.copyDirtyProperties(master); - - try { - if (forTry) { - if (!master.tryUpdate()) { - // Master record does not exist. To ensure consistency, - // delete record from replica. - tryDeleteReplica(replica); - throw abortTry(); - } - } else { - try { - master.update(); - } catch (PersistNoneException e) { - // Master record does not exist. To ensure consistency, - // delete record from replica. - tryDeleteReplica(replica); - throw e; - } + } else { + try { + master.update(); + } catch (PersistNoneException e) { + // Master record does not exist. To ensure consistency, + // delete record from replica. + tryDeleteReplica(replica); + throw e; } - } catch (OptimisticLockException e) { - // This may be caused by an inconsistency between replica and - // master. + } + } catch (OptimisticLockException e) { + // This may be caused by an inconsistency between replica and + // master. - repair(replica); + repair(replica); - // Throw original exception since we don't know what the user's - // intentions really are. - throw e; - } + // Throw original exception since we don't know what the user's + // intentions really are. + throw e; } // Copy master properties back, since its repository may have -- cgit v1.2.3