-
Notifications
You must be signed in to change notification settings - Fork 408
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
Add a Redis Session Store to share session #1096
Comments
@Przem83 internally we are using a Our implementation is limited to PSK (probably one of the reason if is not already provided in Leshan release) I adapted the code to new Californium API but I didn't test it. Maybe it could help you. |
Regarding to issue triggered by @Przem83 we developed PoC of Shared DTLS Session Store based on redission client. Our solution you can find here: (https://github.com/Przem83/leshan) branch opl/session_store/redisson (https://github.com/Przem83/leshan/tree/opl/session_store%2Fredisson))
@sbernard31 What is your opinion about this solution ? |
I try to look at this next week :) |
Some context about my thinking about the leshan-server-redis component : First point, I must confess that I often think about this module and I ask myself if it should really be part of the Leshan scope. I mean the lib must provide a way to let user implement persistent/shared store but I'm not sure we need to provide one implementation in the library itself. I guess there pros :
and cons :
Second point, currently leshan-server-redis depends on californium/scandium but with #1025, I guess we need to go to something with does not depends of transport layer. So I'm not so sure now that a RedisSessionStore should be placed in leshan-server-redis. 🤔 If we want to talk more about this point I can create a dedicated issue, just let me know. About the I don't know so much about redisson, so not sure to get the benefits comparing to jedis. But if we decide to put in in Leshan I guess we should avoid to have 2 Redis client library. Anyway, if this is not integrated in Leshan there is no problem about using Redisson. At first sight the code seems very simple. Maybe it makes more sense to pass a About "Magic serialization/deserializaion", this allow to write less code but this can be annoying to handle changes about data format / backward compatibility. (This can also create silent break) Anyway for a kind of cache this is maybe not an issue, if you know this is OK to sweep all old data when data format changes. About Making Californium class serializable, please ask directly to Californium Maintainer. (I'm out of Cf now) Last point, when we try to implement this we face a situation where most of our clients create a lot of new Session. As only the last one is generally reused we try to remove the oldest one. (see RedisSessionStore.put(), We implement it only for PSK. but maybe this is not an issue for you (or you don't face it ?) |
We think that using Redis server as leshan data repository is very valuable for leshan users. Therefore, starting a discussion is a very good idea. |
After internal disscusion we decided to develop a RedisSessionStore based on shared by you (627136a) code |
Do we have this feature integrated? This is very helpful . |
In #1096 (comment) I said :
More than 1 year later, this finally happens. With new transport layer abstraction design, leshan-server-redis has no more dependency to Californium/Scandium. We could consider to create a new module but currently I really feel this is out of Leshan scope. So I guess this would maybe make more sense to :
@JaroslawLegierski do you have a more recent version of |
We tested session store feature using this branch. But from what I can see the RedisSessionStore version is probably the same |
I will create a simple project with new transport layer abstraction and try this feature. |
Good to know 👍 |
Sure . Thank you . NIDD is over Http , here i want to use DTLS so probably I cannot try the NIDD . |
The idea is to create a RedisSessionStore to share session between several node when leshan is used in a cluster.
This issue was triggered by @Przem83 #1049 (comment)
Not sure we will integrate it but this issue can be used to talk about this.
The text was updated successfully, but these errors were encountered: