address comments

Signed-off-by: Yi Jin <[email protected]>
thanos-io · Dec 2, 2024 · 0b25115 · 0b25115
1 parent ca6c149
commit 0b25115
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 21 deletions.
diff --git a/pkg/receive/multitsdb.go b/pkg/receive/multitsdb.go
@@ -136,24 +136,13 @@ func NewMultiTSDB(
 	return mt
 }
 
-// getTenant returns the tenant with the given tenantID.
-// @testing: This method is only for testing purposes.
-func (t *MultiTSDB) getTenant(tenantID string) *tenant {
+// testGetTenant returns the tenant with the given tenantID for testing purposes.
+func (t *MultiTSDB) testGetTenant(tenantID string) *tenant {
 	t.mtx.RLock()
 	defer t.mtx.RUnlock()
 	return t.tenants[tenantID]
 }
 
-// tsdbLocalClientsCopied returns a copy of tsdbClients.
-// @testing: This method is only for testing purposes.
-func (t *MultiTSDB) tsdbLocalClientsCopied() []store.Client {
-	t.mtx.Lock()
-	defer t.mtx.Unlock()
-	copied := make([]store.Client, len(t.tsdbClients))
-	copy(copied, t.tsdbClients)
-	return copied
-}
-
 func (t *MultiTSDB) updateTSDBClients() {
 	t.tsdbClients = t.tsdbClients[:0]
 	for _, tenant := range t.tenants {

diff --git a/pkg/receive/multitsdb_test.go b/pkg/receive/multitsdb_test.go
@@ -194,7 +194,7 @@ func TestMultiTSDB(t *testing.T) {
 		testutil.Ok(t, m.Open())
 		testutil.Ok(t, appendSample(m, testTenant, time.Now()))
 
-		tenant := m.getTenant(testTenant)
+		tenant := m.testGetTenant(testTenant)
 		db := tenant.readyStorage().Get()
 
 		testutil.Equals(t, 0, len(db.Blocks()))
@@ -843,7 +843,10 @@ func appendSampleWithLabels(m *MultiTSDB, tenant string, lbls labels.Labels, tim
 
 func queryLabelValues(ctx context.Context, m *MultiTSDB) error {
 	proxy := store.NewProxyStore(nil, nil, func() []store.Client {
-		clients := m.tsdbLocalClientsCopied()
+		m.mtx.Lock()
+		defer m.mtx.Unlock()
+		clients := make([]store.Client, len(m.tsdbClients))
+		copy(clients, m.tsdbClients)
 		if len(clients) > 0 {
 			clients[0] = &slowClient{clients[0]}
 		}

diff --git a/pkg/receive/receive_test.go b/pkg/receive/receive_test.go
@@ -210,7 +210,7 @@ func TestAddingExternalLabelsForTenants(t *testing.T) {
 
 			for _, c := range tc.cfg {
 				for _, tenantId := range c.Tenants {
-					if m.getTenant(tenantId) == nil {
+					if m.testGetTenant(tenantId) == nil {
 						err = appendSample(m, tenantId, time.Now())
 						require.NoError(t, err)
 					}
@@ -294,7 +294,7 @@ func TestLabelSetsOfTenantsWhenAddingTenants(t *testing.T) {
 
 		for _, c := range initialConfig {
 			for _, tenantId := range c.Tenants {
-				if m.getTenant(tenantId) == nil {
+				if m.testGetTenant(tenantId) == nil {
 					err = appendSample(m, tenantId, time.Now())
 					require.NoError(t, err)
 				}
@@ -319,7 +319,7 @@ func TestLabelSetsOfTenantsWhenAddingTenants(t *testing.T) {
 
 		for _, c := range changedConfig {
 			for _, tenantId := range c.Tenants {
-				if m.getTenant(tenantId) == nil {
+				if m.testGetTenant(tenantId) == nil {
 					err = appendSample(m, tenantId, time.Now())
 					require.NoError(t, err)
 				}
@@ -534,7 +534,7 @@ func TestLabelSetsOfTenantsWhenChangingLabels(t *testing.T) {
 
 			for _, c := range initialConfig {
 				for _, tenantId := range c.Tenants {
-					if m.getTenant(tenantId) == nil {
+					if m.testGetTenant(tenantId) == nil {
 						err = appendSample(m, tenantId, time.Now())
 						require.NoError(t, err)
 					}
@@ -704,7 +704,7 @@ func TestAddingLabelsWhenTenantAppearsInMultipleHashrings(t *testing.T) {
 
 			for _, c := range initialConfig {
 				for _, tenantId := range c.Tenants {
-					if m.getTenant(tenantId) == nil {
+					if m.testGetTenant(tenantId) == nil {
 						err = appendSample(m, tenantId, time.Now())
 						require.NoError(t, err)
 					}
@@ -778,7 +778,7 @@ func TestReceiverLabelsNotOverwrittenByExternalLabels(t *testing.T) {
 
 		for _, c := range cfg {
 			for _, tenantId := range c.Tenants {
-				if m.getTenant(tenantId) == nil {
+				if m.testGetTenant(tenantId) == nil {
 					err = appendSample(m, tenantId, time.Now())
 					require.NoError(t, err)
 				}