Skip to content

Commit 4c96033

Browse files
Improve cluster metadata and metadata persistence tests (temporalio#7249)
## What changed? Improved persistence tests to account for differing behavior of some persistence backends. ## Why? These tests made assumptions about persistence that don't necessarily apply to all persistence backends. Specifically the tests assumed: - All persistence backends return a final empty page when paginating results - All persistence backends are guaranteed to expire TTL'd records (nearly) instantly upon expiration. ## How did you test it? Ran the tests with various persistence backends ## Potential risks This is a test-only update ## Documentation No changes. ## Is hotfix candidate? No
1 parent 833193c commit 4c96033

File tree

2 files changed

+44
-33
lines changed

2 files changed

+44
-33
lines changed

common/persistence/persistence-tests/cluster_metadata_manager.go

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ func (s *ClusterMetadataManagerSuite) SetupTest() {
6565

6666
// TearDownTest implementation
6767
func (s *ClusterMetadataManagerSuite) TearDownTest() {
68+
// Ensure all tests clean up after themselves
69+
// Todo: MetaMgr should provide api to clear all members
70+
s.waitForPrune(1 * time.Second)
6871
s.cancel()
6972
}
7073

@@ -100,16 +103,12 @@ func (s *ClusterMetadataManagerSuite) TestClusterMembershipUpsertCanReadAny() {
100103
s.Nil(err)
101104
s.NotNil(resp)
102105
s.NotEmpty(resp.ActiveMembers)
106+
107+
s.waitForPrune(5 * time.Second)
103108
}
104109

105110
// TestClusterMembershipUpsertCanRead verifies that we can UpsertClusterMembership and read our result
106111
func (s *ClusterMetadataManagerSuite) TestClusterMembershipUpsertCanPageRead() {
107-
// Expire previous records
108-
// Todo: MetaMgr should provide api to clear all members
109-
time.Sleep(time.Second * 3)
110-
err := s.ClusterMetadataManager.PruneClusterMembership(s.ctx, &p.PruneClusterMembershipRequest{MaxRecordsPruned: 100})
111-
s.Nil(err)
112-
113112
expectedIds := make(map[string]int, 100)
114113
for i := 0; i < 100; i++ {
115114
hostID := primitives.NewUUID().Downcast()
@@ -148,9 +147,7 @@ func (s *ClusterMetadataManagerSuite) TestClusterMembershipUpsertCanPageRead() {
148147
s.Zero(val, "identifier was either not found in db, or shouldn't be there - "+id)
149148
}
150149

151-
time.Sleep(time.Second * 3)
152-
err = s.ClusterMetadataManager.PruneClusterMembership(s.ctx, &p.PruneClusterMembershipRequest{MaxRecordsPruned: 1000})
153-
s.NoError(err)
150+
s.waitForPrune(5 * time.Second)
154151
}
155152

156153
func (s *ClusterMetadataManagerSuite) validateUpsert(req *p.UpsertClusterMembershipRequest, resp *p.GetClusterMembersResponse, err error) {
@@ -223,10 +220,7 @@ func (s *ClusterMetadataManagerSuite) TestClusterMembershipReadFiltersCorrectly(
223220
)
224221

225222
s.validateUpsert(req, resp, err)
226-
227-
time.Sleep(time.Second * 3)
228-
err = s.ClusterMetadataManager.PruneClusterMembership(s.ctx, &p.PruneClusterMembershipRequest{MaxRecordsPruned: 1000})
229-
s.NoError(err)
223+
s.waitForPrune(5 * time.Second)
230224
}
231225

232226
// TestClusterMembershipUpsertExpiresCorrectly verifies RecordExpiry functions properly for ClusterMembership records
@@ -263,19 +257,27 @@ func (s *ClusterMetadataManagerSuite) TestClusterMembershipUpsertExpiresCorrectl
263257
s.Equal(resp.ActiveMembers[0].HostID, req.HostID)
264258
s.Equal(resp.ActiveMembers[0].Role, req.Role)
265259

266-
time.Sleep(time.Second * 2)
267-
268-
err = s.ClusterMetadataManager.PruneClusterMembership(s.ctx, &p.PruneClusterMembershipRequest{MaxRecordsPruned: 100})
269-
s.Nil(err)
260+
s.waitForPrune(5 * time.Second)
261+
}
270262

271-
resp, err = s.ClusterMetadataManager.GetClusterMembers(
272-
s.ctx,
273-
&p.GetClusterMembersRequest{LastHeartbeatWithin: time.Minute * 10},
274-
)
263+
// waitForPrune waits up for the persistence backend to prune all records. Some persistence backends
264+
// may not remove TTL'd entries at the exact instant they should expire, so we allow some timing flexibility here.
265+
func (s *ClusterMetadataManagerSuite) waitForPrune(waitFor time.Duration) {
266+
s.Eventually(func() bool {
267+
err := s.ClusterMetadataManager.PruneClusterMembership(s.ctx, &p.PruneClusterMembershipRequest{MaxRecordsPruned: 100})
268+
s.Nil(err)
269+
270+
resp, err := s.ClusterMetadataManager.GetClusterMembers(
271+
s.ctx,
272+
&p.GetClusterMembersRequest{LastHeartbeatWithin: time.Minute * 10},
273+
)
274+
s.NoError(err)
275+
s.NotNil(resp)
276+
return len(resp.ActiveMembers) == 0
275277

276-
s.Nil(err)
277-
s.NotNil(resp)
278-
s.Empty(resp.ActiveMembers)
278+
},
279+
waitFor,
280+
500*time.Millisecond)
279281
}
280282

281283
// TestClusterMembershipUpsertInvalidExpiry verifies we cannot specify a non-positive RecordExpiry duration

common/persistence/persistence-tests/metadata_persistence_v2.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,14 +1334,17 @@ func (m *MetadataPersistenceSuiteV2) TestListNamespaces() {
13341334
// so we can test == easily
13351335
namespace.NotificationVersion = 0
13361336
}
1337-
pageCount++
1337+
// Some persistence backends return an unavoidable empty final page.
1338+
if len(resp.Namespaces) > 0 {
1339+
pageCount++
1340+
}
13381341
if len(token) == 0 {
13391342
break
13401343
}
13411344
}
13421345

1343-
// 2 pages with data and 1 empty page which is unavoidable.
1344-
m.Equal(pageCount, 3)
1346+
// There should be 2 non-empty pages.
1347+
m.Equal(pageCount, 2)
13451348
m.Equal(len(inputNamespaces), len(outputNamespaces))
13461349
for _, namespace := range inputNamespaces {
13471350
m.DeepEqual(namespace, outputNamespaces[namespace.Namespace.Info.Id])
@@ -1415,14 +1418,17 @@ func (m *MetadataPersistenceSuiteV2) TestListNamespaces_DeletedNamespace() {
14151418
m.NoError(err)
14161419
token = resp.NextPageToken
14171420
listNamespacesPageSize2 = append(listNamespacesPageSize2, resp.Namespaces...)
1418-
pageCount++
1421+
// Some persistence backends return an unavoidable empty final page.
1422+
if len(resp.Namespaces) > 0 {
1423+
pageCount++
1424+
}
14191425
if len(token) == 0 {
14201426
break
14211427
}
14221428
}
14231429

1424-
// 1 page with data and 1 empty page which is unavoidable.
1425-
m.Equal(2, pageCount)
1430+
// There should be 1 non-empty page.
1431+
m.Equal(1, pageCount)
14261432
m.Len(listNamespacesPageSize2, 2)
14271433
for _, namespace := range listNamespacesPageSize2 {
14281434
m.NotEqual(namespace.Namespace.Info.State, enumspb.NAMESPACE_STATE_DELETED)
@@ -1435,14 +1441,17 @@ func (m *MetadataPersistenceSuiteV2) TestListNamespaces_DeletedNamespace() {
14351441
m.NoError(err)
14361442
token = resp.NextPageToken
14371443
listNamespacesPageSize1 = append(listNamespacesPageSize1, resp.Namespaces...)
1438-
pageCount++
1444+
// Some persistence backends return an unavoidable empty final page.
1445+
if len(resp.Namespaces) > 0 {
1446+
pageCount++
1447+
}
14391448
if len(token) == 0 {
14401449
break
14411450
}
14421451
}
14431452

1444-
// 2 pages with data and 1 empty page which is unavoidable.
1445-
m.Equal(3, pageCount)
1453+
// There should be 2 non-empty pages.
1454+
m.Equal(2, pageCount)
14461455
m.Len(listNamespacesPageSize1, 2)
14471456
for _, namespace := range listNamespacesPageSize1 {
14481457
m.NotEqual(namespace.Namespace.Info.State, enumspb.NAMESPACE_STATE_DELETED)

0 commit comments

Comments
 (0)