Skip to content

Commit 8933e02

Browse files
muchnikchristophstrobl
authored andcommitted
Remove phantom copy of expiring data when source gets persisted.
This commit makes sure to clean up resources when a previously expiring entity is persisted by setting the time to live to zero or negative. In case a phantom copy exists for the changed entity, it is removed to free space on the server and prevent expiration events from being sent. See #1955 Original Pull Request: #1961 # Conflicts: # src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java
1 parent d0e44f5 commit 8933e02

File tree

2 files changed

+56
-3
lines changed

2 files changed

+56
-3
lines changed

src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java

+15-3
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
*
100100
* @author Christoph Strobl
101101
* @author Mark Paluch
102+
* @author Andrey Muchnik
102103
* @since 1.7
103104
*/
104105
public class RedisKeyValueAdapter extends AbstractKeyValueAdapter
@@ -245,6 +246,11 @@ public Object put(Object id, Object item, String keyspace) {
245246
connection.expire(phantomKey, rdo.getTimeToLive() + PHANTOM_KEY_TTL);
246247
}
247248

249+
boolean isNoExpire = rdo.getTimeToLive() == null || rdo.getTimeToLive() != null && rdo.getTimeToLive() < 0;
250+
if (isNoExpire && !isNew && keepShadowCopy()) {
251+
connection.del(ByteUtils.concat(objectKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX));
252+
}
253+
248254
connection.sAdd(toBytes(rdo.getKeyspace()), key);
249255

250256
IndexWriter indexWriter = new IndexWriter(connection, converter);
@@ -487,7 +493,7 @@ public void update(PartialUpdate<?> update) {
487493
} else {
488494

489495
connection.persist(redisKey);
490-
connection.persist(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX));
496+
connection.del(ByteUtils.concat(redisKey, BinaryKeyspaceIdentifier.PHANTOM_SUFFIX));
491497
}
492498
}
493499

@@ -736,6 +742,10 @@ private void initKeyExpirationListener() {
736742
}
737743
}
738744

745+
private boolean keepShadowCopy() {
746+
return this.expirationListener.get() != null;
747+
}
748+
739749
/**
740750
* {@link MessageListener} implementation used to capture Redis keyspace notifications. Tries to read a previously
741751
* created phantom key {@code keyspace:id:phantom} to provide the expired object as part of the published
@@ -777,7 +787,8 @@ public void onMessage(Message message, @Nullable byte[] pattern) {
777787

778788
byte[] key = message.getBody();
779789

780-
byte[] phantomKey = ByteUtils.concat(key, converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class));
790+
byte[] phantomKey = ByteUtils.concat(key,
791+
converter.getConversionService().convert(KeyspaceIdentifier.PHANTOM_SUFFIX, byte[].class));
781792

782793
Map<byte[], byte[]> hash = ops.execute((RedisCallback<Map<byte[], byte[]>>) connection -> {
783794

@@ -794,7 +805,8 @@ public void onMessage(Message message, @Nullable byte[] pattern) {
794805

795806
byte[] channelAsBytes = message.getChannel();
796807
String channel = !ObjectUtils.isEmpty(channelAsBytes)
797-
? converter.getConversionService().convert(channelAsBytes, String.class) : null;
808+
? converter.getConversionService().convert(channelAsBytes, String.class)
809+
: null;
798810

799811
RedisKeyExpiredEvent event = new RedisKeyExpiredEvent(channel, key, value);
800812

src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java

+41
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
/**
6161
* @author Christoph Strobl
6262
* @author Mark Paluch
63+
* @author Andrey Muchnik
6364
*/
6465
@RunWith(Parameterized.class)
6566
public class RedisKeyValueAdapterTests {
@@ -687,6 +688,41 @@ public void updateShouldAlterGeoIndexCorrectlyOnUpdate() {
687688
assertThat(updatedLocation.getY()).isCloseTo(18D, offset(0.005));
688689
}
689690

691+
@Test // DATAREDIS-1955
692+
public void phantomKeyIsDeletedWhenPutWithNegativeTimeToLiveAndOldEntryTimeToLiveWasPositiveAndWhenShadowCopyIsTurnedOn() {
693+
ExpiringPerson rand = new ExpiringPerson();
694+
rand.id = "1";
695+
rand.ttl = 3000L;
696+
697+
adapter.put("1", rand, "persons");
698+
699+
assertThat(template.getExpire("persons:1:phantom")).isPositive();
700+
701+
rand.ttl = -1L;
702+
703+
adapter.put("1", rand, "persons");
704+
705+
assertThat(template.hasKey("persons:1:phantom")).isFalse();
706+
}
707+
708+
@Test // DATAREDIS-1955
709+
public void updateWithRefreshTtlAndWithoutPositiveTtlShouldDeletePhantomKey() {
710+
ExpiringPerson person = new ExpiringPerson();
711+
person.id = "1";
712+
person.ttl = 100L;
713+
714+
adapter.put("1", person, "persons");
715+
716+
assertThat(template.getExpire("persons:1:phantom")).isPositive();
717+
718+
PartialUpdate<ExpiringPerson> update = new PartialUpdate<>("1", ExpiringPerson.class) //
719+
.refreshTtl(true);
720+
721+
adapter.update(update);
722+
723+
assertThat(template.hasKey("persons:1:phantom")).isFalse();
724+
}
725+
690726
/**
691727
* Wait up to 5 seconds until {@code key} is no longer available in Redis.
692728
*
@@ -776,6 +812,11 @@ static class TaVeren extends Person {
776812

777813
}
778814

815+
static class ExpiringPerson extends Person {
816+
817+
@TimeToLive Long ttl;
818+
}
819+
779820
@KeySpace("locations")
780821
static class Location {
781822

0 commit comments

Comments
 (0)