Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: expiration time did not respect environment variable #915 #916

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

fix: expiration time did not respect environment variable #915 #916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b56f518
fix: expiration time did not expect environment #915
Roiocam Aug 22, 2024
fb1e4c7
fix condition mistake
Roiocam Aug 22, 2024
687a0f6
add deprecated and translate comment
Roiocam Aug 22, 2024
fb305d6
rollback method
Roiocam Aug 22, 2024
e226421
remove force argument
Roiocam Sep 4, 2024
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
32 changes: 14 additions & 18 deletions jetcache-core/src/main/java/com/alicp/jetcache/MultiLevelCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,12 @@ public MultiLevelCacheConfig<K, V> config() {

@Override
public CacheResult PUT(K key, V value) {
if (config.isUseExpireOfSubCache()) {
return PUT(key, value, 0, null);
} else {
return PUT(key, value, config().getExpireAfterWriteInMillis(), TimeUnit.MILLISECONDS);
}
return PUT(key, value, config().getExpireAfterWriteInMillis(), TimeUnit.MILLISECONDS);
}

@Override
public CacheResult PUT_ALL(Map<? extends K, ? extends V> map) {
if (config.isUseExpireOfSubCache()) {
return PUT_ALL(map, 0, null);
} else {
return PUT_ALL(map, config().getExpireAfterWriteInMillis(), TimeUnit.MILLISECONDS);
}
return PUT_ALL(map, config().getExpireAfterWriteInMillis(), TimeUnit.MILLISECONDS);
}

@Override
Expand Down Expand Up @@ -109,13 +101,16 @@ private void checkResultAndFillUpperCache(K key, int i, CacheValueHolder<V> h) {
long currentExpire = h.getExpireTime();
long now = System.currentTimeMillis();
if (now <= currentExpire) {
if(config.isUseExpireOfSubCache()){
PUT_caches(i, key, h.getValue(), 0, null);
} else {
long restTtl = currentExpire - now;
if (restTtl > 0) {
PUT_caches(i, key, h.getValue(), restTtl, TimeUnit.MILLISECONDS);
}
/*
There are two situations here from the legacy version:
- if isUseExpireOfSubCache, let subordinate cache choose the expiration time by itself
- if not, choose the remaining time of the current cache as the expiration time
In the current version, the remaining time of the current cache is always chosen as the expiration time,
but a parameter isForce is added to control that it cannot exceed the expiration time that subordinate cache configured
*/
long restTtl = currentExpire - now;
if (restTtl > 0) {
PUT_caches(i, key, h.getValue(), restTtl, TimeUnit.MILLISECONDS);
}
}
}
Expand Down Expand Up @@ -177,7 +172,8 @@ private CacheResult PUT_caches(int lastIndex, K key, V value, long expire, TimeU
if (timeUnit == null) {
r = cache.PUT(key, value);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在这里取min （expire，cache的默认expire）比较好，就不用加isForce参数了

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

那这样的话，如果用户主动 PUT 一个大于配置时间的过期时间，就不行了。这个 PR 我想到一些边界，还没测试

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你说的这个问题存在，怎么又改回去了呢

} else {
r = cache.PUT(key, value, expire, timeUnit);
long useExpireTime = Math.min(expire, cache.config().getExpireAfterWriteInMillis());
r = cache.PUT(key, value, useExpireTime, timeUnit);
}
future = combine(future, r);
}
Expand Down
4 changes: 3 additions & 1 deletion jetcache-core/src/main/java/com/alicp/jetcache/MultiLevelCacheBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ public void setCaches(List<Cache> caches) {
getConfig().setCaches(caches);
}

@Deprecated
public T useExpireOfSubCache(boolean useExpireOfSubCache) {
getConfig().setUseExpireOfSubCache(useExpireOfSubCache);
return self();
}

@Deprecated
public void setUseExpireOfSubCache(boolean useExpireOfSubCache) {
getConfig().setUseExpireOfSubCache(useExpireOfSubCache);
}
Expand All @@ -69,4 +71,4 @@ public void setExpireAfterAccessInMillis(long expireAfterAccessInMillis) {
throw new UnsupportedOperationException("MultiLevelCache do not support expireAfterAccess");
}

}
}
2 changes: 2 additions & 0 deletions jetcache-core/src/main/java/com/alicp/jetcache/MultiLevelCacheConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ public void setCaches(List<Cache<K, V>> caches) {
this.caches = caches;
}

@Deprecated
public boolean isUseExpireOfSubCache() {
return useExpireOfSubCache;
}

@Deprecated
public void setUseExpireOfSubCache(boolean useExpireOfSubCache) {
this.useExpireOfSubCache = useExpireOfSubCache;
}
Expand Down
2 changes: 0 additions & 2 deletions jetcache-core/src/main/java/com/alicp/jetcache/SimpleCacheManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,9 @@ private Cache create(QuickConfig config) {
Cache remote = buildRemote(config);


boolean useExpireOfSubCache = config.getLocalExpire() != null;
cache = MultiLevelCacheBuilder.createMultiLevelCacheBuilder()
.expireAfterWrite(remote.config().getExpireAfterWriteInMillis(), TimeUnit.MILLISECONDS)
.addCache(local, remote)
.useExpireOfSubCache(useExpireOfSubCache)
.cacheNullValue(config.getCacheNullValue() != null ?
config.getCacheNullValue() : DEFAULT_CACHE_NULL_VALUE)
.buildCache();
Expand Down
2 changes: 0 additions & 2 deletions jetcache-test/src/test/java/com/alicp/jetcache/MultiLevelCacheTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ private void doTest(int expireMillis) throws Exception {

private void testUseExpireOfSubCache() throws Exception {
long oldExpire = l1Cache.config().getExpireAfterWriteInMillis();
((MultiLevelCacheConfig<Object, Object>)cache.config()).setUseExpireOfSubCache(true);
l1Cache.config().setExpireAfterWriteInMillis(15);

cache.put("useSubExpire_key", "V1");
Expand All @@ -194,7 +193,6 @@ private void testUseExpireOfSubCache() throws Exception {
Assert.assertNull(l1Cache.get("useSubExpire_key"));
cache.remove("useSubExpire_key");

((MultiLevelCacheConfig<Object, Object>)cache.config()).setUseExpireOfSubCache(false);
l1Cache.config().setExpireAfterAccessInMillis(oldExpire);
}

Expand Down
2 changes: 0 additions & 2 deletions jetcache-test/src/test/java/com/alicp/jetcache/anno/filed/CreateCacheTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,11 @@ public void test() throws Exception {

private void testCacheWithLocalExpire() {
MultiLevelCacheConfig<?,?> config = (MultiLevelCacheConfig) cacheWithLocalExpire_1.config();
Assert.assertTrue(config.isUseExpireOfSubCache());
Assert.assertEquals(2000, config.getExpireAfterWriteInMillis());
Assert.assertEquals(1000, config.getCaches().get(0).config().getExpireAfterWriteInMillis());
Assert.assertEquals(2000, config.getCaches().get(1).config().getExpireAfterWriteInMillis());

config = (MultiLevelCacheConfig) cacheWithLocalExpire_2.config();
Assert.assertFalse(config.isUseExpireOfSubCache());
Assert.assertEquals(2000, config.getExpireAfterWriteInMillis());
Assert.assertEquals(2000, config.getCaches().get(0).config().getExpireAfterWriteInMillis());
Assert.assertEquals(2000, config.getCaches().get(1).config().getExpireAfterWriteInMillis());
Expand Down
Loading