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 expire cache time #6480

Merged
merged 2 commits into from
Mar 26, 2021
Merged

fix expire cache time #6480

merged 2 commits into from
Mar 26, 2021

Conversation

wenbochang888
Copy link

What is the purpose of the change

  • fix expire cache bug
  • Optimize code
public static void main(String[] args) {
	Map<String, String> map = new ConcurrentHashMap<>();
	map.put("cache.seconds", "10");
	map.put("cache.interval", "10");
	URL url = new URL("", "", "", "", 8888, "", map);
	ExpiringCache cache = new ExpiringCache(url);

	long start = System.currentTimeMillis();
	boolean in = false;
	boolean out10 = false;
	boolean out20 = false;
	while (true) {
		long end = System.currentTimeMillis();
		// put k,v in 5s when the programmer start
		if (!in && (end - start) > 5 * 1000) {
			cache.put("aaa", "aaa");
			cache.put("bbb", "bbb");
			log.info("put val");
			in = true;
		}
		// get value after expire time(10s)
		// but I get can get the value where is no null. wrong
		if (!out10 && (end - start) > 16 * 1000) {
			log.info("val = {}", cache.get("aaa"));
			out10 = true;
		}

		// second thread clean。this is null, currect
		if (!out20 && (end - start) > 21 * 1000) {
			log.info("val = {}", cache.get("bbb"));
			out20 = true;
		}
	}
}

Let's look at this piece of code. Set the expiration time to 10s and clear the cache every 10s. At the 5s, we put "aaa" and "bbb" into memory. In theory, after 15s, he should be cleared. When we took the aaa-key in 16s, he was still in memory. And when we took the bbb-key in 21s, he was null. At this time we The bug can be located.

Brief changelog

fix expire cache map

Verifying this change

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a GITHUB_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Run mvn clean install -DskipTests=false & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
  • If this contribution is large, please follow the Software Donation Guide.

@AlbumenJ AlbumenJ added the status/waiting-for-feedback Need reporters to triage label Mar 14, 2021
@wenbochang888
Copy link
Author

我用中文解释一下

dubbo的缓存 https://dubbo.apache.org/zh/docs/v2.7/user/examples/result-cache/ 其中有一个种为:ExpiringCache

它类似于Redis中的设置key的过期时间,比如我设置了10s后过期,那么我在10s就拿不到了

dubbo官方采用的是一个while循环,processExpires方法,每过一段时间检测map里面所有的key是否过期,如果过期 remove

这种是定时删除,但他有个问题如下描述:

我设置过期时间为10s,同时每10s定时删除。

processExpires会在第0s, 10s, 20s, 30s....执行定期删除

我在第5s放入一个key,第10s检测没有过期,此时我在第17s get的时候,理论上是过期了,但此时我get不为null

bug复现,应和Redis一样,不仅采用定时删除,也要采用惰性删除。

即每次get的时候,如果不为空,检测其是否过期。

@AlbumenJ
Copy link
Member

LGTM.

I will merge it later.

@wenbochang888 would you please add some ut test cases for this pr in the future?

@AlbumenJ AlbumenJ merged commit 4d5158f into apache:master Mar 26, 2021
AlbumenJ added a commit to AlbumenJ/dubbo that referenced this pull request May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/waiting-for-feedback Need reporters to triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants