This article is participating in the Java Theme Month – Java Debug Notes Event, see the event link for details

preface

Yesterday I wrote a WeChat access token interface super limit problem, then I give the solution is hot to solve the problems, but it seems this solution is a little stupid, maybe I was the only such stupid people will want to come out of method, is not very elegant, user small eyes talk today technology under the article review the code with a lock lock implementation manual lock, After getting the manual lock release mechanism to solve the problem gracefully, I suddenly realized that dishes are original sin. I still have little contact with this multi-threaded programming, but I never thought of using locks to solve it. Once it was pointed out, my eyes suddenly lit up, and FINALLY I planned to change according to this friend’s plan. On the basis of optimization, random number of expiration time should be added when saving each token, so that all tokens cannot expire at the same time. If all tokens expire, there will be problems with the locking scheme. This time, I really realized that Nuggets is really a community that helps developers grow up. I am not in the community to write this bug I have not been able to get this elegant solution, write wrong, write the food does not matter, can grow is the biggest harvest, so I today to write the bug again. Maybe it’s still a bug.

The problem

Suddenly, one day, the scheduled task was not executed, and the project was not executed after the restart. When I was doing online homework, I had a requirement to count the students’ homework of the week at 4:00 p.m. every Sunday, and then I found that the weekly report stayed in a certain week, after which there was no weekly report data statistics.

To analyze problems

Directly open the execution of weekly periodic task code, this code is not written by me, I post the original code. Since multiple nodes are deployed, the distributed lock of Redis is used. If the distributed lock is not used, then each node performs a scheduled task, so the weekly report must be executed repeatedly. The code is as follows:

private static final String LOCK = "task0-job-lock-on"; private static final String KEY = "task0lock-on"; @Scheduled(cron="${data.sync.cron}") public void studentWeeklyReport(){ boolean lock = false; try { lock = redisTemplate.opsForValue().setIfAbsent(KEY, LOCK); if (lock) { System.out.println("start student Weekly Report!" + new Date()); System.out.println("end student Weekly Report! !" + new Date()); } else {logger.info(" No lock, no scheduled task "); } } finally { if (lock) { redisTemplate.delete(KEY); Logger. info(" Mission completed, release lock!" ); } else {logger.info(" No lock, no need to release lock!" ); }}}Copy the code

Description: Data.sync. cron is a CRon expression configured to perform a scheduled task. The logic is simple, Through redisTemplate. OpsForValue (.) setIfAbsent exist (the KEY and LOCK) to determine whether a redis absentValue values for the LOCK, if there is no returns true, and put the current value in the redis, The next call will get false if the current key is not deleted. If true, execute the business code to generate the weekly report, and then check whether the lock exists in finally. If true, delete the key.

After a first look at the code, you can change it to the following code:

@Scheduled(cron = "${data.sync.cron}") public void studentWeeklyReport() { boolean lock = false; try { lock = redisTemplate.opsForValue().setIfAbsent(KEY, LOCK); if (lock) { System.out.println("start student Weekly Report!" + new Date()); System.out.println("end student Weekly Report! !" + new Date()); } } finally { redisTemplate.delete(KEY); Logger. info(" Mission completed, release lock!" ); }}Copy the code

AbsentValue = absentValue = absentValue = absentValue = absentValue = absentValue = absentValue = absentValue = absentValue Then the solution is to add an expiration time, if there is a downtime, the expiration time will automatically expire, then there is no such problem, for the expiration time, do not recommend using the following notation.

@Scheduled(cron = "${data.sync.cron}") public void studentWeeklyReport() { boolean lock = false; try { lock = redisTemplate.opsForValue().setIfAbsent(KEY, LOCK); redisTemplate.expire(KEY, 60, TimeUnit.SECONDS); if (lock) { System.out.println("start student Weekly Report!" + new Date()); System.out.println("end student Weekly Report! !" + new Date()); } } finally { redisTemplate.delete(KEY); Logger. info(" Mission completed, release lock!" ); }}Copy the code

Redistemplate.expire (key, 60, timeunit.seconds) is not an atomic operation. If the service is down in the middle of the setIfAbsent() line, the same is true. Redis perpetuates the value of absentValue. Redis provides a way to set values and expiration times to be atomic. Then modify as follows:

@Scheduled(cron = "${data.sync.cron}") public void studentWeeklyReport() { boolean lock = redisTemplate.opsForValue().setIfAbsent(KEY, LOCK, 1000 * 60, TimeUnit.MILLISECONDS); if (! lock) { return; } try { System.out.println("start student Weekly Report!" + new Date()); System.out.println("end student Weekly Report! !" + new Date()); } catch (Exception e) { e.printStackTrace(); } finally { redisTemplate.delete(KEY); Logger. info(" Mission completed, release lock!" ); }}Copy the code

In this way, the expiration time is set when the value is saved. Even if the service is interrupted, the value in redis will be automatically deleted when the expiration time expires, which does not affect the next scheduled task execution.

Note: Set the timeout period to be longer than the actual time of executing the task. If the timeout period is too small and the previous task is not completed, the redis key automatically expires, and the next task will come in, causing the task to be repeated. So there is still a problem. I don’t know how to set this time properly for the moment. The execution cycle of my business is one week, and the expiration time is easy to predict, but I have no idea to accurately judge whether it will expire.

This project was finally handed over to another project team for maintenance, and I have not been involved in this scheduled task any more. I wonder if there are any other problems with this modification, and welcome to discuss if there is a better solution.

conclusion

If someone else knows the value of my key and my own thread does not delete it, then there is also a problem, which can be solved with the ThreadLocal implementation. The thread that places the key can delete the key. The other thread cannot delete the key. Concrete implementation I will go to study, today’s bug first here. I’ll take a look at ThreadLocal….