From e4bf2a31167cd72ff3bf930988e6018ff56ed3ae Mon Sep 17 00:00:00 2001 From: kklimonda-cl Date: Fri, 13 Dec 2024 09:03:48 +0100 Subject: [PATCH] Add imports location lock to serialize non-atomic operations (#204) Adding and removing items from the imports location list is currently implemented in a non-atomic way, by first reading a list of items from the server, updating it locally, and sending a new list to the server. Introduce a (per-resource, per-location) lock to all resources that require modifications of the import list so that those operations are done sequentially. --- pkg/translate/imports.go | 2 + templates/sdk/service.tmpl | 80 ++++++++++++++++++++++++++++++++------ 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/pkg/translate/imports.go b/pkg/translate/imports.go index bb7aa9e5..f4fa31ec 100644 --- a/pkg/translate/imports.go +++ b/pkg/translate/imports.go @@ -10,6 +10,8 @@ func RenderImports(templateTypes ...string) (string, error) { for _, templateType := range templateTypes { switch templateType { + case "sync": + manager.AddStandardImport("sync", "") case "config": //manager.AddStandardImport("fmt", "") manager.AddStandardImport("encoding/xml", "") diff --git a/templates/sdk/service.tmpl b/templates/sdk/service.tmpl index ef80f314..9331048a 100644 --- a/templates/sdk/service.tmpl +++ b/templates/sdk/service.tmpl @@ -2,9 +2,9 @@ package {{packageName .GoSdkPath}} {{- if .Entry}} {{- if $.Imports}} {{- if $.Spec.Params.uuid}} - {{renderImports "service" "filtering" "audit" "rule" "version"}} + {{renderImports "service" "filtering" "sync" "audit" "rule" "version"}} {{- else}} - {{renderImports "service" "filtering"}} + {{renderImports "service" "filtering" "sync"}} {{- end}} {{- else}} {{- if $.Spec.Params.uuid}} @@ -22,6 +22,28 @@ package {{packageName .GoSdkPath}} {{- end}} {{- end}} +{{- if and $.Imports .Entry }} +var ( + importsMutexMap = make(map[string]*sync.Mutex) + importsMutexMapLock = sync.Mutex{} +) + +func (s *Service) getImportMutex(xpath string) *sync.Mutex { + importsMutexMapLock.Lock() + defer importsMutexMapLock.Unlock() + + var importMutex *sync.Mutex + var ok bool + importMutex, ok = importsMutexMap[xpath] + if !ok { + importMutex = &sync.Mutex{} + importsMutexMap[xpath] = importMutex + } + + return importMutex +} +{{- end }} + type Service struct { client util.PangoClient } @@ -101,8 +123,16 @@ if err != nil { func (s *Service) importToLocations(ctx context.Context, loc Location, importLocations []ImportLocation, entryName string) error { vn := s.client.Versioning() - for _, elt := range importLocations { - xpath, err := elt.XpathForLocation(vn, loc) + + importToLocation := func(il ImportLocation) error { + xpath, err := il.XpathForLocation(vn, loc) + if err != nil { + return err + } + + mutex := s.getImportMutex(util.AsXpath(xpath)) + mutex.Lock() + defer mutex.Unlock() cmd := &xmlapi.Config{ Action: "get", @@ -114,7 +144,7 @@ func (s *Service) importToLocations(ctx context.Context, loc Location, importLoc return err } - existing, err := elt.UnmarshalPangoXML(bytes) + existing, err := il.UnmarshalPangoXML(bytes) if err != nil { return err } @@ -127,7 +157,7 @@ func (s *Service) importToLocations(ctx context.Context, loc Location, importLoc existing = append(existing, entryName) - element, err := elt.MarshalPangoXML(existing) + element, err := il.MarshalPangoXML(existing) if err != nil { return err } @@ -142,19 +172,36 @@ func (s *Service) importToLocations(ctx context.Context, loc Location, importLoc if err != nil { return err } + + return err + } + + for _, elt := range importLocations { + err := importToLocation(elt) + if err != nil { + return err + } } return nil } -func (s *Service) unimportFromLocations(ctx context.Context, updates *xmlapi.MultiConfig, loc Location, importLocations []ImportLocation, values []string) error { +func (s *Service) unimportFromLocations(ctx context.Context, loc Location, importLocations []ImportLocation, values []string) error { vn := s.client.Versioning() valuesByName := make(map[string]bool) for _, elt := range values { valuesByName[elt] = true } - for _, elt := range importLocations { - xpath, err := elt.XpathForLocation(vn, loc) + + unimportFromLocation := func(il ImportLocation) error { + xpath, err := il.XpathForLocation(vn, loc) + if err != nil { + return err + } + + mutex := s.getImportMutex(util.AsXpath(xpath)) + mutex.Lock() + defer mutex.Unlock() cmd := &xmlapi.Config{ Action: "get", @@ -166,7 +213,7 @@ func (s *Service) unimportFromLocations(ctx context.Context, updates *xmlapi.Mul return err } - existing, err := elt.UnmarshalPangoXML(bytes) + existing, err := il.UnmarshalPangoXML(bytes) if err != nil { return err } @@ -178,7 +225,7 @@ func (s *Service) unimportFromLocations(ctx context.Context, updates *xmlapi.Mul } } - element, err := elt.MarshalPangoXML(filtered) + element, err := il.MarshalPangoXML(filtered) if err != nil { return err } @@ -193,6 +240,15 @@ func (s *Service) unimportFromLocations(ctx context.Context, updates *xmlapi.Mul if err != nil { return err } + + return err + } + + for _, elt := range importLocations { + err := unimportFromLocation(elt) + if err != nil { + return err + } } return nil @@ -529,7 +585,7 @@ vn := s.client.Versioning() var err error deletes := xmlapi.NewMultiConfig(len(values)) {{- if .Imports }} - err = s.unimportFromLocations(ctx, deletes, loc, importLocations, values) + err = s.unimportFromLocations(ctx, loc, importLocations, values) if err != nil { return err }