Skip to content

Redis supports failover through Sentinel #39

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

juntaozhang
Copy link

Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/

@jordansissel
Copy link
Contributor

Can you write the description of the change proposed in the PR? Looks like
the "thanks for contributing" was left unchanged.

On Tuesday, September 6, 2016, Juntao [email protected] wrote:

Thanks for contributing to Logstash! If you haven't already signed our

CLA, here's a handy link: https://www.elastic.co/contributor-agreement/

You can view, comment on, or merge this pull request online at:

#39
Commit Summary

  • Redis supports failover through Sentinel

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#39, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAIC6gH37K1E-yyfGT3Oh0X0nKPSmlx0ks5qnmC6gaJpZM4J2mJj
.

@jstoja
Copy link

jstoja commented Feb 10, 2018

hey @juntaozhang do you want to fix this or I make another PR for this feature?

@jstoja
Copy link

jstoja commented Feb 11, 2018

@jsvd I think this could be a great addition. Do you think that it could be merged, now?

@Evesy
Copy link

Evesy commented Jun 23, 2018

Are there any plans to merge this in, along with logstash-plugins/logstash-input-redis#50 too?

cc @jsvd @jordansissel

@nunofernandes
Copy link

+1

@jstoja
Copy link

jstoja commented Sep 4, 2018

Hello @jsvd, if this PR doesn't seem good to you, don't hesitate to tell us, we can surely fix it!

@crazywen
Copy link

+1024, we want it!

@jstoja
Copy link

jstoja commented Sep 23, 2018

I guess that those logstash plugins are not a priority anymore. So I doubt it'll be merged some day... Saldy...

Redis.new(params)
end # def connect

# A string used to identify a Redis instance in log messages
def identity
if @sentinel_hosts
return "redis-sentinel://#{@password} #{$sentinel_hosts} #{@db} #{@data_type}:#{@key}"
Copy link
Member

Choose a reason for hiding this comment

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

should this be @sentinel_hosts instead?

@@ -92,6 +92,10 @@ class LogStash::Outputs::Redis < LogStash::Outputs::Base
# Zero means to check on every event.
config :congestion_interval, :validate => :number, :default => 1

config :sentinel_hosts, :validate => :array
Copy link
Member

Choose a reason for hiding this comment

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

can you provide documentation for these settings in the docs/index.asciidoc file?

@jsvd jsvd self-assigned this Sep 25, 2018
@jsvd jsvd self-requested a review September 25, 2018 13:26
@L3o-pold
Copy link

L3o-pold commented Jul 2, 2019

Any news on this?

@Paramuj
Copy link

Paramuj commented Jul 17, 2019

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants