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

Health endpoint fails with Spring Session when redis is down #4560

Closed
sodik82 opened this issue Nov 20, 2015 · 19 comments
Closed

Health endpoint fails with Spring Session when redis is down #4560

sodik82 opened this issue Nov 20, 2015 · 19 comments

Comments

@sodik82
Copy link

sodik82 commented Nov 20, 2015

Health endpoint (in my case /manage/health) fails when I shut down the redis instance with Whitelabel Error Page and exception:

There was an unexpected error (type=Internal Server Error, status=500).
Cannot get Jedis connection; nested exception is redis.clients.jedis.exceptions.JedisConnectionException: Could not get a resource from the pool

When I try to shut down different resources like mongo or postgres, health endpoint correctly report status "DOWN" for them.

Version: Spring IO 2.0.0.RELEASE (Spring boot 1.3.0) - JDK 1.8

@snicoll snicoll self-assigned this Nov 20, 2015
@snicoll snicoll added this to the 1.3.1 milestone Nov 20, 2015
@snicoll
Copy link
Member

snicoll commented Nov 23, 2015

I can't reproduce with a Vanilla setup. Could you please provide more details? (a sample would be better).

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Nov 23, 2015
@sodik82
Copy link
Author

sodik82 commented Nov 23, 2015

You are right. It works fine in "vanilla setup". The missing piece is that I am using Spring Session backed by Redis.

    at org.springframework.session.data.redis.RedisOperationsSessionRepository.getSession(RedisOperationsSessionRepository.java:141) ~[spring-session-1.0.2.RELEASE.jar:na]
    at org.springframework.session.web.http.SessionRepositoryFilter$SessionRepositoryRequestWrapper.getSession(SessionRepositoryFilter.java:276) ~[spring-session-1.0.2.RELEASE.jar:na]
    at org.springframework.web.context.request.ServletRequestAttributes.updateAccessedSessionAttributes(ServletRequestAttributes.java:255) ~[spring-web-4.2.3.RELEASE.jar:4.2.3.RELEASE]
    at org.springframework.web.context.request.AbstractRequestAttributes.requestCompleted(AbstractRequestAttributes.java:48) ~[spring-web-4.2.3.RELEASE.jar:4.2.3.RELEASE]
    at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:106) ~[spring-web-4.2.3.RELEASE.jar:4.2.3.RELEASE]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-4.2.3.RELEASE.jar:4.2.3.RELEASE]

The question is what to do with chicken-egg problem. I would be the best if health endpoint won't be affected by other not working parts. However I understand that Spring Session works as filter and it is hard to do something about that.

Any thoughts?

@snicoll snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Nov 23, 2015
@snicoll snicoll removed this from the 1.3.1 milestone Nov 23, 2015
@snicoll
Copy link
Member

snicoll commented Nov 23, 2015

I hardly see what we could do about it. @rwinch, @wilkinsona any idea?

@sodik82
Copy link
Author

sodik82 commented Nov 23, 2015

One option could be ability to specify "mandatory/optional" URL patterns for Spring Session. Question is what would be the fallback strategy (fallback to anonymous would be fine here but in general?). However such setting could make sense also in different scenarios (web site with public and private (admin) part... I would rather have working public part if my redis fails although I wouldn't be able to manage it via admin)

@wilkinsona
Copy link
Member

The simplest option is to run the actuator on a separate port, for example:

management.port:8081

@wilkinsona
Copy link
Member

The alternative is to take control of the registration of Spring Session's filter and specify the URL patterns to which you want it to be applied. I don't think this is something that Spring Boot can do automatically (there's no way for us to say everything but /health, for example), but given knowledge of the needs of your own application it's pretty straightforward. Just add a @Bean to your configuration the registers the auto-configured Spring Session filter:

@Bean
public FilterRegistrationBean sessionFilterRegistration(
        SessionRepositoryFilter<?> filter) {
    FilterRegistrationBean registration = new FilterRegistrationBean(filter);
    registration.setUrlPatterns(Arrays.asList("/foo/*", "/bar/*"));
    return registration;
}

@sodik82
Copy link
Author

sodik82 commented Nov 25, 2015

Thank you. I can confirm that the change of management port does not lead to "error page". I still have problem how to access sensitive information in secure way (but I believe this is another problem :))

The other alternative seems problematic for me as I can't simply define URL patterns just to exclude /health. Additionally I am not sure how the "securing" of actuator endpoints would work in such case. But it can work for some type of applications.

@rwinch
Copy link
Member

rwinch commented Nov 29, 2015

Since Spring Session is lazy about looking up the session, you can also avoid invoking HttpServletRequest.getSession() for the health endpoints.

@wilkinsona
Copy link
Member

@rwinch Thanks, but none of the health indicators call getSession().

Looking at the stacktrace posted above led me to look at ServletRequestAttributes.updateAccessedSessionAttributes(). It calls getSession even when there are no attributes to update, however it passes in false indicating that it doesn't want a session to be created. Spring Session finds a requested session id (from a cookie created earlier?) so it has to go out to Redis to try to retrieve the existing session.

It looks as if a nice optimisation, from the perspective of this problem at least, would be to change ServletRequestAttributes so that it only calls getSession if its map of updated attributes isn't empty.

@rwinch
Copy link
Member

rwinch commented Nov 30, 2015

@wilkinsona Keep in mind that getSession() might be called by something that is securing the health endpoints (i.e. Spring Security).

@rwinch
Copy link
Member

rwinch commented Dec 1, 2015

@wilkinsona The other thing I'd add is that I'd be surprised if the monitoring of the endpoints actually passes in a session id. Since it is usually a background task, it does not track sessions but rather passes in a credential like a username/password over basic authentication for every request. If it does pass in a session id, then perhaps it should not.

@snicoll snicoll removed their assignment Dec 1, 2015
@wilkinsona
Copy link
Member

Thanks, @rwinch.

@sodik82 The failure will only occur if you send a SESSION cookie as part of the request. I can curl the health endpoint when Redis is down and receive the expected response:

$ curl -i localhost:8080/health
HTTP/1.1 503 Service Unavailable
Server: Apache-Coyote/1.1
X-Application-Context: application
Content-Type: application/json;charset=UTF-8
Transfer-Encoding: chunked
Date: Tue, 01 Dec 2015 09:53:52 GMT
Connection: close

{"status":"DOWN","diskSpace":{"status":"UP","total":499046809600,"free":266940710912,"threshold":10485760},"redis":{"status":"DOWN","error":"org.springframework.data.redis.RedisConnectionFailureException: Cannot get Jedis connection; nested exception is redis.clients.jedis.exceptions.JedisConnectionException: Could not get a resource from the pool"}}

You should correct whatever you are using to monitor the health of your application so that it does not send a SESSION cookie in its GET request. If you want to secure the health endpoint then, as suggested by @rwinch, you should do so using basic authentication so that there's no need for a cookie to be involved.

@sodik82
Copy link
Author

sodik82 commented Dec 1, 2015

ok, thanks. I will have to adapt my security configuration and I won't be able to use it "directly" from my admin interface but such constraint is acceptable for me.

@rwinch
Copy link
Member

rwinch commented Dec 1, 2015

Likely obvious, but keep in mind that you run into similar issues if you
are using basic authentication and the user data store is down.
On Dec 1, 2015 4:01 AM, "Stanislav Miklik" notifications@github.com wrote:

ok, thanks. I will have to adapt my security configuration and I won't be
able to use it "directly" from my admin interface but such constraint is
acceptable for me.


Reply to this email directly or view it on GitHub
#4560 (comment)
.

@snicoll snicoll changed the title Health endpoint fails when redis is down Health endpoint fails with Spring Session when redis is down Dec 24, 2015
@thekosmix
Copy link

thekosmix commented Apr 29, 2019

although it's closed but excluding redis from management did the job for me
management.health.redis.enabled=false

@Alos
Copy link

Alos commented Sep 26, 2019

FYI I still found this problem and I had to disable Redis from the health actuator.

@wilkinsona
Copy link
Member

@Alos That's a different problem. This issue is specific to using Spring Session backed by Redis and making a request to the Actuator with a SESSION cookie in the request. While the description talks about the health endpoint, requests to any endpoint would be affected and disabling the Redis health indicator would not make any difference.

@Venkat2811
Copy link

Venkat2811 commented Oct 20, 2021

I am still facing this issue.

management.health.redis.enabled=false

Will not work because it doesn't prevent this call:

o.s.s.w.h.S.SESSION_LOGGER : No session found by id: Caching result for getSession(false) for this HttpServletRequest.

Our SetUp:

  • Our application partially uses redis.
  • There are flows that doesn't require redis.
  • When one of the node in clustered redis is down, it takes sometime for redis to be fully up.
  • Our health check fails while calling /health endpoint and 2-3 restarts happen.
  • I am okay with endpoints that are using redis to be down while automatic failover happens. What I don't want is /health endpoint failing because of redis

@wilkinsona
Copy link
Member

wilkinsona commented Oct 20, 2021

@Venkat2811 It sounds like you're sending a SESSION cookie in the request. As described above, you should avoid doing so if you do not want that request to be affected by the session store being unavailable. If that doesn't sound applicable to you then you may have a different problem. If you'd like to us to spend some time investigating, please create a minimal sample that reproduces the problem and open a new issue.

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

No branches or pull requests

7 participants