Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Remove non-transactional mutation from KeyValueStore #500

Merged
merged 1 commit into from
Jan 2, 2019
Merged

Remove non-transactional mutation from KeyValueStore #500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...gasys/pantheon/ethereum/storage/keyvalue/KeyValueStoragePrefixedKeyBlockchainStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public Optional<TransactionLocation> getTransactionLocation(final Hash transacti

@Override
public Updater updater() {
return new Updater(storage.getStartTransaction());
return new Updater(storage.startTransaction());
}

private List<TransactionReceipt> rlpDecodeTransactionReceipts(final BytesValue bytes) {
Expand Down
2 changes: 1 addition & 1 deletion ...ava/tech/pegasys/pantheon/ethereum/storage/keyvalue/KeyValueStorageWorldStateStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public Optional<BytesValue> getAccountStorageTrieNode(final Bytes32 nodeHash) {

@Override
public Updater updater() {
return new Updater(keyValueStorage.getStartTransaction());
return new Updater(keyValueStorage.startTransaction());
}

public static class Updater implements WorldStateStorage.Updater {
Expand Down
2 changes: 1 addition & 1 deletion ethereum/trie/src/main/java/tech/pegasys/pantheon/ethereum/trie/KeyValueMerkleStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void commit() {
// Nothing to do
return;
}
final KeyValueStorage.Transaction kvTx = keyValueStorage.getStartTransaction();
final KeyValueStorage.Transaction kvTx = keyValueStorage.startTransaction();
for (final Map.Entry<Bytes32, BytesValue> entry : pendingUpdates.entrySet()) {
kvTx.put(entry.getKey(), entry.getValue());
}
Expand Down
24 changes: 1 addition & 23 deletions ...kvstore/src/main/java/tech/pegasys/pantheon/services/kvstore/InMemoryKeyValueStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,7 @@ public Optional<BytesValue> get(final BytesValue key) {
}

@Override
public void put(final BytesValue key, final BytesValue value) {
final Lock lock = rwLock.writeLock();
lock.lock();
try {
hashValueStore.put(key, value);
} finally {
lock.unlock();
}
}

@Override
public void remove(final BytesValue key) throws StorageException {
final Lock lock = rwLock.writeLock();
lock.lock();
try {
hashValueStore.remove(key);
} finally {
lock.unlock();
}
}

@Override
public Transaction getStartTransaction() {
public Transaction startTransaction() {
return new InMemoryTransaction();
}

Expand Down
15 changes: 1 addition & 14 deletions services/kvstore/src/main/java/tech/pegasys/pantheon/services/kvstore/KeyValueStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,25 +30,12 @@ public interface KeyValueStorage extends Closeable {
*/
Optional<BytesValue> get(BytesValue key) throws StorageException;

/**
* @param key Index into persistent data repository.
* @param value The value persisted at the key index.
*/
void put(BytesValue key, BytesValue value) throws StorageException;

/**
* Remove the data corresponding to the given key.
*
* @param key Index into persistent data repository.
*/
void remove(BytesValue key) throws StorageException;

/**
* Begins a transaction. Returns a transaction object that can be updated and committed.
*
* @return An object representing the transaction.
*/
Transaction getStartTransaction() throws StorageException;
Transaction startTransaction() throws StorageException;

/**
* Stream all stored key-value pairs.
Expand Down
22 changes: 1 addition & 21 deletions .../kvstore/src/main/java/tech/pegasys/pantheon/services/kvstore/RocksDbKeyValueStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,27 +73,7 @@ public Optional<BytesValue> get(final BytesValue key) throws StorageException {
}

@Override
public void put(final BytesValue key, final BytesValue value) throws StorageException {
throwIfClosed();
try {
db.put(key.extractArray(), value.extractArray());
} catch (final RocksDBException e) {
throw new StorageException(e);
}
}

@Override
public void remove(final BytesValue key) throws StorageException {
throwIfClosed();
try {
db.delete(key.extractArray());
} catch (final RocksDBException e) {
throw new StorageException(e);
}
}

@Override
public Transaction getStartTransaction() throws StorageException {
public Transaction startTransaction() throws StorageException {
throwIfClosed();
final WriteOptions options = new WriteOptions();
return new RocksDbTransaction(db.beginTransaction(options), options);
Expand Down
88 changes: 59 additions & 29 deletions ...ore/src/test/java/tech/pegasys/pantheon/services/kvstore/AbstractKeyValueStorageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ public void twoStoresAreIndependent() throws Exception {
final KeyValueStorage store1 = createStore();
final KeyValueStorage store2 = createStore();

store1.put(BytesValue.fromHexString("0001"), BytesValue.fromHexString("0FFF"));
Transaction tx = store1.startTransaction();
tx.put(BytesValue.fromHexString("0001"), BytesValue.fromHexString("0FFF"));
tx.commit();
final Optional<BytesValue> result = store2.get(BytesValue.fromHexString("0001"));
assertEquals(Optional.empty(), result);
}
Expand All @@ -51,27 +53,47 @@ public void twoStoresAreIndependent() throws Exception {
public void put() throws Exception {
final KeyValueStorage store = createStore();

store.put(BytesValue.fromHexString("0F"), BytesValue.fromHexString("0ABC"));
Transaction tx = store.startTransaction();
tx.put(BytesValue.fromHexString("0F"), BytesValue.fromHexString("0ABC"));
tx.commit();
assertEquals(
Optional.of(BytesValue.fromHexString("0ABC")), store.get(BytesValue.fromHexString("0F")));

store.put(BytesValue.fromHexString("0F"), BytesValue.fromHexString("0DEF"));
tx = store.startTransaction();
tx.put(BytesValue.fromHexString("0F"), BytesValue.fromHexString("0DEF"));
tx.commit();
assertEquals(
Optional.of(BytesValue.fromHexString("0DEF")), store.get(BytesValue.fromHexString("0F")));
}

@Test
public void removeExisting() throws Exception {
final KeyValueStorage store = createStore();
store.put(BytesValue.fromHexString("0F"), BytesValue.fromHexString("0ABC"));
store.remove(BytesValue.fromHexString("0F"));
Transaction tx = store.startTransaction();
tx.put(BytesValue.fromHexString("0F"), BytesValue.fromHexString("0ABC"));
tx.commit();
tx = store.startTransaction();
tx.remove(BytesValue.fromHexString("0F"));
tx.commit();
assertEquals(Optional.empty(), store.get(BytesValue.fromHexString("0F")));
}

@Test
public void removeExistingSameTransaction() throws Exception {
final KeyValueStorage store = createStore();
Transaction tx = store.startTransaction();
tx.put(BytesValue.fromHexString("0F"), BytesValue.fromHexString("0ABC"));
tx.remove(BytesValue.fromHexString("0F"));
tx.commit();
assertEquals(Optional.empty(), store.get(BytesValue.fromHexString("0F")));
}

@Test
public void removeNonExistent() throws Exception {
final KeyValueStorage store = createStore();
store.remove(BytesValue.fromHexString("0F"));
Transaction tx = store.startTransaction();
tx.remove(BytesValue.fromHexString("0F"));
tx.commit();
assertEquals(Optional.empty(), store.get(BytesValue.fromHexString("0F")));
}

Expand All @@ -83,9 +105,11 @@ public void entries() throws Exception {
Arrays.asList(
Entry.create(BytesValue.fromHexString("01"), BytesValue.fromHexString("0ABC")),
Entry.create(BytesValue.fromHexString("02"), BytesValue.fromHexString("0DEF")));
Transaction tx = store.startTransaction();
for (final Entry testEntry : testEntries) {
store.put(testEntry.getKey(), testEntry.getValue());
tx.put(testEntry.getKey(), testEntry.getValue());
}
tx.commit();

final List<Entry> actualEntries = store.entries().collect(Collectors.toList());
testEntries.sort(Comparator.comparing(Entry::getKey));
Expand All @@ -106,7 +130,9 @@ public void concurrentUpdate() throws Exception {
() -> {
try {
for (int i = 0; i < keyCount; i++) {
store.put(BytesValues.toMinimalBytes(i), value);
Transaction tx = store.startTransaction();
tx.put(BytesValues.toMinimalBytes(i), value);
tx.commit();
}
} finally {
finishedLatch.countDown();
Expand Down Expand Up @@ -134,12 +160,14 @@ public void concurrentUpdate() throws Exception {
public void transactionCommit() throws Exception {
final KeyValueStorage store = createStore();
// Add some values
store.put(BytesValue.of(1), BytesValue.of(1));
store.put(BytesValue.of(2), BytesValue.of(2));
store.put(BytesValue.of(3), BytesValue.of(3));
Transaction tx = store.startTransaction();
tx.put(BytesValue.of(1), BytesValue.of(1));
tx.put(BytesValue.of(2), BytesValue.of(2));
tx.put(BytesValue.of(3), BytesValue.of(3));
tx.commit();

// Start transaction that adds, modifies, and removes some values
final Transaction tx = store.getStartTransaction();
tx = store.startTransaction();
tx.put(BytesValue.of(2), BytesValue.of(3));
tx.put(BytesValue.of(2), BytesValue.of(4));
tx.remove(BytesValue.of(3));
Expand All @@ -164,12 +192,14 @@ public void transactionCommit() throws Exception {
public void transactionRollback() throws Exception {
final KeyValueStorage store = createStore();
// Add some values
store.put(BytesValue.of(1), BytesValue.of(1));
store.put(BytesValue.of(2), BytesValue.of(2));
store.put(BytesValue.of(3), BytesValue.of(3));
Transaction tx = store.startTransaction();
tx.put(BytesValue.of(1), BytesValue.of(1));
tx.put(BytesValue.of(2), BytesValue.of(2));
tx.put(BytesValue.of(3), BytesValue.of(3));
tx.commit();

// Start transaction that adds, modifies, and removes some values
final Transaction tx = store.getStartTransaction();
tx = store.startTransaction();
tx.put(BytesValue.of(2), BytesValue.of(3));
tx.put(BytesValue.of(2), BytesValue.of(4));
tx.remove(BytesValue.of(3));
Expand All @@ -193,77 +223,77 @@ public void transactionRollback() throws Exception {
@Test
public void transactionCommitEmpty() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.commit();
}

@Test
public void transactionRollbackEmpty() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.rollback();
}

@Test(expected = IllegalStateException.class)
public void transactionPutAfterCommit() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.commit();
tx.put(BytesValue.of(1), BytesValue.of(1));
}

@Test(expected = IllegalStateException.class)
public void transactionRemoveAfterCommit() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.commit();
tx.remove(BytesValue.of(1));
}

@Test(expected = IllegalStateException.class)
public void transactionPutAfterRollback() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.rollback();
tx.put(BytesValue.of(1), BytesValue.of(1));
}

@Test(expected = IllegalStateException.class)
public void transactionRemoveAfterRollback() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.rollback();
tx.remove(BytesValue.of(1));
}

@Test(expected = IllegalStateException.class)
public void transactionCommitAfterRollback() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.rollback();
tx.commit();
}

@Test(expected = IllegalStateException.class)
public void transactionCommitTwice() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.commit();
tx.commit();
}

@Test(expected = IllegalStateException.class)
public void transactionRollbackAfterCommit() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.commit();
tx.rollback();
}

@Test(expected = IllegalStateException.class)
public void transactionRollbackTwice() throws Exception {
final KeyValueStorage store = createStore();
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
tx.rollback();
tx.rollback();
}
Expand All @@ -272,8 +302,8 @@ public void transactionRollbackTwice() throws Exception {
public void twoTransactions() throws Exception {
final KeyValueStorage store = createStore();

final Transaction tx1 = store.getStartTransaction();
final Transaction tx2 = store.getStartTransaction();
final Transaction tx1 = store.startTransaction();
final Transaction tx2 = store.startTransaction();

tx1.put(BytesValue.of(1), BytesValue.of(1));
tx2.put(BytesValue.of(2), BytesValue.of(2));
Expand All @@ -295,7 +325,7 @@ public void transactionIsolation() throws Exception {
(value) ->
new Thread(
() -> {
final Transaction tx = store.getStartTransaction();
final Transaction tx = store.startTransaction();
for (int i = 0; i < keyCount; i++) {
tx.put(BytesValues.toMinimalBytes(i), value);
}
Expand Down