Skip to content

Commit

Permalink
Add imports location lock to serialize non-atomic operations (#204)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kklimonda-cl authored Dec 13, 2024
1 parent a6e7dad commit e4bf2a3
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 12 deletions.
2 changes: 2 additions & 0 deletions pkg/translate/imports.go
Original file line number Diff line number Diff line change
Expand Up @@ -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", "")
Expand Down
80 changes: 68 additions & 12 deletions templates/sdk/service.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand All @@ -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
}
Expand Down Expand Up @@ -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",
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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",
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit e4bf2a3

Please sign in to comment.