go-sessions icon indicating copy to clipboard operation
go-sessions copied to clipboard

Flaw in redis backend architecture

Open vividvilla opened this issue 5 years ago • 2 comments

In recent redis backend update it seems like session fields are stored as a individual key value pair where value is a JSON encoded bydefault (default encode/decoder) and are namespaced as sessionid_fieldname.

Methods like Clear uses redis SCAN with count 9999999999 to find all the fields and clear it but this is a bad approach since redis SCAN is a blocking command and in a system with reasonable number of session keys blocking time is considerably high. I have tested this by inserting few million fake session keys in redis and called Clear method which took several seconds to return.

I think the better approach is to use redis hashmap for storing session. Session fields will be individual keys in the map and values are encoded with default encoder. Clear method can just delete the hashmap to clear the session in backend instead of iterating over keys. Performance of GetAll method all will be improved by redis HGETALL command which returns all the keys for given session id.

I can send a PR to address the above issue. Please let me know what you think.

vividvilla avatar Jul 24 '18 12:07 vividvilla

We used that approach in the previous release, I know the benefits and the downsides. You can create a PR and I can review it, maybe you can do it better with redis, I have years to use redis in my own products or on work.

kataras avatar Aug 14 '18 12:08 kataras

Created a pull request (#19) with a fix. Pardon me but I didn't get you in above reply of yours

maybe you can do it better with redis, I have years to use redis in my own products or on work.

vividvilla avatar Aug 16 '18 06:08 vividvilla