From 5afc22924f422786b6a1a01e971249c8f7d568db Mon Sep 17 00:00:00 2001 From: LordMathis Date: Thu, 16 Oct 2025 20:15:22 +0200 Subject: [PATCH] Refactor Status struct --- pkg/instance/instance.go | 88 +++++++++++++++++++++++++++------------ pkg/instance/status.go | 82 +++++++++++++++++++++++++++--------- pkg/manager/manager.go | 2 +- pkg/manager/operations.go | 2 +- 4 files changed, 125 insertions(+), 49 deletions(-) diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go index 5123dab..0480062 100644 --- a/pkg/instance/instance.go +++ b/pkg/instance/instance.go @@ -16,23 +16,21 @@ import ( // Instance represents a running instance of the llama server type Instance struct { - Name string `json:"name"` - options *CreateInstanceOptions `json:"-"` + // Immutable identity (no locking needed after creation) + Name string `json:"name"` + Created int64 `json:"created,omitempty"` // Unix timestamp when the instance was created + + // Mutable state - each owns its own lock + status *status `json:"status"` // unexported - status owns its lock + options *CreateInstanceOptions `json:"-"` + + // Global configuration (read-only, no lock needed) globalInstanceSettings *config.InstancesConfig globalBackendSettings *config.BackendConfig - // Status - Status Status `json:"status"` - onStatusChange func(oldStatus, newStatus Status) - - // Creation time - Created int64 `json:"created,omitempty"` // Unix timestamp when the instance was created - - // Logging file - logger *logger `json:"-"` - - // Proxy component - proxy *proxy `json:"-"` // HTTP proxy and request tracking + // Components (can be nil for remote instances or when stopped) + logger *logger `json:"-"` // nil for remote instances + proxy *proxy `json:"-"` // nil for remote instances // internal cmd *exec.Cmd `json:"-"` // Command to run the instance @@ -56,6 +54,10 @@ func NewInstance(name string, globalBackendSettings *config.BackendConfig, globa // Create the instance logger logger := NewLogger(name, globalInstanceSettings.LogsDir) + // Create status wrapper + status := newStatus(Stopped) + status.onStatusChange = onStatusChange + instance := &Instance{ Name: name, options: options, @@ -63,8 +65,7 @@ func NewInstance(name string, globalBackendSettings *config.BackendConfig, globa globalBackendSettings: globalBackendSettings, logger: logger, Created: time.Now().Unix(), - Status: Stopped, - onStatusChange: onStatusChange, + status: status, } // Create Proxy component @@ -79,6 +80,29 @@ func (i *Instance) GetOptions() *CreateInstanceOptions { return i.options } +// GetStatus returns the current status, delegating to status component +func (i *Instance) GetStatus() Status { + if i.status == nil { + return Stopped + } + return i.status.Get() +} + +// SetStatus sets the status, delegating to status component +func (i *Instance) SetStatus(s Status) { + if i.status != nil { + i.status.Set(s) + } +} + +// IsRunning returns true if the status is Running, delegating to status component +func (i *Instance) IsRunning() bool { + if i.status == nil { + return false + } + return i.status.IsRunning() +} + func (i *Instance) GetPort() int { i.mu.RLock() defer i.mu.RUnlock() @@ -182,14 +206,17 @@ func (i *Instance) MarshalJSON() ([]byte, error) { } } - // Use anonymous struct to avoid recursion - type Alias Instance + // Explicitly serialize to maintain backward compatible JSON format return json.Marshal(&struct { - *Alias + Name string `json:"name"` + Status *status `json:"status"` + Created int64 `json:"created,omitempty"` Options *CreateInstanceOptions `json:"options,omitempty"` DockerEnabled bool `json:"docker_enabled,omitempty"` }{ - Alias: (*Alias)(i), + Name: i.Name, + Status: i.status, + Created: i.Created, Options: i.options, DockerEnabled: dockerEnabled, }) @@ -197,26 +224,33 @@ func (i *Instance) MarshalJSON() ([]byte, error) { // UnmarshalJSON implements json.Unmarshaler for Instance func (i *Instance) UnmarshalJSON(data []byte) error { - // Use anonymous struct to avoid recursion - type Alias Instance + // Explicitly deserialize to match MarshalJSON format aux := &struct { - *Alias + Name string `json:"name"` + Status *status `json:"status"` + Created int64 `json:"created,omitempty"` Options *CreateInstanceOptions `json:"options,omitempty"` - }{ - Alias: (*Alias)(i), - } + }{} if err := json.Unmarshal(data, aux); err != nil { return err } + // Set the fields + i.Name = aux.Name + i.Created = aux.Created + i.status = aux.Status + // Handle options with validation and defaults if aux.Options != nil { aux.Options.ValidateAndApplyDefaults(i.Name, i.globalInstanceSettings) i.options = aux.Options } - // Initialize fields that are not serialized + // Initialize fields that are not serialized or may be nil + if i.status == nil { + i.status = newStatus(Stopped) + } if i.logger == nil && i.globalInstanceSettings != nil { i.logger = NewLogger(i.Name, i.globalInstanceSettings.LogsDir) } diff --git a/pkg/instance/status.go b/pkg/instance/status.go index 2c1045e..8c24f49 100644 --- a/pkg/instance/status.go +++ b/pkg/instance/status.go @@ -3,9 +3,10 @@ package instance import ( "encoding/json" "log" + "sync" ) -// Enum for instance status +// Status is the enum for status values (exported). type Status int const ( @@ -26,24 +27,7 @@ var statusToName = map[Status]string{ Failed: "failed", } -func (p *Instance) SetStatus(status Status) { - oldStatus := p.Status - p.Status = status - - if p.onStatusChange != nil { - p.onStatusChange(oldStatus, status) - } -} - -func (p *Instance) GetStatus() Status { - return p.Status -} - -// IsRunning returns true if the status is Running -func (p *Instance) IsRunning() bool { - return p.Status == Running -} - +// Status enum JSON marshaling methods func (s Status) MarshalJSON() ([]byte, error) { name, ok := statusToName[s] if !ok { @@ -52,7 +36,7 @@ func (s Status) MarshalJSON() ([]byte, error) { return json.Marshal(name) } -// UnmarshalJSON implements json.Unmarshaler +// UnmarshalJSON implements json.Unmarshaler for Status enum func (s *Status) UnmarshalJSON(data []byte) error { var str string if err := json.Unmarshal(data, &str); err != nil { @@ -68,3 +52,61 @@ func (s *Status) UnmarshalJSON(data []byte) error { *s = status return nil } + +// status represents the instance status with thread-safe access (unexported). +type status struct { + mu sync.RWMutex + s Status + + // Callback for status changes + onStatusChange func(oldStatus, newStatus Status) +} + +// newStatus creates a new status wrapper with the given initial status +func newStatus(initial Status) *status { + return &status{ + s: initial, + } +} + +// Get returns the current status +func (st *status) Get() Status { + st.mu.RLock() + defer st.mu.RUnlock() + return st.s +} + +// Set updates the status and triggers the onStatusChange callback if set +func (st *status) Set(newStatus Status) { + st.mu.Lock() + oldStatus := st.s + st.s = newStatus + callback := st.onStatusChange + st.mu.Unlock() + + // Call the callback outside the lock to avoid potential deadlocks + if callback != nil { + callback(oldStatus, newStatus) + } +} + +// IsRunning returns true if the status is Running +func (st *status) IsRunning() bool { + st.mu.RLock() + defer st.mu.RUnlock() + return st.s == Running +} + +// MarshalJSON implements json.Marshaler for status wrapper +func (st *status) MarshalJSON() ([]byte, error) { + st.mu.RLock() + defer st.mu.RUnlock() + return st.s.MarshalJSON() +} + +// UnmarshalJSON implements json.Unmarshaler for status wrapper +func (st *status) UnmarshalJSON(data []byte) error { + st.mu.Lock() + defer st.mu.Unlock() + return st.s.UnmarshalJSON(data) +} diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 12e1a59..da5a900 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -289,7 +289,7 @@ func (im *instanceManager) loadInstance(name, path string) error { // Restore persisted fields that NewInstance doesn't set inst.Created = persistedInstance.Created - inst.SetStatus(persistedInstance.Status) + inst.SetStatus(persistedInstance.GetStatus()) // Handle remote instance mapping if isRemote { diff --git a/pkg/manager/operations.go b/pkg/manager/operations.go index 80858d0..cc2dba4 100644 --- a/pkg/manager/operations.go +++ b/pkg/manager/operations.go @@ -39,7 +39,7 @@ func (im *instanceManager) updateLocalInstanceFromRemote(localInst *instance.Ins // Update the local instance with all remote data localInst.SetOptions(&updatedOptions) - localInst.Status = remoteInst.Status + localInst.SetStatus(remoteInst.GetStatus()) localInst.Created = remoteInst.Created }