From 2bd0f7816876c0e937a89efe67a1ecea2db4e33d Mon Sep 17 00:00:00 2001 From: Muchnik Andrey Date: Mon, 8 Feb 2021 21:18:54 +0300 Subject: [PATCH 1/3] DATAREDIS-1955 - Phantom copy is not persisted when entity ttl is changed from positive to negative, causing phantom copy not updated to no expire and counting down, then RedisKeyExpiredEvent publishing When updating an object that has a positive time to live to a negative time to live you must persist the phantom key. Otherwise, after expiring time to live of the phantom, which was created when object with a positive time to live was saved - the indices are dropped and the keys are deleted from the sets as if entity is expired. --- pom.xml | 2 +- .../data/redis/core/RedisKeyValueAdapter.java | 5 +++++ .../redis/core/RedisKeyValueAdapterTests.java | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a5d3a47ec6..8b64370059 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 2.5.0-SNAPSHOT + 2.5.0-DATAREDIS-1955-SNAPSHOT Spring Data Redis diff --git a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java index 832231c667..bf457de664 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java @@ -248,6 +248,11 @@ public Object put(Object id, Object item, String keyspace) { } } + boolean isNoExpire = rdo.getTimeToLive() == null || rdo.getTimeToLive() != null && rdo.getTimeToLive() < 0; + if (isNoExpire && !isNew && keepShadowCopy()){ + connection.persist(ByteUtils.concat(objectKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX)); + } + connection.sAdd(toBytes(rdo.getKeyspace()), key); IndexWriter indexWriter = new IndexWriter(connection, converter); diff --git a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java index 48330cd237..ae6c80e810 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java @@ -705,6 +705,22 @@ void phantomKeyInsertedOnPutWhenShadowCopyIsInDefaultAndKeyspaceNotificationEnab assertThat(template.hasKey("persons:1:phantom")).isTrue(); } + @Test // DATAREDIS-1955 + void phantomKeyNotPersistedWhenPutWithNegativeTimeToLiveAndOldEntryTimeToLiveWasPositiveAndWhenShadowCopyIsTurnedOn() { + ExpiringPerson rand = new ExpiringPerson(); + rand.ttl = 3000L; + + adapter.put("1", rand, "persons"); + + assertThat(template.getExpire("persons:1:phantom")).isPositive(); + + rand.ttl = -1L; + + adapter.put("1", rand, "persons"); + + assertThat(template.getExpire("persons:1:phantom")).isEqualTo(-1L); + } + /** * Wait up to 5 seconds until {@code key} is no longer available in Redis. * From d6de0177a446894e0f005123a83a4b1a92e4df62 Mon Sep 17 00:00:00 2001 From: Muchnik Andrey Date: Mon, 8 Feb 2021 21:48:23 +0300 Subject: [PATCH 2/3] Added myself as author to touched classes. --- .../springframework/data/redis/core/RedisKeyValueAdapter.java | 1 + .../data/redis/core/RedisKeyValueAdapterTests.java | 1 + 2 files changed, 2 insertions(+) diff --git a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java index bf457de664..e971331069 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java @@ -99,6 +99,7 @@ * * @author Christoph Strobl * @author Mark Paluch + * @author Andrey Muchnik * @since 1.7 */ public class RedisKeyValueAdapter extends AbstractKeyValueAdapter diff --git a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java index ae6c80e810..c774b2fd31 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java @@ -54,6 +54,7 @@ * * @author Christoph Strobl * @author Mark Paluch + * @author Andrey Muchnik */ @ExtendWith(LettuceConnectionFactoryExtension.class) public class RedisKeyValueAdapterTests { From b7853f2c9b70ccf7bcb7e135f3d64cd610a76a17 Mon Sep 17 00:00:00 2001 From: Muchnik Andrey Date: Wed, 10 Feb 2021 22:06:42 +0300 Subject: [PATCH 3/3] In order to reduce the footprint on server side in case of update and partial update, when phantom key is not needed - it now deletes instead of persisting. --- .../data/redis/core/RedisKeyValueAdapter.java | 4 ++-- .../redis/core/RedisKeyValueAdapterTests.java | 23 +++++++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java index e971331069..28d9c8e204 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java @@ -251,7 +251,7 @@ public Object put(Object id, Object item, String keyspace) { boolean isNoExpire = rdo.getTimeToLive() == null || rdo.getTimeToLive() != null && rdo.getTimeToLive() < 0; if (isNoExpire && !isNew && keepShadowCopy()){ - connection.persist(ByteUtils.concat(objectKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX)); + connection.del(ByteUtils.concat(objectKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX)); } connection.sAdd(toBytes(rdo.getKeyspace()), key); @@ -498,7 +498,7 @@ public void update(PartialUpdate update) { } else { connection.persist(redisKey); - connection.persist(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX)); + connection.del(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX)); } } diff --git a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java index c774b2fd31..60903098b6 100644 --- a/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java +++ b/src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java @@ -707,8 +707,9 @@ void phantomKeyInsertedOnPutWhenShadowCopyIsInDefaultAndKeyspaceNotificationEnab } @Test // DATAREDIS-1955 - void phantomKeyNotPersistedWhenPutWithNegativeTimeToLiveAndOldEntryTimeToLiveWasPositiveAndWhenShadowCopyIsTurnedOn() { + void phantomKeyIsDeletedWhenPutWithNegativeTimeToLiveAndOldEntryTimeToLiveWasPositiveAndWhenShadowCopyIsTurnedOn() { ExpiringPerson rand = new ExpiringPerson(); + rand.id = "1"; rand.ttl = 3000L; adapter.put("1", rand, "persons"); @@ -719,7 +720,25 @@ void phantomKeyNotPersistedWhenPutWithNegativeTimeToLiveAndOldEntryTimeToLiveWas adapter.put("1", rand, "persons"); - assertThat(template.getExpire("persons:1:phantom")).isEqualTo(-1L); + assertThat(template.hasKey("persons:1:phantom")).isFalse(); + } + + @Test // DATAREDIS-1955 + void updateWithRefreshTtlAndWithoutPositiveTtlShouldDeletePhantomKey() { + ExpiringPerson person = new ExpiringPerson(); + person.id = "1"; + person.ttl = 100L; + + adapter.put("1", person, "persons"); + + assertThat(template.getExpire("persons:1:phantom")).isPositive(); + + PartialUpdate update = new PartialUpdate<>("1", ExpiringPerson.class) // + .refreshTtl(true); + + adapter.update(update); + + assertThat(template.hasKey("persons:1:phantom")).isFalse(); } /**