spanner: take sessions in LIFO order

Session pool should hand out sessions in LIFO order

Fixes #1604

Change-Id: I55f68ad33c7e79bac25af75fba6e8db83c204c8f
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/46990
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Shanika Kuruppu <skuruppu@google.com>

spanner/session.go

@@ -828,12 +828,12 @@ func (p *sessionPool) recycle(s *session) bool {
 		// Reject the session if session is invalid or pool itself is invalid.
 		return false
 	}
-	// Put session at the back of the list to round robin for load balancing
+	// Put session at the top of the list to be handed out in LIFO order for load balancing
 	// across channels.
 	if s.isWritePrepared() {
-		s.setIdleList(p.idleWriteList.PushBack(s))
+		s.setIdleList(p.idleWriteList.PushFront(s))
 	} else {
-		s.setIdleList(p.idleList.PushBack(s))
+		s.setIdleList(p.idleList.PushFront(s))
 	}
 	// Broadcast that a session has been returned to idle list.
 	close(p.mayGetSession)

spanner/session_test.go

@@ -129,6 +129,81 @@ func TestSessionCreation(t *testing.T) {
 	hc.mu.Unlock()
 }
 
+// TestLIFOSessionOrder tests if session pool hand out sessions in LIFO order.
+func TestLIFOSessionOrder(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	_, client, teardown := setupMockedTestServerWithConfig(t,
+		ClientConfig{
+			SessionPoolConfig: SessionPoolConfig{
+				MaxOpened: 3,
+				MinOpened: 3,
+			},
+		})
+	defer teardown()
+	sp := client.idleSessions
+	// Create/take three sessions and recycle them.
+	shs, shsIDs := make([]*sessionHandle, 3), make([]string, 3)
+	for i := 0; i < len(shs); i++ {
+		var err error
+		if shs[i], err = sp.take(ctx); err != nil {
+			t.Fatalf("failed to take session(%v): %v", i, err)
+		}
+		shsIDs[i] = shs[i].getID()
+	}
+	for i := 0; i < len(shs); i++ {
+		shs[i].recycle()
+	}
+	for i := 2; i >= 0; i-- {
+		sh, err := sp.take(ctx)
+		if err != nil {
+			t.Fatalf("cannot take session from session pool: %v", err)
+		}
+		// check, if sessions returned in LIFO order.
+		if wantID, gotID := shsIDs[i], sh.getID(); wantID != gotID {
+			t.Fatalf("got session with id: %v, want: %v", gotID, wantID)
+		}
+	}
+}
+
+// TestLIFOTakeWriteSessionOrder tests if write session pool hand out sessions in LIFO order.
+func TestLIFOTakeWriteSessionOrder(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	_, client, teardown := setupMockedTestServerWithConfig(t,
+		ClientConfig{
+			SessionPoolConfig: SessionPoolConfig{
+				MaxOpened:     3,
+				MinOpened:     3,
+				WriteSessions: 1,
+			},
+		})
+	defer teardown()
+	sp := client.idleSessions
+	// Create/take three sessions and recycle them.
+	shs, shsIDs := make([]*sessionHandle, 3), make([]string, 3)
+	for i := 0; i < len(shs); i++ {
+		var err error
+		if shs[i], err = sp.takeWriteSession(ctx); err != nil {
+			t.Fatalf("failed to take session(%v): %v", i, err)
+		}
+		shsIDs[i] = shs[i].getID()
+	}
+	for i := 0; i < len(shs); i++ {
+		shs[i].recycle()
+	}
+	for i := 2; i >= 0; i-- {
+		ws, err := sp.takeWriteSession(ctx)
+		if err != nil {
+			t.Fatalf("cannot take session from session pool: %v", err)
+		}
+		// check, if write sessions returned in LIFO order.
+		if wantID, gotID := shsIDs[i], ws.getID(); wantID != gotID {
+			t.Fatalf("got session with id: %v, want: %v", gotID, wantID)
+		}
+	}
+}
+
 // TestTakeFromIdleList tests taking sessions from session pool's idle list.
 func TestTakeFromIdleList(t *testing.T) {
 	t.Parallel()