-
Notifications
You must be signed in to change notification settings - Fork 183
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 redis::instance defined type #200
Conversation
e0c496a
to
1175c44
Compare
bb43a12
to
6abb0d4
Compare
@jacobmw @v6 @spo0nman @bostrowski13 Working MVP of two redis instances on one server, running on two different ports: class { '::redis':
default_install => false,
}
redis::instance {'redis1':
port => '7777',
pid_file => '/var/run/redis/redis-server-redis1.pid',
}
redis::instance {'redis2':
port => '8888',
pid_file => '/var/run/redis/redis-server-redis2.pid',
} |
d91b607
to
b5d1d3f
Compare
29a3ee1
to
d10933a
Compare
* Move the config logic into a defined type * This will allow us to have multiple instances of redis on one machine * Example code: ``` # Install redis package class { '::redis': default_install => false, } redis::instance {'redis1': port => '7777', } redis::instance {'redis2': port => '8888', } ```
d10933a
to
161b04a
Compare
this work sfor me. are other params defined within each instance like you specified the pid_file? |
@bostrowski13 Yep, all of the parameters defined are the same that would be the defined the main class. In fact, you no longer need to specify the pid, you can just do: class { '::redis':
default_install => false,
}
redis::instance {'redis1':
port => '7777',
}
redis::instance {'redis2':
port => '8888',
} |
that is excellent. thank you! |
@petems good to know that this code is getting used. In my earlier PR there was a bug where the data directory for redis was shared between the instances, I fixed it in my fork but never published. Look out for this bug. Good luck! |
@spo0nman Ooh, interesting, that's a good point actually. I should probably add a test to make sure each instance uses it's own data folder But yes, your original code was instrumental in getting this working so thank you very much! 😄 |
What about having separate sentinel instances? Is it possible to do this or can sentinel monitor multiple Redis instances without creating a separate sentinel instance for each. |
@tdotgreenshirt From my understanding, you don't want the sentinel instance on the same machine as the redis instance because it would be a SPOF: When you lose that host you lose both. If there is a case for multi-sentinal on one host, then it could be added as a feature? |
@petems This is similar to what we are doing: However, if we have separate Redis instances, we also need a separate Sentinel instances. |
@tdotgreenshirt Ok, can you add that as an issue and I'll tag it as an upcoming feature. Now we have a defined instance type, and redis-sentinel is just another redis instance, this should be fairly quick to implement 😄 |
thats actually very similar as to what we're after as well. be great to have that option for sentinel. It also seems like thats a via model based on their documentation: https://redis.io/topics/sentinel |
Created issue for multiple Sentinel instances: #223 |
* Move the config logic into a defined type * This will allow us to have multiple instances of redis on one machine * Example code: ``` # Install redis package class { '::redis': default_install => false, } redis::instance {'redis1': port => '7777', } redis::instance {'redis2': port => '8888', } ```
This will allow configuring multiple redis instances on one machine, allowing the first step towards #113