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

Incorrectly appends default memcached port to provided port, thus breaking all memcached backed session functionality #60

Closed
edwardotis opened this issue Apr 13, 2016 · 3 comments

Comments

@edwardotis
Copy link

On Tomcat 8.0.30, oracle jdk 8.77
Using https://github.com/jsimone/webapp-runner with server string for MEMCACHE_SERVERS:
n1:pub-memcache-42.us-east-1-1.1.ec2.garantiadata.com:11733

Results in default port being appended to provided port:
a
Apr 13, 2016 4:52:57 AM de.javakaffee.web.msm.MemcachedSessionService startInternal
INFO: starts initialization... (configured nodes definition n1:pub-memcache-42.us-east-1-1.1.ec2.garantiadata.com:11733:11211, failover nodes null)
Apr 13, 2016 4:52:57 AM org.apache.catalina.core.StandardContext startInternal
SEVERE: The session manager failed to start
org.apache.catalina.LifecycleException: Failed to start component [de.javakaffee.web.msm.MemcachedBackupSessionManager[]]
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:154)
at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5272)
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1408)
at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1398)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IllegalArgumentException: Configured memcachedNodes attribute has wrong format, must match ([\w]+):([^:]+):([\d]+)(?:(?:\s+|,)([\w]+):([^:]+):([\d]+))*
at de.javakaffee.web.msm.MemcachedNodesManager.createFor(MemcachedNodesManager.java:190)
at de.javakaffee.web.msm.MemcachedSessionService.createMemcachedNodesManager(MemcachedSessionService.java:480)
at de.javakaffee.web.msm.MemcachedSessionService.startInternal(MemcachedSessionService.java:423)
at de.javakaffee.web.msm.MemcachedBackupSessionManager.startInternal(MemcachedBackupSessionManager.java:518)
at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
... 8 more

Reference
magro/memcached-session-manager#294

@edwardotis edwardotis changed the title Incorrectly appends default memcached port to server, thus breaking all memcached backed session functionality Incorrectly appends default memcached port to provided port, thus breaking all memcached backed session functionality Apr 13, 2016
@edwardotis
Copy link
Author

Here's the line that is incorrectly adding default port, and thus breaking all possible providers of memcached on heroku, except for the custom supported memcachier.

manager.setMemcachedNodes(System.getenv("MEMCACHE_SERVERS") + ":11211");

https://github.com/jsimone/webapp-runner/blob/df41b59e1bb36755ed614809b6ad6a235465100a/src/main/java/webapp/runner/launch/MemcacheSessionStore.java#L25

In fact, all the code around parsing MEMCACHE_SERVERS is broken. It fails to support multiple memcached servers, and thus breaks the contract with the underlying memached session library.

@jkutner
Copy link
Contributor

jkutner commented Apr 14, 2016

I will gladly accept a PR for that. Looks like it should probably work like the MEMCACHIER_SERVERS env var.

@Malax
Copy link
Member

Malax commented Nov 11, 2022

Hello! Since this is an old issue and many things have changed since it was opened I will close it for now. Feel absolutely free to re-open it if it is still relevant so we can work on it! :)

@Malax Malax closed this as completed Nov 11, 2022
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

3 participants