[ONOS-6345] Track tombstones within transactions for optimistic locking on null values
Change-Id: Ib4764721e512462ec1552124ff696b8f89687d8f
diff --git a/core/api/src/main/java/org/onosproject/store/primitives/MapUpdate.java b/core/api/src/main/java/org/onosproject/store/primitives/MapUpdate.java
index e04f10e..f720ded 100644
--- a/core/api/src/main/java/org/onosproject/store/primitives/MapUpdate.java
+++ b/core/api/src/main/java/org/onosproject/store/primitives/MapUpdate.java
@@ -31,7 +31,6 @@
*
* @param <K> map key type
* @param <V> map value type
- *
*/
public final class MapUpdate<K, V> {
@@ -39,47 +38,38 @@
* Type of database update operation.
*/
public enum Type {
- /**
- * Insert/Update entry without any checks.
- */
- PUT,
/**
- * Insert an entry iff there is no existing entry for that key.
+ * Acquires a read lock on a key.
+ * <p>
+ * This record type will check to ensure that the lock version matches the current version for the key
+ * in the map and acquire a read lock on the key for the duration of the transaction.
*/
- PUT_IF_ABSENT,
+ LOCK,
/**
- * Update entry if the current version matches specified version.
+ * Checks the version of a key without locking the key.
+ * <p>
+ * This record type will perform a simple version check during the prepare phase of the two-phase commit
+ * protocol to ensure that the key has not changed during a transaction.
+ */
+ VERSION_MATCH,
+
+ /**
+ * Updates an entry if the current version matches specified version.
*/
PUT_IF_VERSION_MATCH,
/**
- * Update entry if the current value matches specified value.
- */
- PUT_IF_VALUE_MATCH,
-
- /**
- * Remove entry without any checks.
- */
- REMOVE,
-
- /**
- * Remove entry if the current version matches specified version.
+ * Removes an entry if the current version matches specified version.
*/
REMOVE_IF_VERSION_MATCH,
-
- /**
- * Remove entry if the current value matches specified value.
- */
- REMOVE_IF_VALUE_MATCH,
}
private Type type;
private K key;
private V value;
- private V currentValue;
- private long currentVersion = -1;
+ private long version = -1;
/**
* Returns the type of update operation.
@@ -106,19 +96,11 @@
}
/**
- * Returns the expected current value for the key.
- * @return current value in database.
- */
- public V currentValue() {
- return currentValue;
- }
-
- /**
* Returns the expected current version in the database for the key.
* @return expected version.
*/
- public long currentVersion() {
- return currentVersion;
+ public long version() {
+ return version;
}
/**
@@ -135,14 +117,13 @@
.withType(type)
.withKey(keyMapper.apply(key))
.withValue(value == null ? null : valueMapper.apply(value))
- .withCurrentValue(currentValue == null ? null : valueMapper.apply(currentValue))
- .withCurrentVersion(currentVersion)
+ .withVersion(version)
.build();
}
@Override
public int hashCode() {
- return Objects.hash(type, key, value, currentValue, currentVersion);
+ return Objects.hash(type, key, value, version);
}
@Override
@@ -152,8 +133,7 @@
return this.type == that.type
&& Objects.equals(this.key, that.key)
&& Objects.equals(this.value, that.value)
- && Objects.equals(this.currentValue, that.currentValue)
- && Objects.equals(this.currentVersion, that.currentVersion);
+ && Objects.equals(this.version, that.version);
}
return false;
}
@@ -164,8 +144,7 @@
.add("type", type)
.add("key", key)
.add("value", value instanceof byte[] ? new ByteArraySizeHashPrinter((byte[]) value) : value)
- .add("currentValue", currentValue)
- .add("currentVersion", currentVersion)
+ .add("version", version)
.toString();
}
@@ -205,48 +184,38 @@
return this;
}
- public Builder<K, V> withCurrentValue(V value) {
- update.currentValue = value;
- return this;
- }
-
public Builder<K, V> withValue(V value) {
update.value = value;
return this;
}
- public Builder<K, V> withCurrentVersion(long version) {
- update.currentVersion = version;
+ public Builder<K, V> withVersion(long version) {
+ update.version = version;
return this;
}
private void validateInputs() {
checkNotNull(update.type, "type must be specified");
- checkNotNull(update.key, "key must be specified");
switch (update.type) {
- case PUT:
- case PUT_IF_ABSENT:
- checkNotNull(update.value, "value must be specified.");
- break;
- case PUT_IF_VERSION_MATCH:
- checkNotNull(update.value, "value must be specified.");
- checkState(update.currentVersion >= 0, "current version must be specified");
- break;
- case PUT_IF_VALUE_MATCH:
- checkNotNull(update.value, "value must be specified.");
- checkNotNull(update.currentValue, "currentValue must be specified.");
- break;
- case REMOVE:
- break;
- case REMOVE_IF_VERSION_MATCH:
- checkState(update.currentVersion >= 0, "current version must be specified");
- break;
- case REMOVE_IF_VALUE_MATCH:
- checkNotNull(update.currentValue, "currentValue must be specified.");
- break;
- default:
- throw new IllegalStateException("Unknown operation type");
+ case VERSION_MATCH:
+ break;
+ case LOCK:
+ checkNotNull(update.key, "key must be specified");
+ checkState(update.version >= 0, "version must be specified");
+ break;
+ case PUT_IF_VERSION_MATCH:
+ checkNotNull(update.key, "key must be specified");
+ checkNotNull(update.value, "value must be specified.");
+ checkState(update.version >= 0, "version must be specified");
+ break;
+ case REMOVE_IF_VERSION_MATCH:
+ checkNotNull(update.key, "key must be specified");
+ checkState(update.version >= 0, "version must be specified");
+ break;
+ default:
+ throw new IllegalStateException("Unknown operation type");
}
+
}
}
}
diff --git a/core/api/src/main/java/org/onosproject/store/service/TransactionLog.java b/core/api/src/main/java/org/onosproject/store/service/TransactionLog.java
index c367f5d..ab84c3a 100644
--- a/core/api/src/main/java/org/onosproject/store/service/TransactionLog.java
+++ b/core/api/src/main/java/org/onosproject/store/service/TransactionLog.java
@@ -31,10 +31,12 @@
*/
public class TransactionLog<T> {
private final TransactionId transactionId;
+ private final long version;
private final List<T> records;
- public TransactionLog(TransactionId transactionId, List<T> records) {
+ public TransactionLog(TransactionId transactionId, long version, List<T> records) {
this.transactionId = transactionId;
+ this.version = version;
this.records = ImmutableList.copyOf(records);
}
@@ -48,6 +50,15 @@
}
/**
+ * Returns the transaction lock version.
+ *
+ * @return the transaction lock version
+ */
+ public long version() {
+ return version;
+ }
+
+ /**
* Returns the list of transaction log records.
*
* @return a list of transaction log records
@@ -75,6 +86,7 @@
public String toString() {
return MoreObjects.toStringHelper(getClass())
.add("transactionId", transactionId)
+ .add("version", version)
.add("records", records)
.toString();
}
@@ -88,6 +100,6 @@
* @param <U> record type of returned instance
*/
public <U> TransactionLog<U> map(Function<T, U> mapper) {
- return new TransactionLog<>(transactionId, Lists.transform(records, mapper::apply));
+ return new TransactionLog<>(transactionId, version, Lists.transform(records, mapper::apply));
}
}
\ No newline at end of file
diff --git a/core/api/src/test/java/org/onosproject/store/primitives/MapUpdateTest.java b/core/api/src/test/java/org/onosproject/store/primitives/MapUpdateTest.java
index 53cc160..6f16e97 100644
--- a/core/api/src/test/java/org/onosproject/store/primitives/MapUpdateTest.java
+++ b/core/api/src/test/java/org/onosproject/store/primitives/MapUpdateTest.java
@@ -29,49 +29,27 @@
public class MapUpdateTest {
private final MapUpdate<String, byte[]> stats1 = MapUpdate.<String, byte[]>newBuilder()
- .withCurrentValue("1".getBytes())
.withValue("2".getBytes())
- .withCurrentVersion(3)
+ .withVersion(3)
.withKey("4")
- .withType(MapUpdate.Type.PUT)
+ .withType(MapUpdate.Type.PUT_IF_VERSION_MATCH)
.build();
private final MapUpdate<String, byte[]> stats2 = MapUpdate.<String, byte[]>newBuilder()
- .withCurrentValue("1".getBytes())
- .withValue("2".getBytes())
- .withCurrentVersion(3)
- .withKey("4")
- .withType(MapUpdate.Type.REMOVE)
+ .withType(MapUpdate.Type.VERSION_MATCH)
+ .withVersion(10)
.build();
private final MapUpdate<String, byte[]> stats3 = MapUpdate.<String, byte[]>newBuilder()
- .withCurrentValue("1".getBytes())
.withValue("2".getBytes())
- .withCurrentVersion(3)
- .withKey("4")
- .withType(MapUpdate.Type.REMOVE_IF_VALUE_MATCH)
- .build();
-
- private final MapUpdate<String, byte[]> stats4 = MapUpdate.<String, byte[]>newBuilder()
- .withCurrentValue("1".getBytes())
- .withValue("2".getBytes())
- .withCurrentVersion(3)
+ .withVersion(3)
.withKey("4")
.withType(MapUpdate.Type.REMOVE_IF_VERSION_MATCH)
.build();
- private final MapUpdate<String, byte[]> stats5 = MapUpdate.<String, byte[]>newBuilder()
- .withCurrentValue("1".getBytes())
+ private final MapUpdate<String, byte[]> stats4 = MapUpdate.<String, byte[]>newBuilder()
.withValue("2".getBytes())
- .withCurrentVersion(3)
- .withKey("4")
- .withType(MapUpdate.Type.PUT_IF_VALUE_MATCH)
- .build();
-
- private final MapUpdate<String, byte[]> stats6 = MapUpdate.<String, byte[]>newBuilder()
- .withCurrentValue("1".getBytes())
- .withValue("2".getBytes())
- .withCurrentVersion(3)
+ .withVersion(3)
.withKey("4")
.withType(MapUpdate.Type.PUT_IF_VERSION_MATCH)
.build();
@@ -81,11 +59,10 @@
*/
@Test
public void testConstruction() {
- assertThat(stats1.currentValue(), is("1".getBytes()));
assertThat(stats1.value(), is("2".getBytes()));
- assertThat(stats1.currentVersion(), is(3L));
+ assertThat(stats1.version(), is(3L));
assertThat(stats1.key(), is("4"));
- assertThat(stats1.type(), is(MapUpdate.Type.PUT));
+ assertThat(stats1.type(), is(MapUpdate.Type.PUT_IF_VERSION_MATCH));
}
/**
@@ -102,11 +79,6 @@
.addEqualityGroup(stats3, stats3)
.addEqualityGroup(stats4)
.testEquals();
-
- new EqualsTester()
- .addEqualityGroup(stats5, stats5)
- .addEqualityGroup(stats6)
- .testEquals();
}
/**