Optimize Connection role lookups with roleSet for O(1) performance

Replace Connection.Roles []string with roleSet (map[string]struct{}) to
enable O(1) role lookups instead of O(n) iterations in filterByRoles().
While most nodes have 1-3 roles where sequential comparison might be
faster, this future-proofs performance for any role count and provides
consistent algorithmic behavior.

Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

Author: sean-

opensearchtransport/connection.go

@@ -31,7 +31,6 @@ import (
 	"fmt"
 	"math"
 	"net/url"
-	"slices"
 	"sort"
 	"strings"
 	"sync"
@@ -99,7 +98,7 @@ type Connection struct {
 	URL        *url.URL
 	ID         string
 	Name       string
-	Roles      []string
+	Roles      roleSet
 	Attributes map[string]any
 
 	failures atomic.Int64
@@ -580,8 +579,8 @@ func (s *RoleBasedSelector) filterByRoles(connections []*Connection) []*Connecti
 		// Check if connection has at least one required role
 		hasRequiredRole := len(s.requiredRoles) == 0 // If no required roles, all nodes qualify
 		if !hasRequiredRole {
-			for _, role := range conn.Roles {
-				if slices.Contains(s.requiredRoles, role) {
+			for _, role := range s.requiredRoles {
+				if conn.Roles.has(role) {
 					hasRequiredRole = true
 					break
 				}
@@ -594,8 +593,8 @@ func (s *RoleBasedSelector) filterByRoles(connections []*Connection) []*Connecti
 
 		// Check if connection has any excluded roles
 		hasExcludedRole := false
-		for _, role := range conn.Roles {
-			if slices.Contains(s.excludedRoles, role) {
+		for _, role := range s.excludedRoles {
+			if conn.Roles.has(role) {
 				hasExcludedRole = true
 				break
 			}

opensearchtransport/discovery.go

@@ -121,6 +121,30 @@ func (rs roleSet) has(roleName string) bool {
 	return exists
 }
 
+// toSlice converts the roleSet back to a []string slice for compatibility.
+// The roles are sorted alphabetically for consistent ordering.
+func (rs roleSet) toSlice() []string {
+	if len(rs) == 0 {
+		return nil
+	}
+
+	roles := make([]string, 0, len(rs))
+	for role := range rs {
+		// Skip the internal cluster_manager alias added for deprecated master role
+		if role == RoleClusterManager {
+			// Check if this was added as an alias for deprecated master role
+			if _, hasMaster := rs[RoleMaster]; hasMaster {
+				continue // Skip the alias, keep only the original master role
+			}
+		}
+		roles = append(roles, role)
+	}
+
+	// Sort roles alphabetically for consistent ordering
+	slices.Sort(roles)
+	return roles
+}
+
 // isDedicatedClusterManager implements the logic from upstream Java client
 // NodeSelector.SKIP_DEDICATED_CLUSTER_MASTERS to determine if a node should be skipped.
 // It returns true for nodes that are cluster-manager eligible but have no "work" roles
@@ -199,7 +223,7 @@ func (c *Client) DiscoverNodes() error {
 			URL:        node.URL,
 			ID:         node.ID,
 			Name:       node.Name,
-			Roles:      node.Roles,
+			Roles:      newRoleSet(node.Roles),
 			Attributes: node.Attributes,
 		})
 	}

opensearchtransport/discovery_internal_test.go

@@ -39,6 +39,7 @@ import (
 	"net/url"
 	"os"
 	"reflect"
+	"slices"
 	"testing"
 	"time"
 
@@ -553,8 +554,14 @@ func TestDiscovery(t *testing.T) {
 				}
 
 				for _, conn := range pool.mu.live {
-					if !reflect.DeepEqual(tt.args.Nodes[conn.ID].Roles, conn.Roles) {
-						t.Errorf("Unexpected roles for node %s, want=%s, got=%s", conn.Name, tt.args.Nodes[conn.ID], conn.Roles)
+					expectedRoles := make([]string, len(tt.args.Nodes[conn.ID].Roles))
+					copy(expectedRoles, tt.args.Nodes[conn.ID].Roles)
+					slices.Sort(expectedRoles)
+
+					actualRoles := conn.Roles.toSlice()
+
+					if !reflect.DeepEqual(expectedRoles, actualRoles) {
+						t.Errorf("Unexpected roles for node %s, want=%s, got=%s", conn.Name, expectedRoles, actualRoles)
 					}
 				}
 
@@ -1023,11 +1030,11 @@ func TestIncludeDedicatedClusterManagersConfiguration(t *testing.T) {
 // TestGenericRoleBasedSelector tests the new generic role-based selector
 func TestGenericRoleBasedSelector(t *testing.T) {
 	connections := []*Connection{
-		{Name: "data-node", Roles: []string{RoleData}},
-		{Name: "ingest-node", Roles: []string{RoleIngest}},
-		{Name: "data-ingest-node", Roles: []string{RoleData, RoleIngest}},
-		{Name: "cluster-manager-node", Roles: []string{RoleClusterManager}},
-		{Name: "coordinating-node", Roles: []string{}}, // No specific roles
+		{Name: "data-node", Roles: newRoleSet([]string{RoleData})},
+		{Name: "ingest-node", Roles: newRoleSet([]string{RoleIngest})},
+		{Name: "data-ingest-node", Roles: newRoleSet([]string{RoleData, RoleIngest})},
+		{Name: "cluster-manager-node", Roles: newRoleSet([]string{RoleClusterManager})},
+		{Name: "coordinating-node", Roles: newRoleSet([]string{})}, // No specific roles
 	}
 
 	fallback := &mockSelector{}
@@ -1096,13 +1103,13 @@ func TestGenericRoleBasedSelector(t *testing.T) {
 func TestRoleBasedSelectors(t *testing.T) {
 	// Create test connections with different roles
 	connections := []*Connection{
-		{Name: "data-node", Roles: []string{RoleData}},
-		{Name: "ingest-node", Roles: []string{RoleIngest}},
-		{Name: "data-ingest-node", Roles: []string{RoleData, RoleIngest}},
-		{Name: "cluster-manager-node", Roles: []string{RoleClusterManager}},
-		{Name: "warm-node", Roles: []string{RoleWarm}},
-		{Name: "search-node", Roles: []string{RoleSearch}},
-		{Name: "coordinating-node", Roles: []string{}}, // No specific roles
+		{Name: "data-node", Roles: newRoleSet([]string{RoleData})},
+		{Name: "ingest-node", Roles: newRoleSet([]string{RoleIngest})},
+		{Name: "data-ingest-node", Roles: newRoleSet([]string{RoleData, RoleIngest})},
+		{Name: "cluster-manager-node", Roles: newRoleSet([]string{RoleClusterManager})},
+		{Name: "warm-node", Roles: newRoleSet([]string{RoleWarm})},
+		{Name: "search-node", Roles: newRoleSet([]string{RoleSearch})},
+		{Name: "coordinating-node", Roles: newRoleSet([]string{})}, // No specific roles
 	}
 
 	// Mock fallback selector that just returns the first connection
@@ -1180,9 +1187,9 @@ func TestRoleBasedSelectors(t *testing.T) {
 func TestSmartSelector(t *testing.T) {
 	// Create test connections
 	connections := []*Connection{
-		{Name: "data-node", Roles: []string{RoleData}},
-		{Name: "ingest-node", Roles: []string{RoleIngest}},
-		{Name: "data-ingest-node", Roles: []string{RoleData, RoleIngest}},
+		{Name: "data-node", Roles: newRoleSet([]string{RoleData})},
+		{Name: "ingest-node", Roles: newRoleSet([]string{RoleIngest})},
+		{Name: "data-ingest-node", Roles: newRoleSet([]string{RoleData, RoleIngest})},
 	}
 
 	fallback := &mockSelector{}
@@ -1235,8 +1242,8 @@ func TestSmartSelector(t *testing.T) {
 // TestRequestAwareConnectionPool tests the enhanced connection pool
 func TestRequestAwareConnectionPool(t *testing.T) {
 	connections := []*Connection{
-		{Name: "data-node", Roles: []string{RoleData}},
-		{Name: "ingest-node", Roles: []string{RoleIngest}},
+		{Name: "data-node", Roles: newRoleSet([]string{RoleData})},
+		{Name: "ingest-node", Roles: newRoleSet([]string{RoleIngest})},
 	}
 
 	smartSelector := NewSmartSelector(&mockSelector{})

opensearchtransport/metrics.go

@@ -140,7 +140,7 @@ func (c *Client) Metrics() (Metrics, error) {
 			}
 
 			if len(c.Roles) > 0 {
-				cm.Meta.Roles = c.Roles
+				cm.Meta.Roles = c.Roles.toSlice()
 			}
 
 			m.Connections = append(m.Connections, cm)