Skip to content

Conversation

@zmj
Copy link
Contributor

@zmj zmj commented Aug 15, 2020

This PR implements the ROLE command (#1451): https://redis.io/commands/role#sentinel-output . Redis implementation: https://github.com/redis/redis/blob/unstable/src/replication.c#L2672

Because the result format is pretty different for each instance type, we return a base Role type that the caller has to type check to get to the values specific to the instance type.

I've tested the master and replica cases locally. I haven't tested sentinel, but that one is the most straightforward.

/// <param name="server">The server to simulate failure on.</param>
public static void SimulateConnectionFailure(this IServer server) => (server as RedisServer)?.SimulateConnectionFailure();

public static string Role(this IServer server)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the existing test helper and changed the usage to use the new code

/// <summary>
/// This replica's replication state.
/// </summary>
public string State { get; }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be an enum, but returning the raw value seemed better for allowing the caller to do something useful with an unrecognized value.

public void ReplicaRole()
{
var connString = $"{TestConfig.Current.ReplicaServerAndPort},allowAdmin=true";
using var muxer = ConnectionMultiplexer.Connect(connString);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it safe to assume there is always a replica in the test run?

@zmj
Copy link
Contributor Author

zmj commented Aug 15, 2020

A few tests failed. This one looks like a bug: StackExchange.Redis.RedisCommandException : This operation has been disabled in the command-map and cannot be used: ROLE. I guess I need to add ROLE to one (or more) of the lists in CommandMap? Which ones are appropriate?

The others looked like network errors that were probably unrelated to this change.

@mgravell
Copy link
Collaborator

mgravell commented Aug 15, 2020 via email

@zmj
Copy link
Contributor Author

zmj commented Sep 12, 2020

I merged in the main branch's test changes. MultiMaster.TestMultiNoTieBreak is still failing. Doesn't seem related to this PR's changes. Any suggestions?

@mgravell
Copy link
Collaborator

mgravell commented Sep 12, 2020 via email

@mgravell mgravell merged commit eacfff8 into StackExchange:main Oct 21, 2020
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.

2 participants