ossn / fixme_backend

6 stars 17 forks source link

Cache issues and issues count queries #18

Closed Biboswan closed 5 years ago

Biboswan commented 5 years ago

Apologies for the massive delay. Got busy with other stuff Have a look @alexdor I wonder will it really help if we precache the /issues?* results when /issues-count endpoint is hit in a go routine.

I will add the docker configuration as well.

Biboswan commented 5 years ago

How shall we set some redis configuration like maxmemory **MB,maxmemory-policy allkeys-lfu While initializing the cache in the app with redis client itself (don't think a nice idea) or set those parameters in the settings in the production. For development mode are these settings required?

alexdor commented 5 years ago

It's probably better to set them through the config file of redis rather than the client. I don't think that these settings are required for dev, only for production.

Biboswan commented 5 years ago

@alexdor i'm just confused over which way to go. If we use a lua script and scan there then it will be blocking. On the other hand if we implement the scan and unlink right in the go server. Then we shall use ttl(key) to find whether the key was set before the start of deletion process or is a very new key which is valid and shouldn't be deleted.

alexdor commented 5 years ago

I think that it’s ok to delete a key even if it’s a relative new key. And that it’s better to implement it in go so we don’t also have manage a lua script. Otherwise we will introduce unnecessary complexity, what do you think? On 7 Apr 2019, 00.12 +0200, Biboswan Roy notifications@github.com, wrote:

@alexdor i'm just confused over which way to go. If we use a lua script and scan there then it will be blocking. On the other hand if we implement the scan and unlink right in the go server. Then we shall use ttl(key) to find whether the key was set before the start of deletion process or is a very new key which is valid and shouldn't be deleted. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Biboswan commented 5 years ago

Yup keeping things in go.

Biboswan commented 5 years ago

Incorporated the suggestions and and also added docker

Biboswan commented 5 years ago

While running the docker Server should be cache here https://github.com/Biboswan/fixme_backend/blob/cache_layer/cache/cache.go#L14. If this is the best way to go then shall tell so in the Readme

alexdor commented 5 years ago

While running the docker Server should be cache here https://github.com/Biboswan/fixme_backend/blob/cache_layer/cache/cache.go#L14. If this is the best way to go then shall tell so in the Readme

I'm not sure that I understood what you mean here

Biboswan commented 5 years ago

While running the docker Server should be cache here https://github.com/Biboswan/fixme_backend/blob/cache_layer/cache/cache.go#L14. If this is the best way to go then shall tell so in the Readme

I'm not sure that I understood what you mean here

While running the docker CACHE_SERVERvariable in the stated line should be set to valuecache (see docker-compose.yml )for the docker compose to resolve the cache (hostname) to IP address of the cache container.

alexdor commented 5 years ago

Yeah, I'll create a link between Redis and the api and set the env variables inside compose to reflect the location of Redis before I deploy it, no worries : )

Biboswan commented 5 years ago

@alexdor changes were made!

alexdor commented 5 years ago

@Biboswan I've managed to merge your branch into master 🎉

There where a few issues, nothing major though. You shouldn't silently ignore the errors, you should always handle them (there was a crash for example when no page was provided because you didn't check if the string to int conversation had an error)

Once again, good work and thank you for your contribution : )

Biboswan commented 5 years ago

Thanks @alexdor :) Happy to contribute