[SERVER-79545] Improve ClusterRole class Created: 31/Jul/23  Updated: 05/Sep/23

Status: Backlog
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: Billy Donahue Assignee: Backlog - Service Architecture
Resolution: Unresolved Votes: 0
Labels: techdebt
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to SERVER-78810 Improve Cluster Role API usability Open
Assigned Teams:
Service Arch
Participants:

 Description   

The ClusterRole is class filling a pretty Service Arch niche. In working with it, I noticed that it could use a refresh.

This thing is basically an unsigned integer with some accessors, and isn't IMO served well by the amount of C++ overloading and out-of-line function calls, nested constants container of a different type, etc.

Particularly the nested Value enum, the implicit constructors, and the capitalization of constants like ClusterRole::ShardServer. These are going to be used a lot so I think it's important to bring them inline with our naming convention.

I think ServiceArch could take a crack at improving it in these early days.



 Comments   
Comment by Billy Donahue [ 03/Sep/23 ]

This special behavior for the None value caused a bug while I was working on the CommandRegistry split. I believe the behavior of the function doesn't match its documentation, and the special case for the None value should not be part of the API.

This is the docs:

   /**
     * Returns `true` if this node plays the given role, `false` otherwise. Even if the node plays
     * the given role, it is not excluded that it also plays others.
     */
    bool has(const ClusterRole& role) const;

But here's how it works:

bool ClusterRole::has(const ClusterRole& role) const {
    return role._roleMask == None ? _roleMask == None : _roleMask & role._roleMask;
}

So if role is the special value None, then the has function behaves as if it was hasExclusively.
This needs to be fixed. Just don't treat the None value specially at all.

Generated at Thu Feb 08 06:41:15 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.