Comment reflection, large-scale editing

cmd/yorkie/server.go

@@ -39,8 +39,8 @@ var (
 	flagLogLevel string
 
 	adminTokenDuration                       time.Duration
-	housekeepingIntervalDeactivateCandidates time.Duration
-	housekeepingIntervalDeleteDocuments      time.Duration
+	housekeepingDeactivateCandidatesInterval time.Duration
+	housekeepingDeleteDocumentsInterval      time.Duration
 	documentHardDeletionGracefulPeriod       time.Duration
 	clientDeactivateThreshold                string
 
@@ -71,8 +71,8 @@ func newServerCmd() *cobra.Command {
 			conf.Backend.AuthWebhookCacheUnauthTTL = authWebhookCacheUnauthTTL.String()
 			conf.Backend.ProjectInfoCacheTTL = projectInfoCacheTTL.String()
 
-			conf.Housekeeping.IntervalDeactivateCandidates = housekeepingIntervalDeactivateCandidates.String()
-			conf.Housekeeping.IntervalDeleteDocuments = housekeepingIntervalDeleteDocuments.String()
+			conf.Housekeeping.DeactivateCandidatesInterval = housekeepingDeactivateCandidatesInterval.String()
+			conf.Housekeeping.DeleteDocumentsInterval = housekeepingDeleteDocumentsInterval.String()
 			conf.Housekeeping.DocumentHardDeletionGracefulPeriod = documentHardDeletionGracefulPeriod
 
 			if mongoConnectionURI != "" {
@@ -198,15 +198,15 @@ func init() {
 		"Enable runtime profiling data via HTTP server.",
 	)
 	cmd.Flags().StringVar(
-		&conf.Housekeeping.IntervalDeactivateCandidates,
+		&conf.Housekeeping.DeactivateCandidatesInterval,
 		"housekeeping-interval-Deactivate-Candidates",
-		server.DefaultHousekeepingIntervalDeactivateCandidates.String(),
+		server.DefaultHousekeepingDeactivateCandidatesInterval.String(),
 		"housekeeping Interval deactivate candidates between housekeeping runs",
 	)
 	cmd.Flags().StringVar(
-		&conf.Housekeeping.IntervalDeleteDocuments,
+		&conf.Housekeeping.DeleteDocumentsInterval,
 		"housekeeping-interval-Delete-Documents",
-		server.DefaultHousekeepingIntervalDeleteDocuments.String(),
+		server.DefaultHousekeepingDeleteDocumentsInterval.String(),
 		"housekeeping Interval delete documents between housekeeping runs",
 	)
 	cmd.Flags().DurationVar(

server/backend/database/database.go

@@ -172,8 +172,8 @@ type Database interface {
 		documentHardDeletionGracefulPeriod gotime.Duration,
 	) ([]*DocInfo, error)
 
-	// DeleteDocument Document complete deletion in progress
-	DeleteDocument(
+	// DeleteDocuments deletes document of the given key
+	DeleteDocuments(
 		ctx context.Context,
 		candidates []*DocInfo,
 	) (int64, error)

server/backend/database/memory/database.go

@@ -680,30 +680,30 @@ func (d *DB) FindDocumentHardDeletionCandidatesPerProject(
 	candidatesLimit int,
 	documentHardDeletionGracefulPeriod gotime.Duration,
 ) ([]*database.DocInfo, error) {
-
 	txn := d.db.Txn(false)
 	defer txn.Abort()
 
+	offset := gotime.Now().Add(-documentHardDeletionGracefulPeriod)
+
 	var documents []*database.DocInfo
-	iterator, err := txn.Get(
+	iterator, err := txn.ReverseLowerBound(
 		tblDocuments,
-		"project_id",
+		"project_id_removed_at",
 		project.ID.String(),
+		offset,
 	)
 
 	if err != nil {
 		return nil, fmt.Errorf("fetch hard deletion candidates: %w", err)
 	}
 
-	currentTime := gotime.Now()
-	conditionDocumentHardDeletionGracefulPeriod := currentTime.Add(-documentHardDeletionGracefulPeriod)
 	for raw := iterator.Next(); raw != nil; raw = iterator.Next() {
 		document := raw.(*database.DocInfo)
 		if candidatesLimit <= len(documents) && candidatesLimit != 0 {
 			break
 		}
 
-		if !document.RemovedAt.After(conditionDocumentHardDeletionGracefulPeriod) {
+		if !document.RemovedAt.After(offset) {
 			documents = append(documents, document)
 		}
 	}
@@ -755,8 +755,8 @@ func (d *DB) FindDeactivateCandidatesPerProject(
 	return infos, nil
 }
 
-// DeleteDocument Deletes the documents completely.
-func (d *DB) DeleteDocument(
+// DeleteDocuments Deletes the documents completely.
+func (d *DB) DeleteDocuments(
 	_ context.Context,
 	candidates []*database.DocInfo,
 ) (int64, error) {

server/backend/database/memory/indexes.go

@@ -117,11 +117,6 @@ var schema = &memdb.DBSchema{
 					Unique:  true,
 					Indexer: &memdb.StringFieldIndex{Field: "ID"},
 				},
-				"project_id": {
-					Name:    "project_id",
-					Unique:  true,
-					Indexer: &memdb.StringFieldIndex{Field: "ProjectID"},
-				},
 				"project_id_id": {
 					Name:   "project_id_id",
 					Unique: true,
@@ -141,6 +136,15 @@ var schema = &memdb.DBSchema{
 						},
 					},
 				},
+				"project_id_removed_at": {
+					Name: "project_id_removed_at",
+					Indexer: &memdb.CompoundIndex{
+						Indexes: []memdb.Indexer{
+							&memdb.StringFieldIndex{Field: "ProjectID"},
+							&memdb.TimeFieldIndex{Field: "RemovedAt"},
+						},
+					},
+				},
 				"project_id_key_removed_at": {
 					Name: "project_id_key_removed_at",
 					Indexer: &memdb.CompoundIndex{

server/backend/database/mongo/client.go

@@ -227,8 +227,8 @@ func (c *Client) CreateProjectInfo(
 	return info, nil
 }
 
-// DeleteDocument Deletes the documents completely.
-func (c *Client) DeleteDocument(
+// DeleteDocuments Deletes the documents completely.
+func (c *Client) DeleteDocuments(
 	ctx context.Context,
 	candidates []*database.DocInfo,
 ) (int64, error) {
@@ -766,37 +766,6 @@ func (c *Client) FindDeactivateCandidatesPerProject(
 	return clientInfos, nil
 }
 
-// FindDeactivateCandidates finds the clients that need housekeeping.
-func (c *Client) FindDeactivateCandidates(
-	ctx context.Context,
-	documentHardDeletionCandidateLimitPerProject int,
-	projectFetchSize int,
-	lastProjectID types.ID,
-) (types.ID, []*database.ClientInfo, error) {
-	projects, err := c.FindNextNCyclingProjectInfos(ctx, projectFetchSize, lastProjectID)
-	if err != nil {
-		return database.DefaultProjectID, nil, fmt.Errorf("failed to find cycling project infos: %w", err)
-	}
-
-	var candidates []*database.ClientInfo
-	for _, project := range projects {
-		clientInfos, err := c.FindDeactivateCandidatesPerProject(ctx, project, documentHardDeletionCandidateLimitPerProject)
-		if err != nil {
-			return database.DefaultProjectID, nil, err
-		}
-
-		candidates = append(candidates, clientInfos...)
-	}
-
-	var topProjectID types.ID
-	if len(projects) < projectFetchSize {
-		topProjectID = database.DefaultProjectID
-	} else {
-		topProjectID = projects[len(projects)-1].ID
-	}
-	return topProjectID, candidates, nil
-}
-
 // FindDocInfoByKeyAndOwner finds the document of the given key. If the
 // createDocIfNotExist condition is true, create the document if it does not
 // exist.

server/backend/database/testcases/testcases.go

@@ -1411,51 +1411,56 @@ func AssertKeys(t *testing.T, expectedKeys []key.Key, infos []*database.DocInfo)
 	assert.EqualValues(t, expectedKeys, keys)
 }
 
-// RunDocumentHardDeletionTest runs Delete document permanently
+// RunDocumentHardDeletionTest runs the DocumentHardDeletion tests for the given db
 func RunDocumentHardDeletionTest(t *testing.T, db database.Database) {
 	t.Run("housekeeping DocumentHardDeletion test", func(t *testing.T) {
 		ctx := context.Background()
+		docKey := helper.TestDocKey(t)
+
+		// 00. Create a project
 		projectInfo, err := db.CreateProjectInfo(ctx, t.Name(), dummyOwnerID, clientDeactivateThreshold)
 		assert.NoError(t, err)
-		projectID := projectInfo.ID
 
-		// Create a client and two documents
-		clientInfo, err := db.ActivateClient(ctx, projectID, t.Name())
+		// 01. Create a client and a document then attach the document to the client.
+		clientInfo, err := db.ActivateClient(ctx, projectInfo.ID, t.Name())
 		assert.NoError(t, err)
-		docInfo, err := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), helper.TestDocKey(t), true)
+		docInfo, err := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), docKey, true)
 		assert.NoError(t, err)
+		docRefKey := docInfo.RefKey()
 		assert.NoError(t, clientInfo.AttachDocument(docInfo.ID, false))
 		assert.NoError(t, db.UpdateClientInfoAfterPushPull(ctx, clientInfo, docInfo))
 
-		assert.NoError(t, clientInfo.RemoveDocument(docInfo.ID))
-
 		doc := document.New(key.Key(t.Name()))
 		pack := doc.CreateChangePack()
-		err = db.CreateChangeInfos(ctx, projectID, docInfo, 0, pack.Changes, true)
-		assert.NoError(t, err)
 
-		fetchSize := 100
-		lastProjectID := database.DefaultProjectID
+		// 02. Set removed_at in docInfo and store changes
+		assert.NoError(t, clientInfo.RemoveDocument(docInfo.ID))
+		err = db.CreateChangeInfos(ctx, projectInfo.ID, docInfo, 0, pack.Changes, true)
+		assert.NoError(t, err)
 
+		// 03. Set the grace period to 0 seconds.
 		var candidates []*database.DocInfo
-		GracePeriod := "0s"
+		GracePeriod := "-1s"
 		documentHardDeletionGracefulPeriod, err := gotime.ParseDuration(GracePeriod)
 		assert.NoError(t, err)
 
+		// 04. Find documents whose deleted_at time is less than or equal to current time minus GracePeriod.
+		fetchSize := 100
 		candidates, err = db.FindDocumentHardDeletionCandidatesPerProject(
 			ctx,
 			projectInfo,
 			fetchSize,
 			documentHardDeletionGracefulPeriod,
 		)
 		assert.NoError(t, err)
-		assert.Equal(t, database.DefaultProjectID, lastProjectID)
 
-		deletedDocumentsCount, err := db.DeleteDocument(ctx, candidates)
+		// 05. Deletes document of the given key
+		// Compare the number of candidates for deletion with the number of deleted documents.
+		deletedDocumentsCount, err := db.DeleteDocuments(ctx, candidates)
 		assert.NoError(t, err)
 		assert.Equal(t, int(deletedDocumentsCount), len(candidates))
 
-		_, err = db.FindDocInfoByRefKey(ctx, docInfo.RefKey())
+		_, err = db.FindDocInfoByRefKey(ctx, docRefKey)
 		assert.ErrorIs(t, err, database.ErrDocumentNotFound)
 
 	})

server/backend/housekeeping/config.go

@@ -25,11 +25,11 @@ import (
 
 // Config is the configuration for the housekeeping service.
 type Config struct {
-	// IntervalDeactivateCandidates is the time between housekeeping runs for deactivate candidates.
-	IntervalDeactivateCandidates string `yaml:"IntervalDeactivateCandidates"`
+	// DeactivateCandidatesInterval is the time between housekeeping runs for deactivate candidates.
+	DeactivateCandidatesInterval string `yaml:"DeactivateCandidatesInterval"`
 
-	// IntervalDeleteDocuments is the time between housekeeping runs for document deletion.
-	IntervalDeleteDocuments string `yaml:"IntervalDeleteDocuments"`
+	// DeleteDocumentsInterval is the time between housekeeping runs for document deletion.
+	DeleteDocumentsInterval string `yaml:"DeleteDocumentsInterval"`
 
 	// DocumentHardDeletionGracefulPeriod finds documents whose removed_at time is older than that time.
 	DocumentHardDeletionGracefulPeriod time.Duration `yaml:"HousekeepingDocumentHardDeletionGracefulPeriod"`
@@ -46,18 +46,18 @@ type Config struct {
 
 // Validate validates the configuration.
 func (c *Config) Validate() error {
-	if _, err := time.ParseDuration(c.IntervalDeactivateCandidates); err != nil {
+	if _, err := time.ParseDuration(c.DeactivateCandidatesInterval); err != nil {
 		return fmt.Errorf(
 			`invalid argument %s for "--housekeeping-interval-deactivate-candidates" flag: %w`,
-			c.IntervalDeactivateCandidates,
+			c.DeactivateCandidatesInterval,
 			err,
 		)
 	}
 
-	if _, err := time.ParseDuration(c.IntervalDeleteDocuments); err != nil {
+	if _, err := time.ParseDuration(c.DeleteDocumentsInterval); err != nil {
 		return fmt.Errorf(
 			`invalid argument %s for "--housekeeping-interval-delete-documents" flag: %w`,
-			c.IntervalDeleteDocuments,
+			c.DeleteDocumentsInterval,
 			err,
 		)
 	}

server/backend/housekeeping/config_test.go

@@ -27,8 +27,8 @@ import (
 func TestConfig(t *testing.T) {
 	t.Run("validate test", func(t *testing.T) {
 		validConf := housekeeping.Config{
-			IntervalDeactivateCandidates:                 "1m",
-			IntervalDeleteDocuments:                      "1m",
+			DeactivateCandidatesInterval:                 "1m",
+			DeleteDocumentsInterval:                      "1m",
 			DocumentHardDeletionGracefulPeriod:           60,
 			ClientDeactivationCandidateLimitPerProject:   100,
 			DocumentHardDeletionCandidateLimitPerProject: 100,
@@ -37,11 +37,11 @@ func TestConfig(t *testing.T) {
 		assert.NoError(t, validConf.Validate())
 
 		conf1 := validConf
-		conf1.IntervalDeactivateCandidates = "hour"
+		conf1.DeactivateCandidatesInterval = "hour"
 		assert.Error(t, conf1.Validate())
 
 		conf2 := validConf
-		conf2.IntervalDeleteDocuments = "minute"
+		conf2.DeleteDocumentsInterval = "minute"
 		assert.Error(t, conf2.Validate())
 
 		conf3 := validConf

server/backend/housekeeping/housekeeping.go

@@ -48,33 +48,19 @@ func New(conf *Config) (*Housekeeping, error) {
 
 // RegisterTask registers task the housekeeping service.
 func (h *Housekeeping) RegisterTask(
-	intervalDeactivateCandidates time.Duration,
-	intervalDeleteDocuments time.Duration,
-	taskDeactivateCandidates func(ctx context.Context) error,
-	taskDeleteDocuments func(ctx context.Context) error,
+	interval time.Duration,
+	task func(ctx context.Context) error,
 ) error {
 	if _, err := h.scheduler.NewJob(
-		gocron.DurationJob(intervalDeactivateCandidates),
+		gocron.DurationJob(interval),
 		gocron.NewTask(func() {
 			ctx := context.Background()
-			if err := taskDeactivateCandidates(ctx); err != nil {
+			if err := task(ctx); err != nil {
 				logging.From(ctx).Error(err)
 			}
 		}),
 	); err != nil {
-		return fmt.Errorf("scheduler new job for deactivating candidates: %w", err)
-	}
-
-	if _, err := h.scheduler.NewJob(
-		gocron.DurationJob(intervalDeleteDocuments),
-		gocron.NewTask(func() {
-			ctx := context.Background()
-			if err := taskDeleteDocuments(ctx); err != nil {
-				logging.From(ctx).Error(err)
-			}
-		}),
-	); err != nil {
-		return fmt.Errorf("scheduler new job for deleting documents: %w", err)
+		return fmt.Errorf("scheduler new job: %w", err)
 	}
 
 	return nil