diff options
author | Jon Marius Venstad <jvenstad@yahoo-inc.com> | 2018-12-17 11:20:35 +0100 |
---|---|---|
committer | Jon Marius Venstad <jvenstad@yahoo-inc.com> | 2018-12-17 11:20:35 +0100 |
commit | 6bbe56ad28ea4837cbd78760f860a6dc24a19110 (patch) | |
tree | 853d5857fa814278ff642e98076863c3e0111893 | |
parent | 7c0a02a79e3007c1dee4be7a98902dc1f1858023 (diff) |
Increase counter on prepare as well
2 files changed, 16 insertions, 10 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CountingCuratorTransaction.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CountingCuratorTransaction.java index 629ada46852..de803281a87 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CountingCuratorTransaction.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CountingCuratorTransaction.java @@ -12,9 +12,10 @@ import com.yahoo.vespa.curator.transaction.TransactionChanges; * CuratorTransaction wrapper which increments a counter, to signal invalidation of node repository caches. * * This class ensures a CuratorTransaction against the cached data (the node repository data) is - * accompanied by an increment of the data generation counter. This increment must occur <em>after</em> + * accompanied by an increment of the data generation counter. An increment must occur <em>after</em> * the write has completed, successfully or not. It is therefore placed in a {@code finally} block, * wrapping the super class' {@link #commit()}. + * Likewise, {@link #prepare()} is also wrapped with an increment, in case it fails due to an inconsistent cache. * The cache is invalid whenever the generation counter is higher than what the cache contents were read with. * The usual locking for modifications of shared data is then enough to ensure the cache provides a * consistent view of the shared data, with one exception: when incrementing the counter fails. This is @@ -33,8 +34,13 @@ class CountingCuratorTransaction extends CuratorTransaction { @Override public void prepare() { - counter.get(); - super.prepare(); + try { + counter.get(); + super.prepare(); + } + finally { + counter.next(); + } } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseTest.java index 835ac4d0913..149510bdc97 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseTest.java @@ -39,7 +39,7 @@ public class CuratorDatabaseTest { commitCreate("/2", database); commitCreate("/1/1", database); commitCreate("/2/1", database); - assertEquals(4L, (long)curator.counter("/changeCounter").get().get().postValue()); + assertEquals(8L, (long)curator.counter("/changeCounter").get().get().postValue()); List<String> children1Call1 = database.getChildren(Path.fromString("/1")); List<String> children1Call2 = database.getChildren(Path.fromString("/1")); @@ -63,7 +63,7 @@ public class CuratorDatabaseTest { assertArrayEquals(new byte[0], database.getData(Path.fromString("/1")).get()); commitReadingWrite("/1", "hello".getBytes(), database); // Data cached during commit of write transaction. Should be invalid now, and re-read. - assertEquals(2L, (long)curator.counter("/changeCounter").get().get().postValue()); + assertEquals(4L, (long)curator.counter("/changeCounter").get().get().postValue()); assertArrayEquals("hello".getBytes(), database.getData(Path.fromString("/1")).get()); assertEquals(0, database.getChildren(Path.fromString("/1")).size()); @@ -82,7 +82,7 @@ public class CuratorDatabaseTest { commitCreate("/2", database); commitCreate("/1/1", database); commitCreate("/2/1", database); - assertEquals(4L, (long)curator.counter("/changeCounter").get().get().postValue()); + assertEquals(8L, (long)curator.counter("/changeCounter").get().get().postValue()); List<String> children1Call0 = database.getChildren(Path.fromString("/1")); // prime the db; this call returns a different instance List<String> children1Call1 = database.getChildren(Path.fromString("/1")); @@ -104,16 +104,16 @@ public class CuratorDatabaseTest { catch (Exception expected) { // expected because the parent does not exist } - // Counter not increased, since prepare failed. - assertEquals(0L, (long)curator.counter("/changeCounter").get().get().postValue()); + // Counter increased once, since prepare failed. + assertEquals(1L, (long)curator.counter("/changeCounter").get().get().postValue()); try { commitFailing(database); // fail during commit fail("Expected exception"); } catch (Exception expected) { } - // Counter increased, since commit failed. - assertEquals(1L, (long)curator.counter("/changeCounter").get().get().postValue()); + // Counter increased, even though commit failed. + assertEquals(3L, (long)curator.counter("/changeCounter").get().get().postValue()); } private void commitCreate(String path, CuratorDatabase database) { |