Use link caching for $management operations (renew locks) (#248)

Make it so we use a single management link (most of the time) when doing dispositions or lock renewal (or other management related operations).

Also, added in a simple stress test. It's a bit manual at the moment, but it initiates 1000 concurrent renew lock calls at once, so it's a decent test of our response routing.

Author: richardpark-msft

changelog.md

@@ -1,5 +1,11 @@
 # Change Log
 
+## `v0.11.0`
+
+- Use a singleton instance of the management link, avoiding creating a link per management
+  link operations like dispositions or lock renewals. 
+  [PR#248](https://github.com/Azure/azure-service-bus-go/pull/248)
+
 ## `v0.10.16`
 
 - fixed an issue where links weren't being closed when retrying

entity.go

@@ -22,7 +22,6 @@ type (
 	}
 
 	receivingEntity struct {
-		renewMessageLockMutex sync.Mutex
 		*entity
 	}
 
@@ -192,9 +191,6 @@ func (re *receivingEntity) RenewLocks(ctx context.Context, messages ...*Message)
 	ctx, span := re.startSpanFromContext(ctx, "sb.receivingEntity.RenewLocks")
 	defer span.End()
 
-	re.renewMessageLockMutex.Lock()
-	defer re.renewMessageLockMutex.Unlock()
-
 	client, err := re.entity.GetRPCClient(ctx)
 	if err != nil {
 		tab.For(ctx).Error(err)

go.mod

@@ -3,9 +3,9 @@ module github.com/Azure/azure-service-bus-go
 go 1.12
 
 require (
-	github.com/Azure/azure-amqp-common-go/v3 v3.1.0
+	github.com/Azure/azure-amqp-common-go/v3 v3.1.2
 	github.com/Azure/azure-sdk-for-go v51.1.0+incompatible
-	github.com/Azure/go-amqp v0.13.11
+	github.com/Azure/go-amqp v0.13.13
 	github.com/Azure/go-autorest/autorest v0.11.18
 	github.com/Azure/go-autorest/autorest/adal v0.9.13
 	github.com/Azure/go-autorest/autorest/date v0.3.0

go.sum

@@ -1,28 +1,23 @@
-github.com/Azure/azure-amqp-common-go/v3 v3.1.0 h1:1N4YSkWYWffOpQHromYdOucBSQXhNRKzqtgICy6To8Q=
-github.com/Azure/azure-amqp-common-go/v3 v3.1.0/go.mod h1:PBIGdzcO1teYoufTKMcGibdKaYZv4avS+O6LNIp8bq0=
+github.com/Azure/azure-amqp-common-go/v3 v3.1.2 h1:Xu9nboejX9a+7uOGiwEUlpD8/oN4A6f7ZmHTMmJpPkw=
+github.com/Azure/azure-amqp-common-go/v3 v3.1.2/go.mod h1:zN7QL/vfCsq3XQxQaTkg4ScO786CA2rQnZ1LXX7QryE=
 github.com/Azure/azure-sdk-for-go v51.1.0+incompatible h1:7uk6GWtUqKg6weLv2dbKnzwb0ml1Qn70AdtRccZ543w=
 github.com/Azure/azure-sdk-for-go v51.1.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
-github.com/Azure/go-amqp v0.13.0/go.mod h1:qj+o8xPCz9tMSbQ83Vp8boHahuRDl5mkNHyt1xlxUTs=
-github.com/Azure/go-amqp v0.13.11 h1:E28zKoWuzO4+D80iUD88BUorI5PqvIZ/S/77md3hIvA=
-github.com/Azure/go-amqp v0.13.11/go.mod h1:D5ZrjQqB1dyp1A+G73xeL/kNn7D5qHJIIsNNps7YNmk=
+github.com/Azure/go-amqp v0.13.13 h1:OBPwCO50EzniOyZR0M4VbGJYDxceIy3SFOnKVMJktdY=
+github.com/Azure/go-amqp v0.13.13/go.mod h1:D5ZrjQqB1dyp1A+G73xeL/kNn7D5qHJIIsNNps7YNmk=
 github.com/Azure/go-autorest v14.2.0+incompatible h1:V5VMDjClD3GiElqLWO7mz2MxNAK/vTfRHdAubSIPRgs=
 github.com/Azure/go-autorest v14.2.0+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24=
-github.com/Azure/go-autorest/autorest v0.11.3/go.mod h1:JFgpikqFJ/MleTTxwepExTKnFUKKszPS8UavbQYUMuw=
 github.com/Azure/go-autorest/autorest v0.11.18 h1:90Y4srNYrwOtAgVo3ndrQkTYn6kf1Eg/AjTFJ8Is2aM=
 github.com/Azure/go-autorest/autorest v0.11.18/go.mod h1:dSiJPy22c3u0OtOKDNttNgqpNFY/GeWa7GH/Pz56QRA=
-github.com/Azure/go-autorest/autorest/adal v0.9.0/go.mod h1:/c022QCutn2P7uY+/oQWWNcK9YU+MH96NgK+jErpbcg=
 github.com/Azure/go-autorest/autorest/adal v0.9.13 h1:Mp5hbtOePIzM8pJVRa3YLrWWmZtoxRXqUEzCfJt3+/Q=
 github.com/Azure/go-autorest/autorest/adal v0.9.13/go.mod h1:W/MM4U6nLxnIskrw4UwWzlHfGjwUS50aOsc/I3yuU8M=
 github.com/Azure/go-autorest/autorest/date v0.3.0 h1:7gUk1U5M/CQbp9WoqinNzJar+8KY+LPI6wiWrP/myHw=
 github.com/Azure/go-autorest/autorest/date v0.3.0/go.mod h1:BI0uouVdmngYNUzGWeSYnokU+TrmwEsOqdt8Y6sso74=
-github.com/Azure/go-autorest/autorest/mocks v0.4.0/go.mod h1:LTp+uSrOhSkaKrUy935gNZuuIPPVsHlr9DSOxSayd+k=
 github.com/Azure/go-autorest/autorest/mocks v0.4.1 h1:K0laFcLE6VLTOwNgSxaGbUcLPuGXlNkbVvq4cW4nIHk=
 github.com/Azure/go-autorest/autorest/mocks v0.4.1/go.mod h1:LTp+uSrOhSkaKrUy935gNZuuIPPVsHlr9DSOxSayd+k=
 github.com/Azure/go-autorest/autorest/to v0.4.0 h1:oXVqrxakqqV1UZdSazDOPOLvOIz+XA683u8EctwboHk=
 github.com/Azure/go-autorest/autorest/to v0.4.0/go.mod h1:fE8iZBn7LQR7zH/9XU2NcPR4o9jEImooCeWJcYV/zLE=
 github.com/Azure/go-autorest/autorest/validation v0.3.1 h1:AgyqjAd94fwNAoTjl/WQXg4VvFeRFpO+UhNyRXqF1ac=
 github.com/Azure/go-autorest/autorest/validation v0.3.1/go.mod h1:yhLgjC0Wda5DYXl6JAsWyUe4KVNffhoDhG0zVzUMo3E=
-github.com/Azure/go-autorest/logger v0.2.0/go.mod h1:T9E3cAhj2VqvPOtCYAvby9aBXkZmbF5NWuPV8+WeEW8=
 github.com/Azure/go-autorest/logger v0.2.1 h1:IG7i4p/mDa2Ce4TRyAO8IHnVhAVF3RFU+ZtXWSmf4Tg=
 github.com/Azure/go-autorest/logger v0.2.1/go.mod h1:T9E3cAhj2VqvPOtCYAvby9aBXkZmbF5NWuPV8+WeEW8=
 github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUMfuitfgcfuo=
@@ -32,7 +27,6 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/devigned/tab v0.1.1 h1:3mD6Kb1mUOYeLpJvTVSDwSg5ZsfSxfvxGRTxRsJsITA=
 github.com/devigned/tab v0.1.1/go.mod h1:XG9mPq0dFghrYvoBF3xdRrJzSTX1b7IQrvaL9mzjeJY=
-github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
 github.com/form3tech-oss/jwt-go v3.2.2+incompatible h1:TcekIExNqud5crz4xD2pavyTgWiPvpYe4Xau31I0PRk=
 github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k=
 github.com/fortytw2/leaktest v1.3.0 h1:u8491cBMTQ8ft8aeV+adlcytMZylmA5nnwwkRZjI8vw=
@@ -95,8 +89,6 @@ github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVM
 github.com/ugorji/go/codec v1.1.7 h1:2SvQaVZ1ouYrrKKwoSk2pzd4A9evlKJb9oTL+OaLUSs=
 github.com/ugorji/go/codec v1.1.7/go.mod h1:Ax+UKWsSmolVDwsd+7N3ZtXu+yMGCf907BLYF3GoBXY=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
-golang.org/x/crypto v0.0.0-20200728195943-123391ffb6de/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0 h1:hb9wdF1z5waM+dSIICn1l0DkLVDT3hqhhQsDNUmHPRE=
 golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=

internal/stress/mgmtlink/renewLocksStressTest.go

@@ -0,0 +1,114 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"log"
+	"os"
+	"sync"
+	"sync/atomic"
+	"time"
+
+	servicebus "github.com/Azure/azure-service-bus-go"
+	"github.com/joho/godotenv"
+)
+
+const queuePrefetch = 1000
+const renewalsPerMessage = 2
+
+func main() {
+	ctx := context.Background()
+
+	godotenv.Load(".env")
+	cs := os.Getenv("SERVICEBUS_CONNECTION_STRING")
+	queueName := "samples"
+
+	ns, err := servicebus.NewNamespace(servicebus.NamespaceWithConnectionString(cs))
+
+	if err != nil {
+		log.Fatalf("Failed to create namespace client: %s", err.Error())
+	}
+
+	sender, err := ns.NewSender(ctx, queueName)
+
+	if err != nil {
+		log.Fatalf("Failed to create sender: %s", err.Error())
+	}
+
+	ch := make(chan bool, 100)
+
+	for i := 0; i < queuePrefetch; i++ {
+		ch <- true
+		go func(i int) {
+			defer func() { <-ch }()
+			if err = sender.Send(ctx, &servicebus.Message{Data: []byte(fmt.Sprintf("hello world %d", i))}); err != nil {
+				log.Fatalf("Failed to send message: %s", err.Error())
+			}
+		}(i)
+	}
+
+	queue, err := ns.NewQueue(queueName, servicebus.QueueWithPrefetchCount(queuePrefetch))
+
+	if err != nil {
+		log.Fatalf("Failed to create receiver: %s", err.Error())
+	}
+
+	renewals := int32(0)
+	outstandingRenewals := int32(0)
+	failedRenewals := int32(0)
+	lastRenewalWasFailure := int32(0)
+
+	wg := &sync.WaitGroup{}
+
+	go func() {
+		ticker := time.NewTicker(time.Second * 5)
+
+		for {
+			select {
+			case <-ticker.C:
+				log.Printf("Messages: [total: %d, outstanding: %d, failed: %d]", atomic.LoadInt32(&renewals), atomic.LoadInt32(&outstandingRenewals), atomic.LoadInt32(&failedRenewals))
+			}
+		}
+	}()
+
+	err = queue.Receive(ctx, servicebus.HandlerFunc(func(c context.Context, m *servicebus.Message) error {
+		wg.Add(1)
+
+		go func() {
+			worked := false
+
+			for i := 0; i < renewalsPerMessage; i++ {
+				atomic.AddInt32(&outstandingRenewals, 1)
+				if err := queue.RenewLocks(ctx, m); err != nil {
+					worked = false
+					atomic.AddInt32(&failedRenewals, 1)
+					log.Printf("ERROR renewing: %+v", err)
+				} else {
+					worked = true
+				}
+
+				atomic.AddInt32(&outstandingRenewals, -1)
+				atomic.AddInt32(&renewals, 1)
+			}
+
+			m.Complete(ctx)
+
+			if !worked {
+				log.Printf("Last renewal was a failure here.")
+				atomic.AddInt32(&lastRenewalWasFailure, 1)
+			}
+
+			wg.Done()
+		}()
+
+		return nil
+	}))
+
+	log.Printf("Last Renewal was failure: %d", lastRenewalWasFailure)
+
+	if err != nil {
+		log.Fatalf("Failed to receive messages")
+	}
+
+	wg.Wait()
+}

namespace.go

@@ -75,6 +75,11 @@ type (
 		initRefresh sync.Once
 		// populated with the result from auto-refresh, to be called elsewhere
 		cancelRefresh func() <-chan struct{}
+
+		// for testing
+
+		// alias for 'amqp.Dial'
+		amqpDial func(addr string, opts ...amqp.ConnOption) (*amqp.Client, error)
 	}
 
 	// NamespaceOption provides structure for configuring a new Service Bus namespace
@@ -189,6 +194,7 @@ func NamespaceWithTokenProvider(provider auth.TokenProvider) NamespaceOption {
 func NewNamespace(opts ...NamespaceOption) (*Namespace, error) {
 	ns := &Namespace{
 		Environment: azure.PublicCloud,
+		amqpDial:    amqp.Dial,
 	}
 
 	for _, opt := range opts {
@@ -235,7 +241,7 @@ func (ns *Namespace) newClient(ctx context.Context) (*amqp.Client, error) {
 		return amqp.New(nConn, append(defaultConnOptions, amqp.ConnServerHostname(ns.getHostname()))...)
 	}
 
-	return amqp.Dial(ns.getAMQPHostURI(), defaultConnOptions...)
+	return ns.amqpDial(ns.getAMQPHostURI(), defaultConnOptions...)
 }
 
 // negotiateClaim performs initial authentication and starts periodic refresh of credentials.