Skip to content

Feature key prefixes#56

Open
vallbo wants to merge 7 commits into
Kdyby:masterfrom
vallbo:feature-key-prefixes
Open

Feature key prefixes#56
vallbo wants to merge 7 commits into
Kdyby:masterfrom
vallbo:feature-key-prefixes

Conversation

@vallbo

@vallbo vallbo commented Nov 23, 2015

Copy link
Copy Markdown

Adding possibility to create a keys with prefix, because of multiple instances using one Redis server.

Comment thread .gitignore Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add this to your gitignore_global, not here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

deleted in 27657e1

@fprochazka

Copy link
Copy Markdown
Member

This is what databases are for, no?

@vallbo

vallbo commented Nov 25, 2015

Copy link
Copy Markdown
Author

Yes, it is an option for using redis with multiple instances of one application.
The question about multiple database was also discussed on the StackOverflow (http://stackoverflow.com/questions/16221563/whats-the-point-of-multiple-redis-databases).
At the moment application is using wrapper for this library and working without any problems.
But i think it will be more elegant if option of key prefixes will be directly in this library.

@hrach

hrach commented Nov 25, 2015

Copy link
Copy Markdown

Databases have limited count + its possible to distinguished them only by a number (impossible to use it for simultanous app revisions, etc.)

@vallbo

vallbo commented Dec 14, 2015

Copy link
Copy Markdown
Author

@fprochazka Is there some progress, because our company is waiting for this pull request. Thank you for answer.

@hranicka

Copy link
Copy Markdown

👍 for option of key prefixes.

In the current master there is no way of custom keys specification. Only hard-coded "Nette" prefixes are present via constants (eg. https://github.com/Kdyby/Redis/blob/master/src/Kdyby/Redis/RedisStorage.php#L29).

Comment thread src/Kdyby/Redis/DI/RedisExtension.php Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you should upgrade your PhpStorm, the new one doesn't break the indentation in here :)

@fprochazka

Copy link
Copy Markdown
Member

Also, I'm gonna have to ask you to write tests for this.

@fprochazka

Copy link
Copy Markdown
Member

Also, before you continue. Wouldn't it make more sense to implement this on the client level? Just asking, I know it would be much more complex.

Comment thread docs/en/index.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now that I think about it, someone might misinterpret this as adding the prefix to the client object - it would be a little more writing, but it would also make a lot more sense to me having them under the specific sections.

parameters:
    redis:
        namespace: instance1

redis:
    host: 127.0.0.1
    port: 6379
    journal:
        namespace: %redis.namespace%
    session:
        namespace: %redis.namespace%
    storage:
        namespace: %redis.namespace%
    debugger: off

@fprochazka

Copy link
Copy Markdown
Member

Now that I've reviewed your PR. Let me repeat few important facts

  • I still think increasing the max databases count and having separate database for session and separate database for cache for each application is better then namespacing keys. You cannot call flushdb on deploy when you wanna clear cache, if all your applications are sharing a database. You have to call keys instance1_ and pipe that to del command. Which basically murders redis performance. Using only one database doesn't scale! It's best to have an instance of redis for session and another instance for cache-related data.
  • Redis is single threaded => if your application manages to block the only instance that the other 20 applications are using, you've managed to kill 20 applications. Remember that once you've started session, it blocks the request untill it acquires the key, which it cannot do if the instance is blocked. It can only use one cpu. This can effetively cause multiple waiting requests in PHP - they will queue up untill all the workers are bussy and your webserver will start returning errors. This escalates wery easily.
  • Redis has basically nonexistent memory footprint and running 2 instances per application on one machine consumes very little memory. Read Redis Partitioning they're mostly writing about paritioning one dataset of one application but it's interesting.
  • If your cache and sessions combined from 20+ applications in one database is not a performance problem, or inconvinience for you, you probably don't need redis and filesystem cache is more then enough for you.

@hrach

hrach commented Jan 9, 2016

Copy link
Copy Markdown

How do you solve continuos deployment when you have multiple instances. You can't flush the old db, you have to have another separated storage. Not advocating this solution, just asking :-)

@juniwalk

juniwalk commented Jan 9, 2016

Copy link
Copy Markdown

👎 for me, using single database for multiple apps sounds really bad practise to me.

@fprochazka

Copy link
Copy Markdown
Member

@hrach

Either way, I'd strongly suggest having a separate instance for cache (and another one for sessions) for each application.

@enumag

enumag commented Jan 10, 2016

Copy link
Copy Markdown
Member

I don't use Redis yet because of this very problem - databases are numbered and making sure that each app uses a different database is a real pain. That said I have to agree with @fprochazka. This doesn't seem to be the correct solution to me. 👎

Vladimir Bosiak added 3 commits February 15, 2016 17:30
using namespace for different types of storage
add mock extension for testing (mockery/mockery)
[ERROR]   Kdyby/Redis/DI/RedisExtension.php:54    Mixed tabs and spaces indentation
[ERROR]   KdybyTests/Redis/RedisNamespaceStorage.phpt:29    Mixed tabs and spaces indentation
[ERROR]   KdybyTests/Redis/RedisNamespaceJournal.phpt:28    Mixed tabs and spaces indentation
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