Skip to content

Support sharded pubsub commands #2887

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

Merged
merged 12 commits into from
Jun 10, 2025
Merged

Support sharded pubsub commands #2887

merged 12 commits into from
Jun 10, 2025

Conversation

mgravell
Copy link
Collaborator

Continuation of #2498

vandyvilla and others added 3 commits May 20, 2025 10:19
* Support sharded pubsub

* Support sharded pubsub

* fix api

* fix enum

* fix api

---------

Co-authored-by: Marc Gravell <[email protected]>
* Support sharded pubsub

* Support sharded pubsub

* fix api

* fix enum

* fix api

* proposed refactor to single-field implementation

* RedisDatabase needs to use SPUBLISH/PUBLISH correctly

* support ssub on CLIENT response

* add to features; fixup shipped etc

---------

Co-authored-by: xli <[email protected]>
@atakavci
Copy link
Contributor

hi @mgravell ,
i think there are issues around reconnects. And the other fails we see here are more likely to be flaky tests. let me know if am misinterpreting it.
Here i attempted to fix and test the reconnects with sharded pub/sub. Let me know if you would like me to change direction.
i am also planning to check with hashslot changes in another iteration, any initial suggestions/hints for that?

@mgravell
Copy link
Collaborator Author

important TODOs before final merge:

  • investigate break/reconnect logic
  • investigate cluster shard migration logic
  • investigate unsubscribe-all logic

@atakavci
Copy link
Contributor

atakavci commented Jun 4, 2025

hey @mgravell , i think this branch fixes and tests reconnect logic as well as testing for hashslot migrations.

the only item remaining is unsubscribe-all logic; what should be the expected behaviour with unsubscribe-all??

  • investigate break/reconnect logic
  • investigate cluster shard migration logic
  • investigate unsubscribe-all logic


private void MigrateSlotForTestShardChannel(bool rollback)
{
int hashSlotForTestShardChannel = 7177;
Copy link
Collaborator

Choose a reason for hiding this comment

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

^ This needs to be separated per test suite

@mgravell mgravell merged commit ad5f656 into main Jun 10, 2025
3 of 8 checks passed
@mgravell mgravell deleted the ssubscribe branch June 10, 2025 16:06
@baracoder
Copy link

Hey,

This PR has changed the channel type passed to the action in SubscribeAsync. That was probably unintentional, right? I am not sure yet if it has relevance outside of a unit test that turned red.

using StackExchange.Redis;

await using var multiplexer = ConnectionMultiplexer.Connect("localhost:6379");
var database = multiplexer.GetDatabase();
var subscriber = database.Multiplexer.GetSubscriber();

var channel1 = RedisChannel.Literal("abc:123");
await subscriber.SubscribeAsync(
    RedisChannel.Pattern("abc:*"),
    (redisChannel, redisValue) =>
    {
        // 2.8.37: true, 2.8.41: false - .IsPattern differs 
        var areEqual = redisChannel == channel1; 
        Console.WriteLine(areEqual);
    });
await database.PublishAsync(channel1, "xyz", CommandFlags.FireAndForget);

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.

5 participants