Use part of namespace ID for delete namespace name (#2803)

service/worker/deletenamespace/activities.go

@@ -27,8 +27,6 @@ package deletenamespace
 import (
 	"context"
 	"fmt"
-	"math/rand"
-	"time"
 
 	enumspb "go.temporal.io/api/enums/v1"
 	"go.temporal.io/api/serviceerror"
@@ -123,18 +121,11 @@ func (a *activities) MarkNamespaceDeletedActivity(ctx context.Context, nsName na
 	return nil
 }
 
-func (a *activities) GenerateDeletedNamespaceNameActivity(ctx context.Context, nsName namespace.Name) (namespace.Name, error) {
-	var letters = []rune("1234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
-	const suffixLength = 5
-	rand.Seed(time.Now().UnixNano())
+func (a *activities) GenerateDeletedNamespaceNameActivity(ctx context.Context, nsID namespace.ID, nsName namespace.Name) (namespace.Name, error) {
+	const initialSuffixLength = 5
 
-	for { // Just in case. Generated name should always be unique after first attempt.
-		b := make([]rune, suffixLength)
-		for i := range b {
-			b[i] = letters[rand.Intn(len(letters))]
-		}
-
-		newName := fmt.Sprintf("%s-deleted-%s", nsName, string(b))
+	for suffixLength := initialSuffixLength; suffixLength < len(nsID.String()); suffixLength++ { // Just in case. 5 chars from ID should be good enough.
+		newName := fmt.Sprintf("%s-deleted-%s", nsName, nsID.String()[:suffixLength])
 
 		_, err := a.metadataManager.GetNamespace(ctx, &persistence.GetNamespaceRequest{
 			Name: newName,
@@ -151,6 +142,8 @@ func (a *activities) GenerateDeletedNamespaceNameActivity(ctx context.Context, n
 			return namespace.EmptyName, err
 		}
 	}
+	// Should never get here because namespace ID is guaranteed to be unique.
+	panic(fmt.Sprintf("Unable to generate new name for deleted namespace %s. ID %q is not unique.", nsName, nsID))
 }
 
 func (a *activities) RenameNamespaceActivity(ctx context.Context, previousName namespace.Name, newName namespace.Name) error {

service/worker/deletenamespace/activities_test.go

@@ -0,0 +1,69 @@
+// The MIT License
+//
+// Copyright (c) 2020 Temporal Technologies Inc.  All rights reserved.
+//
+// Copyright (c) 2020 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package deletenamespace
+
+import (
+	"context"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
+	"go.temporal.io/api/serviceerror"
+
+	"go.temporal.io/server/common/log"
+	"go.temporal.io/server/common/metrics"
+	"go.temporal.io/server/common/namespace"
+	"go.temporal.io/server/common/persistence"
+)
+
+func Test_GenerateDeletedNamespaceNameActivity(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	metadataManager := persistence.NewMockMetadataManager(ctrl)
+
+	a := &activities{
+		metadataManager: metadataManager,
+		metricsClient:   metrics.NoopClient,
+		logger:          log.NewNoopLogger(),
+	}
+
+	metadataManager.EXPECT().GetNamespace(gomock.Any(), &persistence.GetNamespaceRequest{
+		Name: "namespace-deleted-names",
+	}).Return(nil, serviceerror.NewNamespaceNotFound("namespace-deleted-names"))
+	deletedName, err := a.GenerateDeletedNamespaceNameActivity(context.Background(), "namespace-id", "namespace")
+	require.NoError(t, err)
+	require.Equal(t, namespace.Name("namespace-deleted-names"), deletedName)
+
+	metadataManager.EXPECT().GetNamespace(gomock.Any(), &persistence.GetNamespaceRequest{
+		Name: "namespace-deleted-names",
+	}).Return(nil, nil)
+	metadataManager.EXPECT().GetNamespace(gomock.Any(), &persistence.GetNamespaceRequest{
+		Name: "namespace-deleted-namesp",
+	}).Return(nil, serviceerror.NewNamespaceNotFound("namespace-deleted-namesp"))
+	deletedName, err = a.GenerateDeletedNamespaceNameActivity(context.Background(), "namespace-id", "namespace")
+	require.NoError(t, err)
+	require.Equal(t, namespace.Name("namespace-deleted-namesp"), deletedName)
+
+	ctrl.Finish()
+}

service/worker/deletenamespace/deleteexecutions/workflow_test.go

@@ -26,7 +26,6 @@ package deleteexecutions
 
 import (
 	"context"
-	"sync/atomic"
 	"testing"
 
 	"github.com/golang/mock/gomock"
@@ -130,36 +129,32 @@ func Test_DeleteExecutionsWorkflow_ManyExecutions_NoContinueAsNew(t *testing.T)
 
 	var a *Activities
 
-	var pageNumber atomic.Value
-	pageNumber.Store(0)
-
+	pageNumber := 0
 	env.OnActivity(a.GetNextPageTokenActivity, mock.Anything, mock.Anything).Return(func(_ context.Context, params GetNextPageTokenParams) ([]byte, error) {
 		require.Equal(t, namespace.Name("namespace"), params.Namespace)
 		require.Equal(t, namespace.ID("namespace-id"), params.NamespaceID)
 		require.Equal(t, 3, params.PageSize)
-		pn := pageNumber.Load().(int)
-		if pn == 0 {
+		if pageNumber == 0 {
 			require.Nil(t, params.NextPageToken)
 		} else {
 			require.Equal(t, []byte{3, 22, 83}, params.NextPageToken)
 		}
-
-		pn++
-		pageNumber.Store(pn)
-		if pn == 100 { // Emulate 100 pages of executions.
+		pageNumber++
+		if pageNumber == 100 { // Emulate 100 pages of executions.
 			return nil, nil
 		}
 		return []byte{3, 22, 83}, nil
 	}).Times(100)
 
+	nilTokenOnce := false
 	env.OnActivity(a.DeleteExecutionsActivity, mock.Anything, mock.Anything).Return(func(_ context.Context, params DeleteExecutionsActivityParams) (DeleteExecutionsActivityResult, error) {
 		require.Equal(t, namespace.Name("namespace"), params.Namespace)
 		require.Equal(t, namespace.ID("namespace-id"), params.NamespaceID)
 		require.Equal(t, 100, params.RPS)
 		require.Equal(t, 3, params.ListPageSize)
-		if pageNumber.Load().(int) == 0 {
-			require.Nil(t, params.NextPageToken)
-		} else {
+		if params.NextPageToken == nil {
+			nilTokenOnce = true
+		} else if nilTokenOnce {
 			require.Equal(t, []byte{3, 22, 83}, params.NextPageToken)
 		}
 

service/worker/deletenamespace/workflow.go

@@ -124,7 +124,7 @@ func DeleteNamespaceWorkflow(ctx workflow.Context, params DeleteNamespaceWorkflo
 
 	// Step 3. Rename namespace.
 	ctx3 := workflow.WithLocalActivityOptions(ctx, localActivityOptions)
-	err = workflow.ExecuteLocalActivity(ctx3, a.GenerateDeletedNamespaceNameActivity, params.Namespace).Get(ctx, &result.DeletedNamespace)
+	err = workflow.ExecuteLocalActivity(ctx3, a.GenerateDeletedNamespaceNameActivity, params.NamespaceID, params.Namespace).Get(ctx, &result.DeletedNamespace)
 	if err != nil {
 		return result, fmt.Errorf("%w: GenerateDeletedNamespaceNameActivity: %v", errors.ErrUnableToExecuteActivity, err)
 	}

service/worker/deletenamespace/workflow_test.go

@@ -49,7 +49,7 @@ func Test_DeleteNamespaceWorkflow_ByName(t *testing.T) {
 			Namespace:   "namespace",
 		}, nil).Once()
 	env.OnActivity(a.MarkNamespaceDeletedActivity, mock.Anything, namespace.Name("namespace")).Return(nil).Once()
-	env.OnActivity(a.GenerateDeletedNamespaceNameActivity, mock.Anything, namespace.Name("namespace")).Return(namespace.Name("namespace-delete-220878"), nil).Once()
+	env.OnActivity(a.GenerateDeletedNamespaceNameActivity, mock.Anything, namespace.ID("namespace-id"), namespace.Name("namespace")).Return(namespace.Name("namespace-delete-220878"), nil).Once()
 	env.OnActivity(a.RenameNamespaceActivity, mock.Anything, namespace.Name("namespace"), namespace.Name("namespace-delete-220878")).Return(nil).Once()
 
 	env.RegisterWorkflow(reclaimresources.ReclaimResourcesWorkflow)
@@ -95,7 +95,7 @@ func Test_DeleteNamespaceWorkflow_ByID(t *testing.T) {
 			Namespace:   "namespace",
 		}, nil).Once()
 	env.OnActivity(a.MarkNamespaceDeletedActivity, mock.Anything, namespace.Name("namespace")).Return(nil).Once()
-	env.OnActivity(a.GenerateDeletedNamespaceNameActivity, mock.Anything, namespace.Name("namespace")).Return(namespace.Name("namespace-delete-220878"), nil).Once()
+	env.OnActivity(a.GenerateDeletedNamespaceNameActivity, mock.Anything, namespace.ID("namespace-id"), namespace.Name("namespace")).Return(namespace.Name("namespace-delete-220878"), nil).Once()
 	env.OnActivity(a.RenameNamespaceActivity, mock.Anything, namespace.Name("namespace"), namespace.Name("namespace-delete-220878")).Return(nil).Once()
 
 	env.RegisterWorkflow(reclaimresources.ReclaimResourcesWorkflow)