GODRIVER-1740 Clean up address and description packages (#514)

Author: Divjot Arora

mongo/collection.go

@@ -1633,7 +1633,14 @@ func (coll *Collection) Drop(ctx context.Context) error {
 func makePinnedSelector(sess *session.Client, defaultSelector description.ServerSelector) description.ServerSelectorFunc {
 	return func(t description.Topology, svrs []description.Server) ([]description.Server, error) {
 		if sess != nil && sess.PinnedServer != nil {
-			return sess.PinnedServer.SelectServer(t, svrs)
+			// If there is a pinned server, try to find it in the list of candidates.
+			for _, candidate := range svrs {
+				if candidate.Addr == sess.PinnedServer.Addr {
+					return []description.Server{candidate}, nil
+				}
+			}
+
+			return nil, nil
 		}
 
 		return defaultSelector.SelectServer(t, svrs)

mongo/database.go

@@ -162,7 +162,7 @@ func (db *Database) processRunCommand(ctx context.Context, cmd interface{},
 		description.LatencySelector(db.client.localThreshold),
 	})
 	if sess != nil && sess.PinnedServer != nil {
-		readSelect = sess.PinnedServer
+		readSelect = makePinnedSelector(sess, readSelect)
 	}
 
 	return operation.NewCommand(runCmdDoc).

mongo/description/feature.go

@@ -18,17 +18,14 @@ import (
 	"go.mongodb.org/mongo-driver/tag"
 )
 
-// UnsetRTT is the unset value for a round trip time.
-const UnsetRTT = -1 * time.Millisecond
-
-// SelectedServer represents a selected server that is a member of a topology.
+// SelectedServer augments the Server type by also including the TopologyKind of the topology that includes the server.
+// This type should be used to track the state of a server that was selected to perform an operation.
 type SelectedServer struct {
 	Server
 	Kind TopologyKind
 }
 
-// Server represents a description of a server. This is created from an isMaster
-// command.
+// Server contains information about a node in a cluster. This is created from isMaster command responses.
 type Server struct {
 	Addr address.Address
 
@@ -59,7 +56,7 @@ type Server struct {
 	WireVersion           *VersionRange
 }
 
-// NewServer creates a new server description from the given parameters.
+// NewServer creates a new server description from the given isMaster command response.
 func NewServer(addr address.Address, response bson.Raw) Server {
 	desc := Server{Addr: addr, CanonicalAddr: addr, LastUpdateTime: time.Now().UTC()}
 	elements, err := response.Elements()
@@ -318,12 +315,7 @@ func NewServerFromError(addr address.Address, err error, tv *TopologyVersion) Se
 // SetAverageRTT sets the average round trip time for this server description.
 func (s Server) SetAverageRTT(rtt time.Duration) Server {
 	s.AverageRTT = rtt
-	if rtt == UnsetRTT {
-		s.AverageRTTSet = false
-	} else {
-		s.AverageRTTSet = true
-	}
-
+	s.AverageRTTSet = true
 	return s
 }
 
@@ -335,16 +327,6 @@ func (s Server) DataBearing() bool {
 		s.Kind == Standalone
 }
 
-// SelectServer selects this server if it is in the list of given candidates.
-func (s Server) SelectServer(_ Topology, candidates []Server) ([]Server, error) {
-	for _, candidate := range candidates {
-		if candidate.Addr == s.Addr {
-			return []Server{candidate}, nil
-		}
-	}
-	return nil, nil
-}
-
 // String implements the Stringer interface
 func (s Server) String() string {
 	str := fmt.Sprintf("Addr: %s, Type: %s",
@@ -382,11 +364,6 @@ func decodeStringMap(element bson.RawElement, name string) (map[string]string, e
 	return m, nil
 }
 
-// SupportsRetryWrites returns true if this description represents a server that supports retryable writes.
-func (s Server) SupportsRetryWrites() bool {
-	return s.SessionTimeoutMinutes != 0 && s.Kind != Standalone
-}
-
 // Equal compares two server descriptions and returns true if they are equal
 func (s Server) Equal(other Server) bool {
 	if s.CanonicalAddr.String() != other.CanonicalAddr.String() {
@@ -446,7 +423,7 @@ func (s Server) Equal(other Server) bool {
 		return false
 	}
 
-	if s.TopologyVersion != other.TopologyVersion && CompareTopologyVersion(s.TopologyVersion, other.TopologyVersion) != 0 {
+	if s.TopologyVersion.CompareToIncoming(other.TopologyVersion) != 0 {
 		return false
 	}
 

mongo/description/feature_test.go

@@ -6,7 +6,7 @@
 
 package description
 
-// ServerKind represents the type of a server.
+// ServerKind represents the type of a single server in a topology.
 type ServerKind uint32
 
 // These constants are the possible types of servers.
@@ -20,7 +20,7 @@ const (
 	Mongos      ServerKind = 256
 )
 
-// String implements the fmt.Stringer interface.
+// String returns a stringified version of the kind or "Unknown" if the kind is invalid.
 func (kind ServerKind) String() string {
 	switch kind {
 	case Standalone:

mongo/description/selector_write_test.go

@@ -15,8 +15,9 @@ import (
 	"go.mongodb.org/mongo-driver/tag"
 )
 
-// ServerSelector is an interface implemented by types that can select a server given a
-// topology description.
+// ServerSelector is an interface implemented by types that can perform server selection given a topology description
+// and list of candidate servers. The selector should filter the provided candidates list and return a subset that
+// matches some criteria.
 type ServerSelector interface {
 	SelectServer(Topology, []Server) ([]Server, error)
 }
@@ -33,7 +34,16 @@ type compositeSelector struct {
 	selectors []ServerSelector
 }
 
-// CompositeSelector combines multiple selectors into a single selector.
+// CompositeSelector combines multiple selectors into a single selector by applying them in order to the candidates
+// list.
+//
+// For example, if the initial candidates list is [s0, s1, s2, s3] and two selectors are provided where the first
+// matches s0 and s1 and the second matches s1 and s2, the following would occur during server selection:
+//
+// 1. firstSelector([s0, s1, s2, s3]) -> [s0, s1]
+// 2. secondSelector([s0, s1]) -> [s1]
+//
+// The final list of candidates returned by the composite selector would be [s1].
 func CompositeSelector(selectors []ServerSelector) ServerSelector {
 	return &compositeSelector{selectors: selectors}
 }
@@ -53,7 +63,7 @@ type latencySelector struct {
 	latency time.Duration
 }
 
-// LatencySelector creates a ServerSelector which selects servers based on their latency.
+// LatencySelector creates a ServerSelector which selects servers based on their average RTT values.
 func LatencySelector(latency time.Duration) ServerSelector {
 	return &latencySelector{latency: latency}
 }
@@ -120,7 +130,7 @@ func ReadPrefSelector(rp *readpref.ReadPref) ServerSelector {
 		if _, set := rp.MaxStaleness(); set {
 			for _, s := range candidates {
 				if s.Kind != Unknown {
-					if err := MaxStalenessSupported(s.WireVersion); err != nil {
+					if err := maxStalenessSupported(s.WireVersion); err != nil {
 						return nil, err
 					}
 				}
@@ -140,6 +150,15 @@ func ReadPrefSelector(rp *readpref.ReadPref) ServerSelector {
 	})
 }
 
+// maxStalenessSupported returns an error if the given server version does not support max staleness.
+func maxStalenessSupported(wireVersion *VersionRange) error {
+	if wireVersion != nil && wireVersion.Max < 5 {
+		return fmt.Errorf("max staleness is only supported for servers 3.4 or newer")
+	}
+
+	return nil
+}
+
 func selectForReplicaSet(rp *readpref.ReadPref, t Topology, candidates []Server) ([]Server, error) {
 	if err := verifyMaxStaleness(rp, t); err != nil {
 		return nil, err