GODRIVER-1499 Remove timeout checks from server selection fast path (#356)

Author: Divjot Arora

x/mongo/driver/operation.go

@@ -195,12 +195,6 @@ func (op Operation) selectServer(ctx context.Context) (Server, error) {
 		return nil, err
 	}
 
-	select {
-	case <-ctx.Done():
-		return nil, ctx.Err()
-	default:
-	}
-
 	selector := op.Selector
 	if selector == nil {
 		rp := op.ReadPreference

x/mongo/driver/operation_test.go

@@ -54,20 +54,6 @@ func TestOperation(t *testing.T) {
 				t.Error("Expected a validation error from selectServer, but got <nil>")
 			}
 		})
-		t.Run("returns context error when expired", func(t *testing.T) {
-			op := &Operation{
-				CommandFn:  func([]byte, description.SelectedServer) ([]byte, error) { return nil, nil },
-				Deployment: new(mockDeployment),
-				Database:   "testing",
-			}
-			ctx, cancel := context.WithCancel(context.Background())
-			cancel()
-			want := context.Canceled
-			_, got := op.selectServer(ctx)
-			if got != want {
-				t.Errorf("Did not get expected error. got %v; want %v", got, want)
-			}
-		})
 		t.Run("uses specified server selector", func(t *testing.T) {
 			want := new(mockServerSelector)
 			d := new(mockDeployment)

x/mongo/driver/topology/topology.go

@@ -459,13 +459,8 @@ func (t *Topology) selectServerFromSubscription(ctx context.Context, subscriptio
 func (t *Topology) selectServerFromDescription(ctx context.Context, desc description.Topology,
 	selectionState serverSelectionState) ([]description.Server, error) {
 
-	select {
-	case <-ctx.Done():
-		return nil, ctx.Err()
-	case <-selectionState.timeoutChan:
-		return nil, wrapServerSelectionError(ErrServerSelectionTimeout, t)
-	default:
-	}
+	// Unlike selectServerFromSubscription, this code path does not check ctx.Done or selectionState.timeoutChan because
+	// selecting a server from a description is not a blocking operation.
 
 	var allowed []description.Server
 	for _, s := range desc.Servers {

x/mongo/driver/topology/topology_test.go

@@ -315,8 +315,8 @@ func TestServerSelection(t *testing.T) {
 			t.Errorf("Incorrect sever selected. got %s; want %s", srvs[0].Addr, desc.Servers[1].Addr)
 		}
 	})
-	t.Run("no subscription in fast path", func(t *testing.T) {
-		// assert that a subscription is not created if there is a server available
+	t.Run("fast path does not subscribe or check timeouts", func(t *testing.T) {
+		// Assert that the server selection fast path does not create a Subscription or check for timeout errors.
 		topo, err := New()
 		noerr(t, err)
 		topo.cfg.cs.HeartbeatInterval = time.Minute
@@ -335,13 +335,31 @@ func TestServerSelection(t *testing.T) {
 			topo.servers[srv.Addr] = s
 		}
 
-		// manually close subscriptions so calls to Subscribe will error
+		// Manually close subscriptions so calls to Subscribe will error and pass in a cancelled context to ensure the
+		// fast path ignores timeout errors.
 		topo.subscriptionsClosed = true
-		selectedServer, err := topo.SelectServer(context.Background(), description.WriteSelector())
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+		selectedServer, err := topo.SelectServer(ctx, description.WriteSelector())
 		noerr(t, err)
 		selectedAddr := selectedServer.(*SelectedServer).address
 		assert.Equal(t, primaryAddr, selectedAddr, "expected address %v, got %v", primaryAddr, selectedAddr)
 	})
+	t.Run("default to selecting from subscription if fast path fails", func(t *testing.T) {
+		topo, err := New()
+		noerr(t, err)
+
+		topo.cfg.cs.HeartbeatInterval = time.Minute
+		atomic.StoreInt32(&topo.connectionstate, connected)
+		desc := description.Topology{
+			Servers: []description.Server{},
+		}
+		topo.desc.Store(desc)
+
+		topo.subscriptionsClosed = true
+		_, err = topo.SelectServer(context.Background(), description.WriteSelector())
+		assert.Equal(t, ErrSubscribeAfterClosed, err, "expected error %v, got %v", ErrSubscribeAfterClosed, err)
+	})
 }
 
 func TestSessionTimeout(t *testing.T) {