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.

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
Changes from 1 commit
Commits
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
Next Next commit
fix: expiration time did not expect environment #915
Roiocam committed Aug 22, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit b56f518b9085a0bbb11e8711ddc5811575d35dd9
44 changes: 22 additions & 22 deletions jetcache-core/src/main/java/com/alicp/jetcache/MultiLevelCache.java
Original file line number Diff line number Diff line change
@@ -15,7 +15,6 @@
* @author huangli
*/
public class MultiLevelCache<K, V> extends AbstractCache<K, V> {

private Cache[] caches;

private MultiLevelCacheConfig<K, V> config;
@@ -28,7 +27,6 @@ public MultiLevelCache(Cache... caches) throws CacheConfigException {
CacheConfig lastConfig = caches[caches.length - 1].config();
config = new MultiLevelCacheConfig<>();
config.setCaches(Arrays.asList(caches));
config.setExpireAfterWriteInMillis(lastConfig.getExpireAfterWriteInMillis());
config.setCacheNullValue(lastConfig.isCacheNullValue());
}

@@ -61,20 +59,14 @@ 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);
}
// let each level cache decide the expiration time
return PUT(key, value, 0, null);
}

@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);
}
// let each level cache decide the expiration time
return PUT_ALL(map, 0, null);
}

@Override
@@ -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);
}
/*
原来这里有两种情况:
- 如果 isUseExpireOfSubCache,那么让 Local 自己选择过期时间
- 如果为 false,那么选择远程的剩余时间作为过期时间
现在的代码,总是选择远程的剩余时间作为过期时间,但是增加了参数 isForce 来控制不能超过配置的本地缓存时间
而对于用户直接 PUT_EXPIRED 则允许此类操作
*/
long restTtl = currentExpire - now;
if (restTtl > 0) {
PUT_caches(i, key, h.getValue(), restTtl, TimeUnit.MILLISECONDS,false);
}
}
}
@@ -151,7 +146,7 @@ protected MultiGetResult<K, V> do_GET_ALL(Set<? extends K> keys) {

@Override
protected CacheResult do_PUT(K key, V value, long expireAfterWrite, TimeUnit timeUnit) {
return PUT_caches(caches.length, key, value, expireAfterWrite, timeUnit);
return PUT_caches(caches.length, key, value, expireAfterWrite, timeUnit, true);
}

@Override
@@ -169,15 +164,20 @@ protected CacheResult do_PUT_ALL(Map<? extends K, ? extends V> map, long expireA
return new CacheResult(future);
}

private CacheResult PUT_caches(int lastIndex, K key, V value, long expire, TimeUnit timeUnit) {
private CacheResult PUT_caches(int lastIndex, K key, V value, long expire, TimeUnit timeUnit, boolean isForce) {
CompletableFuture<ResultData> future = CompletableFuture.completedFuture(null);
for (int i = 0; i < lastIndex; i++) {
Cache cache = caches[i];
CacheResult r;
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);
// 只有 isForce = 用户强制 && expire <= 本地缓存时间 才会使用参数的 expire
if (isForce && expire <= cache.config().getExpireAfterWriteInMillis()) {
r = cache.PUT(key, value, expire, timeUnit);
} else {
r = cache.PUT(key, value);
}
}
future = combine(future, r);
}
Original file line number Diff line number Diff line change
@@ -40,15 +40,6 @@ public void setCaches(List<Cache> caches) {
getConfig().setCaches(caches);
}

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

public void setUseExpireOfSubCache(boolean useExpireOfSubCache) {
getConfig().setUseExpireOfSubCache(useExpireOfSubCache);
}

@Override
public T keyConvertor(Function<Object, Object> keyConvertor) {
throw new UnsupportedOperationException("MultiLevelCache do not need a key convertor");
@@ -69,4 +60,4 @@ public void setExpireAfterAccessInMillis(long expireAfterAccessInMillis) {
throw new UnsupportedOperationException("MultiLevelCache do not support expireAfterAccess");
}

}
}
Original file line number Diff line number Diff line change
@@ -10,7 +10,6 @@
*/
public class MultiLevelCacheConfig<K, V> extends CacheConfig<K, V> {
private List<Cache<K, V>> caches = new ArrayList<>();
private boolean useExpireOfSubCache;

@Override
public MultiLevelCacheConfig clone() {
@@ -28,12 +27,4 @@ public List<Cache<K, V>> getCaches() {
public void setCaches(List<Cache<K, V>> caches) {
this.caches = caches;
}

public boolean isUseExpireOfSubCache() {
return useExpireOfSubCache;
}

public void setUseExpireOfSubCache(boolean useExpireOfSubCache) {
this.useExpireOfSubCache = useExpireOfSubCache;
}
}
Original file line number Diff line number Diff line change
@@ -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();
Original file line number Diff line number Diff line change
@@ -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");
@@ -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);
}

Original file line number Diff line number Diff line change
@@ -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());