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.

Sign up for GitHub

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

Jump to bottom

Fix slave can't execute publish command #238

Merged
merged 1 commit into from
Apr 30, 2021
Merged

Fix slave can't execute publish command #238

merged 1 commit into from
Apr 30, 2021

Conversation

karelrooted
Copy link
Contributor

No description provided.

@karelrooted
Copy link
Contributor Author

karelrooted commented Apr 30, 2021 •
edited
Loading

do we need a new way to implement pub/sub without rely on wal replication ? the current solution write to db cause wal seq to increase (have some feedback from user that it sometimes cause a fullsync when a redis-sentinel failover is trigged, cause by "publish __sentinel:hello" message in new slave)

@karelrooted karelrooted force-pushed the fix/slave-can-not-execute-publish-cmd branch from ebc4d6d to 14f049f Compare April 30, 2021 09:41
@git-hulk
Copy link
Member

It seems fine that don’t write the WAL when the role was slave. But it would be better to propagate it without writing to WAL.

@karelrooted karelrooted added the bug type bug label Apr 30, 2021
@ShooterIT
Copy link
Member

I feel ok for current solution. But i have a question, why we persist publish messages？

@git-hulk
Copy link
Member

We want to use the WAL to help us propagating the publish command like others.

@karelrooted karelrooted merged commit 7e09284 into apache:unstable Apr 30, 2021
@karelrooted
Copy link
Contributor Author

@Mergifyio backport 1.3

mergify bot pushed a commit that referenced this pull request Apr 30, 2021
@karelrooted
Fix slave can't execute publish command (#238)
e89dbcb 
(cherry picked from commit 7e09284)
@mergify mergify bot mentioned this pull request Apr 30, 2021
Merged
@mergify
Copy link

mergify bot commented Apr 30, 2021

Command backport 1.3: success

Backports have been created

@karelrooted
Copy link
Contributor Author

@Mergifyio backport 2.0

mergify bot pushed a commit that referenced this pull request Apr 30, 2021
@karelrooted
Fix slave can't execute publish command (#238)
a85e1af 
(cherry picked from commit 7e09284)
@mergify mergify bot mentioned this pull request Apr 30, 2021
Merged
@mergify
Copy link

mergify bot commented Apr 30, 2021

Command backport 2.0: success

Backports have been created

karelrooted pushed a commit that referenced this pull request Apr 30, 2021
@mergify
Fix slave can't execute publish command (#238) (#240)
671629b 
(cherry picked from commit 7e09284)
karelrooted pushed a commit that referenced this pull request Apr 30, 2021
@mergify
Fix slave can't execute publish command (#238) (#241)
000b362 
(cherry picked from commit 7e09284)
ShooterIT added a commit that referenced this pull request May 6, 2021
@ShooterIT
Revert "Fix slave can't execute publish command (#238) (#241)"
c0d7ca1 
This reverts commit 000b362.
This was referenced May 18, 2021
Merged
Merged
ShooterIT pushed a commit that referenced this pull request May 18, 2021
@karelrooted @ShooterIT
Fix slave can't execute publish command (#238)
4897bc0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShooterIT ShooterIT ShooterIT approved these changes

@git-hulk git-hulk git-hulk approved these changes

Assignees
No one assigned
Labels
bug type bug release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@karelrooted @git-hulk @ShooterIT