Skip to content
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

Multi instances #129

Closed
wants to merge 6 commits into from
Closed

Conversation

spo0nman
Copy link

Fixed the errors travis complained about.

Pankaj Kaushal added 2 commits October 17, 2016 17:47
…le instances and sentinel changes to support multiple instances. To achieve this, a few things were changed.

  Redis:
    - Support for multiple_instances
    - Replication master support for multiple instances
    - Seperated slaveof_port as instances have their own ports
    - redis_binary_path needs to be in parameters
    - configurable redis_binary_path to support multiple
    - Configurable client output buffer limits
  Instances:
    - Each instance has it's own name
    - Instance has it's own init file,log file and conf file
    - Instance has it's own redis_binary_path, this is helpful when yo do ps aux and see instance name in the output
  Replication:
    - Slaveof port is added (instances run on different ports)
    - Check that We should not be making a host a slave of itself
  Sentinel:
    - Redhat needs it's own sentinel init file
    - Configurable sentinel_log_file

  Discovered Masters:
    - Sentinel configuration is tricky, You can bake who the master is in the configuration, but as the infrastructure lives on, the reality of who the sentinel thinks is the master for an instance changes. With the discovered_master flag you can use a simple .erb script (discover_master.erb) to find out who all the sentinel think the master is for a given instance. This allows you to reflect the current master in the sentinel configuration. If none of the sentinels are reachable or if the qourem is not reached, it defaults to the first item of $::redis::sentinel::members

    discovered_masters  is disabled by default and works only for instances right now.

Details:
manifests/params.pp
manifests/init.pp
manifests/config.pp
    - $::redis::params::redis_init_template
      $::redis::params::redis_init_file_mode
      $::redis::redis_binary_path
      $::redis::redis_binary_name
       ps aux on the command line gives a bunch of redis-server processes, unless the name of the binary has the name of the instance, it becomes very confusing to see what process is running which instance. This makes this nice. We need to modify the init file in order to pass the instance name to the process name.

       ps: I don't have suse/bsd/debian to test on. this may be wrong for those platforms.

    - $::redis::params::slaveof_port
      Multiple instances run on different ports and need multiple entries for master/slave, thus, we need to move the slave port to be independent of the slave of host.

    - $::redis::params::client_output_buffer_limit_normal
      $::redis::params::client_output_buffer_limit_slave
      $::redis::params::client_output_buffer_limit_pubsub
      Made the output buffer limits configurable.

    - $::redis::params::instances
      Takes a hash of instances. All the config variables that can be tuned in config can be passed along with each instance. if nothing is passed. the redis defaults from params and config are observed.

    - $::redis::provider
      Allows one to overload the puppet provider in case you use a different one from the default puppet provider

    - $::redis::params::sentinel_init_template
      template/redis-sentinel.init.redhat.erb
      start-stop-daemon is not a CentOS thing, so we need a Redhat/CentOS specific init script.
manifests/sentinel.pp
    - $::redis::params::sentinel_log_file
      Sentinel needs its own log file and should not write to the same one as redis.

    - $::redis::instances
      Sentinel is configured with multiple instances. the name of sentinel master is the name of the instance.

    - $::redis::sentinel::members
      $::redis::params::sentinel_discover_master
      $::redis::params::sentinel_members
      $::redis::params::sentinel_multiple_instances

      See, Feature: Discovered Masters
Pankaj Kaushal added 4 commits October 18, 2016 13:44
…te confusing. this being undef is better, but putting it empty so that existing conf does not break.
… but quite confusing. this being undef is better, but putting it empty so that existing conf does not break."

Doesn't do what was hoped.

This reverts commit 8e1b116.
@bostrowski13
Copy link
Contributor

It'd be very nice to have this multi instance features. Is anybody working on this or any timeframe for getting this into master?

@spo0nman
Copy link
Author

Hi, I worked on this, but i was not able to get a proper test environment running to merge this back. I would have some time again in Feb to give it another try.

@jacobmw
Copy link
Contributor

jacobmw commented Mar 8, 2017

I am trying to use this new feature from your fork but seem to be missing something. I'm not quite sure how to pass the hash expected for the instances parameter. You code says it is expecting a hash, should it be an array of hashes? Could you show me an example of how to use multiple instances? Assuming I can get this to work I may be able to help write some tests.

@spo0nman
Copy link
Author

@jacobmw the issue tracking this has an example: #113

@v6
Copy link

v6 commented Apr 12, 2017

// , Can you rebase this on top of master to resolve the conflicts, or is that something for one of the core contributors to do?

@spo0nman
Copy link
Author

@v6 Some of the tests failed with my branch, I do not have a working test environment to test these.
I will try to rebase this on master and create a new branch, are you are interested in helping me with the travis tests?

@bostrowski13
Copy link
Contributor

is there a place to pull the latest of this feature? is there anything i can do to help move this forward?

@petems
Copy link
Member

petems commented Jul 3, 2017

@bostrowski13 I'm still working on the bulk of the work for multi-instances on one server, I should have some slack time over the weekend where I can help implement this.

Can you give me your requirements on what you're looking for in the feature?

@bostrowski13
Copy link
Contributor

really looking for hosting multiple instances across a redis sentinel cluster. we have an older version of redis which hosts 5 different instances hosted on different ports. most have the same config, but would like the option of shared config options with per-instance config, if necessary.

@petems
Copy link
Member

petems commented Jul 3, 2017

@bostrowski13 is this all on the same machine? Because it's possible to configure multi-redis with sentinel on many machines right now with the module?

@petems
Copy link
Member

petems commented Jul 4, 2017

Closing this in favour of #200, lots of rebasing on this PR and should have an MVP of multi-redis instances on one server.

@spo0nman Thanks for your PR, I ended up taking a lot of the work for my implementation. If there's anything I've missed or comments, feel free to comment on #200! 😄

@petems petems closed this Jul 4, 2017
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.

None yet

5 participants