move permember ratelimiter to it's own package

This is required to break the the cyclic dependency and allow membership' tests to use testlogger package
dkrotx · Sep 25, 2024 · eac4235 · eac4235
1 parent 50e3558
commit eac4235
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 12 deletions.
diff --git a/common/membership/hashring_test.go b/common/membership/hashring_test.go
@@ -32,10 +32,11 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	"go.uber.org/goleak"
+	"go.uber.org/zap/zaptest/observer"
 
 	"github.com/uber/cadence/common"
 	"github.com/uber/cadence/common/clock"
-	"github.com/uber/cadence/common/log"
+	"github.com/uber/cadence/common/log/testlogger"
 	"github.com/uber/cadence/common/metrics"
 )
 
@@ -104,6 +105,7 @@ type hashringTestData struct {
 	mockPeerProvider *MockPeerProvider
 	mockTimeSource   clock.MockedTimeSource
 	hashRing         *ring
+	observedLogs     *observer.ObservedLogs
 }
 
 func newHashringTestData(t *testing.T) *hashringTestData {
@@ -114,11 +116,14 @@ func newHashringTestData(t *testing.T) *hashringTestData {
 	td.mockPeerProvider = NewMockPeerProvider(ctrl)
 	td.mockTimeSource = clock.NewMockedTimeSourceAt(time.Now())
 
+	logger, observedLogs := testlogger.NewObserved(t)
+	td.observedLogs = observedLogs
+
 	td.hashRing = newHashring(
 		"test-service",
 		td.mockPeerProvider,
 		td.mockTimeSource,
-		log.NewNoop(),
+		logger,
 		metrics.NoopScope(0),
 	)
 

diff --git a/common/quotas/permember.go → common/quotas/permember/permember.go b/common/quotas/permember.go → common/quotas/permember/permember.go
@@ -18,13 +18,14 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-package quotas
+package permember
 
 import (
 	"math"
 
 	"github.com/uber/cadence/common/dynamicconfig"
 	"github.com/uber/cadence/common/membership"
+	"github.com/uber/cadence/common/quotas"
 )
 
 // PerMember allows creating per instance RPS based on globalRPS averaged by member count for a given service.
@@ -52,7 +53,7 @@ func NewPerMemberDynamicRateLimiterFactory(
 	globalRPS dynamicconfig.IntPropertyFnWithDomainFilter,
 	instanceRPS dynamicconfig.IntPropertyFnWithDomainFilter,
 	resolver membership.Resolver,
-) LimiterFactory {
+) quotas.LimiterFactory {
 	return perMemberFactory{
 		service:     service,
 		globalRPS:   globalRPS,
@@ -68,8 +69,8 @@ type perMemberFactory struct {
 	resolver    membership.Resolver
 }
 
-func (f perMemberFactory) GetLimiter(domain string) Limiter {
-	return NewDynamicRateLimiter(func() float64 {
+func (f perMemberFactory) GetLimiter(domain string) quotas.Limiter {
+	return quotas.NewDynamicRateLimiter(func() float64 {
 		return PerMember(
 			f.service,
 			float64(f.globalRPS(domain)),

diff --git a/common/quotas/permember_test.go → common/quotas/permember/permember_test.go b/common/quotas/permember_test.go → common/quotas/permember/permember_test.go
@@ -18,7 +18,7 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-package quotas
+package permember
 
 import (
 	"testing"

diff --git a/common/resource/resourceImpl.go b/common/resource/resourceImpl.go
@@ -58,8 +58,8 @@ import (
 	"github.com/uber/cadence/common/partition"
 	"github.com/uber/cadence/common/persistence"
 	persistenceClient "github.com/uber/cadence/common/persistence/client"
-	"github.com/uber/cadence/common/quotas"
 	"github.com/uber/cadence/common/quotas/global/rpc"
+	"github.com/uber/cadence/common/quotas/permember"
 	"github.com/uber/cadence/common/service"
 )
 
@@ -182,7 +182,7 @@ func New(
 	persistenceBean, err := persistenceClient.NewBeanFromFactory(persistenceClient.NewFactory(
 		&params.PersistenceConfig,
 		func() float64 {
-			return quotas.PerMember(
+			return permember.PerMember(
 				serviceName,
 				float64(serviceConfig.PersistenceGlobalMaxQPS()),
 				float64(serviceConfig.PersistenceMaxQPS()),

diff --git a/service/frontend/service.go b/service/frontend/service.go
@@ -35,6 +35,7 @@ import (
 	"github.com/uber/cadence/common/log/tag"
 	"github.com/uber/cadence/common/quotas"
 	"github.com/uber/cadence/common/quotas/global/collection"
+	"github.com/uber/cadence/common/quotas/permember"
 	"github.com/uber/cadence/common/resource"
 	"github.com/uber/cadence/common/service"
 	"github.com/uber/cadence/service/frontend/admin"
@@ -263,7 +264,7 @@ func (s *Service) createGlobalQuotaCollections() (globalRatelimiterCollections,
 }
 func (s *Service) createBaseLimiters() ratelimiterCollections {
 	create := func(shared, perInstance dynamicconfig.IntPropertyFnWithDomainFilter) *quotas.Collection {
-		return quotas.NewCollection(quotas.NewPerMemberDynamicRateLimiterFactory(
+		return quotas.NewCollection(permember.NewPerMemberDynamicRateLimiterFactory(
 			service.Frontend,
 			shared,
 			perInstance,

diff --git a/service/history/engine/engineimpl/history_engine.go b/service/history/engine/engineimpl/history_engine.go
@@ -43,6 +43,7 @@ import (
 	cndc "github.com/uber/cadence/common/ndc"
 	"github.com/uber/cadence/common/persistence"
 	"github.com/uber/cadence/common/quotas"
+	"github.com/uber/cadence/common/quotas/permember"
 	"github.com/uber/cadence/common/reconciliation/invariant"
 	"github.com/uber/cadence/common/service"
 	"github.com/uber/cadence/common/types"
@@ -188,15 +189,15 @@ func NewEngineWithShardContext(
 			shard.GetConfig().NumArchiveSystemWorkflows,
 			quotas.NewDynamicRateLimiter(config.ArchiveRequestRPS.AsFloat64()),
 			quotas.NewDynamicRateLimiter(func() float64 {
-				return quotas.PerMember(
+				return permember.PerMember(
 					service.History,
 					float64(config.ArchiveInlineHistoryGlobalRPS()),
 					float64(config.ArchiveInlineHistoryRPS()),
 					shard.GetService().GetMembershipResolver(),
 				)
 			}),
 			quotas.NewDynamicRateLimiter(func() float64 {
-				return quotas.PerMember(
+				return permember.PerMember(
 					service.History,
 					float64(config.ArchiveInlineVisibilityGlobalRPS()),
 					float64(config.ArchiveInlineVisibilityRPS()),