Skip to content

Conversation

@etscrivner
Copy link
Contributor

This pull request adds Redis as an optional cache backend.

Explanation:

The organization I work for (ParkMe) currently uses the mapserver with the memcached backend to render parking location information. Recent updates to our infrastructure have led to a roadmap in which we phase out memcached in favor of redis. This is a provisional take at implementing a mapserver cache backend for redis that is production-ready.

* Contains updated CMake file with option to configure using hiredis
  official C redis library.
* Contains update configuration parse and mapcache.h file to add support
  for redis cache.
* Add lib/cache_redis.c file with preliminary redis support.
* In cmake/FindRedis.cmake remove duplicate lines.
* Add an example redis config the XML config sample file.
* Move expiration time before data segment in SETEX call.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mapcache_cache cache; MUST be the first member of the mapcache_cache_redis struct as that's how structure inheritance is performed. I'm actually quite surprised that you got anything working without this...?

@tbonfort
Copy link
Member

tbonfort commented Dec 4, 2014

thanks for the contribution! I currently don't have the time to fully review, test and apply, but I'll get back to you once I can (not before second half of January). Please do continue working on this if needed as this is definitely a cache backend I'd like to see incorporated.

thomas

@etscrivner
Copy link
Contributor Author

@tbonfort Thanks! Modified the code as you specified and am currently in the process of more thorough testing. Just update this pull request when you've done your review and I'll come back to discuss :)

* Use correct error-handling as specified in the strtoul manpages.
@etscrivner
Copy link
Contributor Author

Also, as an update, I've made several fixes and this his now been verified working against the demo.

@sdlime
Copy link
Member

sdlime commented Dec 4, 2014

Looks pretty cool - might give it a whirl here!

@etscrivner
Copy link
Contributor Author

@sdlime Please do! I think there's likely a lot of room for improvement and I'd love any feedback I can get :)

@tbonfort
Copy link
Member

my etscrivner-add-redis-cache branch contains an updated version of this PR with support for the connection pooling that was introduced in branch-1-4, along with some minor fixups. Please give it a try and report back so we can add this to the next release.

@tbonfort
Copy link
Member

@etscrivner any chance you're going to be able to test this?

@etscrivner
Copy link
Contributor Author

@tbonfort I thoroughly tested the previous version (without connection pooling) on our production system. If you'd like I can do the same with the connection-pooled version as I have time? I'd probably get to it next week some time.

@tbonfort
Copy link
Member

@etscrivner that would be great. I'll merge into master once you give the go, no hurry though

@pcallewaert
Copy link

this looks like a great feature, any update on this?

@t4k1t
Copy link

t4k1t commented Mar 16, 2017

I'm interested in this feature as well. Is there anything specific holding it back?

@etscrivner
Copy link
Contributor Author

@pcallewaert @tablet-mode
I haven't worked on this in a couple of years and don't plan to. It looks like there's some pending connection pooling code as well as merge conflicts.

If either of you are willing to fix those and re-test could probably get this off the ground fairly quickly.

Base automatically changed from master to main January 15, 2021 22:57
@jbo-ads
Copy link
Member

jbo-ads commented Mar 10, 2021

@etscrivner, @tbonfort, @sdlime, @pcallewaert, @t4k1t, Anyone willing to work on this?

@t4k1t
Copy link

t4k1t commented May 2, 2021

@etscrivner, @tbonfort, @sdlime, @pcallewaert, @t4k1t, Anyone willing to work on this?

I'm afraid not. I haven't been with the company interested in this for a few years now.

@jbo-ads jbo-ads merged commit 563e91b into MapServer:main Sep 6, 2021
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

Successfully merging this pull request may close these issues.

6 participants