From 5ae6dcf65207e492d762eab74da2031ed3b935d6 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Thu, 23 Apr 2020 00:54:30 +0200 Subject: [PATCH] introduce image.Service commitTTL and cleanupTTL properties This allows having separate values of TTL for Commit and Cleanup and moving them apart in time, also clarifying their connection to EditTime which was previously outside of the package level. --- backend/app/cmd/server.go | 12 ++++---- backend/app/rest/api/rest_private_test.go | 4 +-- backend/app/rest/api/rest_test.go | 4 +-- backend/app/store/image/image.go | 34 +++++++++++++++-------- backend/app/store/image/image_test.go | 8 +++--- backend/app/store/service/service_test.go | 5 ++-- 6 files changed, 39 insertions(+), 28 deletions(-) diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index 54d0fb7c..949fe889 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -338,7 +338,7 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) { if err != nil { return nil, errors.Wrap(err, "failed to make pictures store") } - log.Printf("[DEBUG] image service for url=%s, ttl=%v", imageService.ImageAPI, imageService.TTL) + log.Printf("[DEBUG] image service for url=%s, EditDuration=%v", imageService.ImageAPI, imageService.EditDuration) dataService := &service.DataStore{ Engine: storeEngine, @@ -580,11 +580,11 @@ func (s *ServerCommand) makeAvatarStore() (avatar.Store, error) { func (s *ServerCommand) makePicturesStore() (*image.Service, error) { imageServiceParams := image.ServiceParams{ - ImageAPI: s.RemarkURL + "/api/v1/picture/", - TTL: 5 * s.EditDuration, // add extra time to image TTL for staging - MaxSize: s.Image.MaxSize, - MaxHeight: s.Image.ResizeHeight, - MaxWidth: s.Image.ResizeWidth, + ImageAPI: s.RemarkURL + "/api/v1/picture/", + EditDuration: s.EditDuration, + MaxSize: s.Image.MaxSize, + MaxHeight: s.Image.ResizeHeight, + MaxWidth: s.Image.ResizeWidth, } switch s.Image.Type { case "bolt": diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 39d45189..729c6d05 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -908,8 +908,8 @@ func TestRest_CreateWithPictures(t *testing.T) { Staging: "/tmp/remark42/images.staging", Location: "/tmp/remark42/images", }, image.ServiceParams{ - TTL: 100 * time.Millisecond, - MaxSize: 2000, + EditDuration: 100 * time.Millisecond, + MaxSize: 2000, }) svc.privRest.imageService = imageService diff --git a/backend/app/rest/api/rest_test.go b/backend/app/rest/api/rest_test.go index cf1f2c1d..2b4b5366 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -376,8 +376,8 @@ func startupT(t *testing.T) (ts *httptest.Server, srv *Rest, teardown func()) { Partitions: 100, Staging: tmp + "/pics-remark42/staging", }, image.ServiceParams{ - TTL: 100 * time.Millisecond, - MaxSize: 10000, + EditDuration: 100 * time.Millisecond, + MaxSize: 10000, }), ImageProxy: &proxy.Image{}, ReadOnlyAge: 10, diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index e88eb997..2f5cc7b9 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -31,7 +31,7 @@ import ( // Service wraps Store with common functions needed for any store implementation // It also provides async Submit with func param retrieving all submitting ids. -// Submitted ids committed (i.e. moved from staging to final) on TTL expiration. +// Submitted ids committed (i.e. moved from staging to final) on commitTTL expiration. type Service struct { ServiceParams @@ -45,11 +45,17 @@ type Service struct { // ServiceParams contains externally adjustable parameters of Service type ServiceParams struct { - TTL time.Duration // for how long file allowed on staging - ImageAPI string // image api matching path - MaxSize int - MaxHeight int - MaxWidth int + EditDuration time.Duration // edit period for comments + ImageAPI string // image api matching path + MaxSize int + MaxHeight int + MaxWidth int + + // duration of time after which images are checked and committed if still + // present in the submitted comment after it's EditDuration is expired + commitTTL time.Duration + // duration of time after which images are deleted from staging + cleanupTTL time.Duration } // StoreInfo contains image store meta information @@ -82,6 +88,10 @@ type submitReq struct { // NewService returns new Service instance func NewService(s Store, p ServiceParams) *Service { + p.commitTTL = p.EditDuration * 15 / 10 // Commit call on every 1.5 * EditDuration + p.cleanupTTL = p.EditDuration * 25 / 10 // Cleanup call on every 2.5 * EditDuration + // In case Cleanup and Submit start at the same time (case of stale staging images check + // on the program start) these TTL values guarantee that Commit will happen before Cleanup. return &Service{ServiceParams: p, store: s} } @@ -98,8 +108,8 @@ func (s *Service) Submit(idsFn func() []string) { go func() { defer s.wg.Done() for req := range s.submitCh { - // wait for TTL expiration with emergency pass on term - for atomic.LoadInt32(&s.term) == 0 && time.Since(req.TS) <= s.TTL/2 { // commit on a half of TTL + // wait for commitTTL expiration with emergency pass on term + for atomic.LoadInt32(&s.term) == 0 && time.Since(req.TS) <= s.commitTTL { time.Sleep(time.Millisecond * 10) // small sleep to relive busy wait but keep reactive for term (close) } for _, id := range req.idsFn() { @@ -139,17 +149,17 @@ func (s *Service) ExtractPictures(commentHTML string) (ids []string, err error) return ids, nil } -// Cleanup runs periodic cleanup with TTL. Blocking loop, should be called inside of goroutine by consumer +// Cleanup runs periodic cleanup with cleanupTTL. Blocking loop, should be called inside of goroutine by consumer func (s *Service) Cleanup(ctx context.Context) { - log.Printf("[INFO] start pictures cleanup, staging ttl=%v", s.TTL) + log.Printf("[INFO] start pictures cleanup, staging ttl=%v", s.cleanupTTL) for { select { case <-ctx.Done(): log.Printf("[INFO] cleanup terminated, %v", ctx.Err()) return - case <-time.After(s.TTL / 2): // cleanup call on every 1/2 TTL - if err := s.store.Cleanup(ctx, s.TTL); err != nil { + case <-time.After(s.cleanupTTL): + if err := s.store.Cleanup(ctx, s.cleanupTTL); err != nil { log.Printf("[WARN] failed to cleanup, %v", err) } } diff --git a/backend/app/store/image/image_test.go b/backend/app/store/image/image_test.go index 6a738f30..cd77995f 100644 --- a/backend/app/store/image/image_test.go +++ b/backend/app/store/image/image_test.go @@ -96,7 +96,7 @@ func TestService_Cleanup(t *testing.T) { store := MockStore{} store.On("Cleanup", mock.Anything, mock.Anything).Times(10).Return(nil) - svc := Service{store: &store, ServiceParams: ServiceParams{TTL: 100 * time.Millisecond}} + svc := NewService(&store, ServiceParams{EditDuration: 20 * time.Millisecond}) ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*549) defer cancel() svc.Cleanup(ctx) @@ -106,7 +106,7 @@ func TestService_Cleanup(t *testing.T) { func TestService_Submit(t *testing.T) { store := MockStore{} store.On("Commit", mock.Anything, mock.Anything).Times(5).Return(nil) - svc := Service{store: &store, ServiceParams: ServiceParams{ImageAPI: "/blah/", TTL: time.Millisecond * 100}} + svc := Service{store: &store, ServiceParams: ServiceParams{ImageAPI: "/blah/", EditDuration: time.Millisecond * 100}} svc.Submit(func() []string { return []string{"id1", "id2", "id3"} }) svc.Submit(func() []string { return []string{"id4", "id5"} }) svc.Submit(nil) @@ -119,7 +119,7 @@ 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) - svc := Service{store: &store, ServiceParams: ServiceParams{ImageAPI: "/blah/", TTL: time.Hour * 24}} + 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) @@ -130,7 +130,7 @@ 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) - svc := Service{store: &store, ServiceParams: ServiceParams{ImageAPI: "/blah/", TTL: time.Millisecond * 100}} + 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"} }) diff --git a/backend/app/store/service/service_test.go b/backend/app/store/service/service_test.go index e662b041..76195d41 100644 --- a/backend/app/store/service/service_test.go +++ b/backend/app/store/service/service_test.go @@ -1278,8 +1278,9 @@ func TestService_submitImages(t *testing.T) { lgr.Setup(lgr.Debug, lgr.CallerFile, lgr.CallerFunc) mockStore := image.MockStore{} - mockStore.On("Commit", mock.Anything, mock.Anything).Times(2).Return(nil) - imgSvc := image.NewService(&mockStore, image.ServiceParams{TTL: 50 * time.Millisecond * 50}) + mockStore.On("Commit", mock.Anything).Times(2).Return(nil) + imgSvc := image.NewService(&mockStore, image.ServiceParams{EditDuration: 50 * time.Millisecond}) + defer imgSvc.Close(context.TODO()) // two comments for https://radio-t.com eng, teardown := prepStoreEngine(t)