From 9fa23cc537be4f6cb7b48152f7c0cdde4970382e Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Mon, 17 May 2021 01:35:28 +0200 Subject: [PATCH] reset image cleanup TTL on Submit Also: - make commitTTL equal to EditDuration, so that image is committed to permanent storage after comment can no longer be edited - move cleanupTTL to Cleanup function, as it's not used elsewhere in the code - add variables to some tests sleeps, so that instead of being magic numbers they would rely on timers of structures they suppose to wait for --- backend/app/store/image/image.go | 10 ++++++++-- backend/app/store/image/image_test.go | 9 ++++++++- backend/app/store/service/service_test.go | 7 +++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 9632fdde..b7e10669 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -134,6 +134,12 @@ func (s *Service) Submit(idsFn func() []string) { atomic.AddInt32(&s.submitCount, 1) + // reset cleanup timer before submitting the images + // to prevent them from being cleaned up while waiting for EditDuration to expire + for _, imgID := range idsFn() { + _ = s.store.ResetCleanupTimer(imgID) + } + s.submitCh <- submitReq{idsFn: idsFn, TS: time.Now()} } @@ -175,9 +181,9 @@ func (s *Service) ExtractPictures(commentHTML string) (ids []string) { return ids } -// Cleanup runs periodic cleanup with 2.5*ServiceParams.EditDuration. Blocking loop, should be called inside of goroutine by consumer +// Cleanup runs periodic cleanup with 1.5*ServiceParams.EditDuration. Blocking loop, should be called inside of goroutine by consumer func (s *Service) Cleanup(ctx context.Context) { - cleanupTTL := s.EditDuration * 25 / 10 // cleanup images older than 2.5 * EditDuration + cleanupTTL := s.EditDuration * 15 / 10 // cleanup images older than 1.5 * EditDuration log.Printf("[INFO] start pictures cleanup, staging ttl=%v", cleanupTTL) for { diff --git a/backend/app/store/image/image_test.go b/backend/app/store/image/image_test.go index 229819f4..781248f9 100644 --- a/backend/app/store/image/image_test.go +++ b/backend/app/store/image/image_test.go @@ -122,7 +122,7 @@ func TestService_Cleanup(t *testing.T) { svc := NewService(&store, ServiceParams{EditDuration: 20 * time.Millisecond}) // cancel context after 2.1 cleanup TTLs - ctx, cancel := context.WithTimeout(context.Background(), svc.EditDuration / 100 * 25 * 21) + ctx, cancel := context.WithTimeout(context.Background(), svc.EditDuration / 100 * 15 * 21) defer cancel() svc.Cleanup(ctx) store.AssertNumberOfCalls(t, "Cleanup", 2) @@ -131,11 +131,14 @@ func TestService_Cleanup(t *testing.T) { func TestService_Submit(t *testing.T) { store := MockStore{} store.On("Commit", mock.Anything, mock.Anything).Times(7).Return(nil) + store.On("ResetCleanupTimer", mock.Anything, mock.Anything).Times(7).Return(nil) svc := NewService(&store, ServiceParams{ImageAPI: "/blah/", EditDuration: time.Millisecond * 100}) svc.Submit(func() []string { return []string{"id1", "id2", "id3"} }) + store.AssertNumberOfCalls(t, "ResetCleanupTimer", 3) err := svc.SubmitAndCommit(func() []string { return []string{"id4", "id5"} }) assert.NoError(t, err) svc.Submit(func() []string { return []string{"id6", "id7"} }) + store.AssertNumberOfCalls(t, "ResetCleanupTimer", 5) svc.Submit(nil) store.AssertNumberOfCalls(t, "Commit", 2) time.Sleep(time.Millisecond * 175) @@ -146,10 +149,12 @@ func TestService_Submit(t *testing.T) { func TestService_Close(t *testing.T) { store := MockStore{} store.On("Commit", mock.Anything, mock.Anything).Times(5).Return(nil) + store.On("ResetCleanupTimer", mock.Anything, mock.Anything).Times(5).Return(nil) svc := Service{store: &store, ServiceParams: ServiceParams{ImageAPI: "/blah/", EditDuration: time.Hour * 24}} svc.Submit(func() []string { return []string{"id1", "id2", "id3"} }) svc.Submit(func() []string { return []string{"id4", "id5"} }) svc.Submit(nil) + store.AssertNumberOfCalls(t, "ResetCleanupTimer", 5) svc.Close(context.TODO()) store.AssertNumberOfCalls(t, "Commit", 5) } @@ -157,11 +162,13 @@ func TestService_Close(t *testing.T) { func TestService_SubmitDelay(t *testing.T) { store := MockStore{} store.On("Commit", mock.Anything, mock.Anything).Times(5).Return(nil) + store.On("ResetCleanupTimer", mock.Anything, mock.Anything).Times(5).Return(nil) svc := NewService(&store, ServiceParams{EditDuration: 20 * time.Millisecond}) svc.Submit(func() []string { return []string{"id1", "id2", "id3"} }) time.Sleep(150 * time.Millisecond) // let first batch to pass TTL svc.Submit(func() []string { return []string{"id4", "id5"} }) svc.Submit(nil) + store.AssertNumberOfCalls(t, "ResetCleanupTimer", 5) store.AssertNumberOfCalls(t, "Commit", 3) svc.Close(context.TODO()) store.AssertNumberOfCalls(t, "Commit", 5) diff --git a/backend/app/store/service/service_test.go b/backend/app/store/service/service_test.go index e9e32384..fd45ce73 100644 --- a/backend/app/store/service/service_test.go +++ b/backend/app/store/service/service_test.go @@ -1342,6 +1342,8 @@ func TestService_submitImages(t *testing.T) { mockStore := image.MockStore{} mockStore.On("Commit", "dev/pic1.png").Once().Return(nil) mockStore.On("Commit", "dev/pic2.png").Once().Return(nil) + mockStore.On("ResetCleanupTimer", "dev/pic1.png").Once().Return(nil) + mockStore.On("ResetCleanupTimer", "dev/pic2.png").Once().Return(nil) imgSvc := image.NewService(&mockStore, image.ServiceParams{ EditDuration: 50 * time.Millisecond, @@ -1367,6 +1369,7 @@ func TestService_submitImages(t *testing.T) { assert.NoError(t, err) b.submitImages(c) + mockStore.AssertNumberOfCalls(t, "ResetCleanupTimer", 2) time.Sleep(b.EditDuration + 100 * time.Millisecond) mockStore.AssertNumberOfCalls(t, "Commit", 2) } @@ -1385,6 +1388,10 @@ func TestService_ResubmitStagingImages(t *testing.T) { defer teardown() b := DataStore{Engine: eng, EditDuration: 10 * time.Millisecond, ImageService: imgSvc} + mockStore.On("ResetCleanupTimer", "dev_user/bqf122eq9r8ad657n3ng").Once().Return(nil) + mockStore.On("ResetCleanupTimer", "dev_user/bqf321eq9r8ad657n3ng").Once().Return(nil) + mockStore.On("ResetCleanupTimer", "cached_images/12318fbd4c55e9d177b8b5ae197bc89c5afd8e07-a41fcb00643f28d700504256ec81cbf2e1aac53e").Once().Return(nil) + // create comment with three images without preparing it properly comment := store.Comment{ ID: "id-0",