Skip to content

@EnableRedisWebSession, some sessions do not set a timeout when they are stored in redis, but never expire. #2464

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

Closed
SheldonLi666 opened this issue Oct 10, 2023 · 7 comments
Assignees
Labels
Milestone

Comments

@SheldonLi666
Copy link

My project uses the spring webflux framework and uses spring-session-data-redis to store sessions.

[project spring version]

Spring-session-core:3.0.0
Spring-session-data-redis:3.0.0
Spring-webflux:6.0.5

[question]
After using @EnableRedisWebSession to enable the use of spring session framework. Checking redis will find that some hashkeys starting with spring:session:sessions: will never expire, that is, ttl = -1. What's going on?

I tried to find the source code and debug it, and found that in the class org.springframework.session.data.redis.ReactiveRedisSessionRepository, when the save() method was called for the first time, the internal saveDelta() executed hmset and expire separately. code show as below:

    private Mono<Void> saveDelta() {
		if (this.delta.isEmpty()) {
			return Mono.empty();
		}

		String sessionKey = getSessionKey(getId());
		Mono<Boolean> update = ReactiveRedisSessionRepository.this.sessionRedisOperations.opsForHash()
				.putAll(sessionKey, new HashMap<>(this.delta));
		Mono<Boolean> setTtl;
		if (getMaxInactiveInterval().getSeconds() >= 0) {
			setTtl = ReactiveRedisSessionRepository.this.sessionRedisOperations.expire(sessionKey,
					getMaxInactiveInterval());
		}
		else {
			setTtl = ReactiveRedisSessionRepository.this.sessionRedisOperations.persist(sessionKey);
		}

		return update.and(setTtl).and((s) -> {
			this.delta.clear();
			s.onComplete();
		}).then();
	}

You can see that the two monos update.and(setTtl) are executed like this. Could it be that the order is out of order for this reason, causing expire to be executed first and then hmset?

Observing the redis log should be able to prove my guess:
The following is one of the reasons why the key never expires that I found by observing the monitor log of redis:

1696907574.685407 [0 20.205.170.7:36946] "EXPIRE" "spring:session:sessions:e9f5ca03-f6a3-468e-9f79-58c9ff9b2837" "1800"
1696907574.685445 [0 20.205.169.250:27842] "HMSET" "spring:session:sessions:e9f5ca03-f6a3-468e-9f79-58c9ff9b2837" "lastAccessedTime" "1696907574320" "creationTime" "1696907574320" "maxInactiveInterval" "1800"

Finally, execute to check this key, it will indeed never expire:
image

@SheldonLi666 SheldonLi666 added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Oct 10, 2023
@marcusdacoregio marcusdacoregio self-assigned this Oct 10, 2023
@marcusdacoregio marcusdacoregio removed the status: waiting-for-triage An issue we've not yet triaged label Oct 10, 2023
@marcusdacoregio
Copy link
Contributor

Hi, @SheldonLi666. It is not clear to me why that is happening yet. Is it possible that you set the maxInactiveIntervalInSeconds property on the annotation?

Can you create a minimal, reproducible sample where we can debug it on our side?

@SheldonLi666
Copy link
Author

Hi @marcusdacoregio.
I'm glad to see your reply. This problem has troubled me for a long time, causing the online redis library to continuously accumulate these never-expired sessions.

First of all, I did not set maxInactiveIntervalInSeconds. I used the default expiration time of 1800s, and most sessions successfully set this expiration time in redis.

Excluding project business, my project only uses spring webflux, spring security and spring session, and there are health checks in the online and test environments that request my project interface every 10s to determine connectivity. Nothing else.

In addition, this never-expired session will only appear when it is first created. If the session is not accessed subsequently, there will be no problem of no expiration time.

I think this problem occurs in saveDelta() under org.springframework.session.data.redis.ReactiveRedisSessionRepository, which combines two Monos, Mono update and Mono setTtl, into one Mono through and connection, resulting in There is a certain probability that setTtl will be executed before update. Maybe flapMap should be used here to control the order of Mono update and Mono setTtl instead of and? Looking forward for your reply, thank you!

image

@marcusdacoregio
Copy link
Contributor

Hi, @SheldonLi666. Can you put together a sample that demonstrates that Mono#and can be problematic? In other words, a sample that shows it doesn't guarantee the order of the execution.

@marcusdacoregio marcusdacoregio added the status: waiting-for-feedback We need additional information before we can continue label Oct 11, 2023
@SheldonLi666
Copy link
Author

Hi, @marcusdacoregio. I'm glad to see your reply.
The following is my test code written in kotlin. There are 4 Monos. mono1(1 millisecond output delay) and mono2 use Mono#and and call it. mono3(1 millisecond output delay) and mono4 use Mono#flatmap and call it.
The result is that the output of mono2 is in front of mono1 (mono1 should be output in front of mono2). But the output of mono3 and mono4 is in normal order
Looking forward for your reply, thank you!

import org.junit.jupiter.api.Test
import org.springframework.boot.test.context.SpringBootTest
import reactor.core.publisher.Mono
import reactor.core.scheduler.Schedulers


@SpringBootTest
class TestMono {

    @Test
    fun test() {
        val mono1 = Mono.delay(java.time.Duration.ofMillis(1)).doOnNext { println("mono1") }
        val mono2 = Mono.fromRunnable<Void> { println("mono2") }
        mono1.and(mono2).subscribeOn(Schedulers.boundedElastic()).subscribe()

        val mono3 = Mono.delay(java.time.Duration.ofMillis(1)).doOnNext { println("mono3") }
        val mono4 = Mono.fromRunnable<Void> { println("mono4") }
        mono3.flatMap { mono4 }.subscribeOn(Schedulers.boundedElastic()).subscribe()
    }

}

image

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 12, 2023
@Annaseron
Copy link

@SheldonLi666 before the Contributors fixed the bug,you can add filter to deal with this problem,in the filter you can create the hash with redis api manually before the spring websessionmanager creates it and set TTL for it;

alternatively,you can create scheduled tasks to scan the keys with specific prefix,and set a expiration for it

@SheldonLi666
Copy link
Author

Hi, @Annaseron
I'm very happy to receive your reply. I was planning to use the second method you mentioned to deal with it! I will try to use the first method for now.
May I ask which version will this bug be fixed in?

@marcusdacoregio
Copy link
Contributor

Thanks everyone for your help on this. This is now fixed in 2.7.x, 3.0.x, 3.1.x, and main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants