Skip to content

feat: added GetChangedStrategies API #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ianchen0119 merged 1 commit into from
Jan 24, 2026
Merged

feat: added GetChangedStrategies API #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 49 additions & 6 deletions plugin/gthulhu/gthulhu.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/Gthulhu/plugin/models"
reg "github.com/Gthulhu/plugin/plugin/internal/registry"
"github.com/Gthulhu/plugin/plugin/util"
)

func init() {
Expand Down Expand Up @@ -65,8 +66,11 @@ type GthulhuPlugin struct {
minVruntime uint64

// Strategy map for PID-based scheduling strategies
strategyMap map[int32]SchedulingStrategy
strategyMu sync.RWMutex
oldStrategyMap map[int32]util.SchedulingStrategy
strategyMap map[int32]util.SchedulingStrategy
newStrategy []util.SchedulingStrategy
removedStrategy []util.SchedulingStrategy
strategyMu sync.RWMutex

// JWT client for API authentication
jwtClient *JWTClient
Expand All @@ -82,7 +86,7 @@ func NewGthulhuPlugin(sliceNsDefault, sliceNsMin uint64) *GthulhuPlugin {
taskPool: make([]Task, taskPoolSize),
taskPoolCount: 0,
minVruntime: 0,
strategyMap: make(map[int32]SchedulingStrategy),
strategyMap: make(map[int32]util.SchedulingStrategy),
}

// Override defaults if provided
Expand Down Expand Up @@ -348,24 +352,63 @@ func (g *GthulhuPlugin) GetSchedulerConfig() (uint64, uint64) {
}

// FetchSchedulingStrategies fetches scheduling strategies from the API server
func (g *GthulhuPlugin) FetchSchedulingStrategies(apiUrl string) ([]SchedulingStrategy, error) {
func (g *GthulhuPlugin) FetchSchedulingStrategies(apiUrl string) ([]util.SchedulingStrategy, error) {
if g.jwtClient == nil {
return nil, nil // Silently skip if JWT client not initialized
}
return fetchSchedulingStrategies(g.jwtClient, apiUrl)
}

// UpdateStrategyMap updates the strategy map from a slice of strategies
func (g *GthulhuPlugin) UpdateStrategyMap(strategies []SchedulingStrategy) {
func (g *GthulhuPlugin) UpdateStrategyMap(strategies []util.SchedulingStrategy) {
// Create a new map to avoid concurrent access issues
newMap := make(map[int32]SchedulingStrategy)
newMap := make(map[int32]util.SchedulingStrategy)

for _, strategy := range strategies {
newMap[int32(strategy.PID)] = strategy
}

// Replace the old map with the new one
g.strategyMu.Lock()
g.oldStrategyMap = g.strategyMap
g.strategyMap = newMap
changed, removed := g.caculateChangedStrategies()
g.newStrategy = append(g.newStrategy, changed...)
g.removedStrategy = append(g.removedStrategy, removed...)
Comment on lines +373 to +377
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UpdateStrategyMap accumulates both g.newStrategy and g.removedStrategy, but GetChangedStrategies currently returns only the changed slice and always returns nil for the removed slice, and never clears g.removedStrategy. This will both break the intended API and can cause unbounded memory growth. Return (changed, removed) and clear both buffers once they’ve been read.

Copilot uses AI. Check for mistakes.
g.strategyMu.Unlock()
}

// Campare g.oldStrategyMap and g.strategyMap and return the list of SchedulingStrategy that have changed strategies
func (g *GthulhuPlugin) caculateChangedStrategies() ([]util.SchedulingStrategy, []util.SchedulingStrategy) {
Comment on lines +381 to +382
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo/unclear naming: caculateChangedStrategies and the comments above it (“Campare…”) are misspelled and misleading. Rename to calculateChangedStrategies (or similar) and update the comments to accurately describe what the method returns (e.g., added/changed vs removed).

Copilot uses AI. Check for mistakes.
changed := []util.SchedulingStrategy{}
removed := []util.SchedulingStrategy{}

// Check for removed strategies
for pid, oldStrategy := range g.oldStrategyMap {
_, exists := g.strategyMap[pid]
if !exists {
removed = append(removed, oldStrategy)
}
}

// Check for changed or new strategies
for pid, newStrategy := range g.strategyMap {
oldStrategy, exists := g.oldStrategyMap[pid]
if !exists || oldStrategy != newStrategy {
changed = append(changed, newStrategy)
}
}
return changed, removed
}

// Campare g.oldStrategyMap and g.strategyMap and return the list of SchedulingStrategy that have changed strategies
func (g *GthulhuPlugin) GetChangedStrategies() ([]util.SchedulingStrategy, []util.SchedulingStrategy) {
changed := []util.SchedulingStrategy{}
Comment on lines +405 to +406
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetChangedStrategies introduces new externally-consumed behavior (returning strategy deltas and clearing internal buffers), but there are no tests covering its semantics (e.g., returns changes after UpdateStrategyMap, returns removals, and returns empty on subsequent calls). Please add unit tests to lock down the API contract and prevent regressions.

Copilot uses AI. Check for mistakes.
g.strategyMu.RLock()
defer g.strategyMu.RUnlock()

// copy g.newStrategy to changed and clear g.newStrategy
changed = append(changed, g.newStrategy...)
g.newStrategy = []util.SchedulingStrategy{}
return changed, nil
Comment on lines +410 to +413
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetChangedStrategies takes an RLock but then mutates shared state by clearing g.newStrategy. Because RLock allows concurrent readers, multiple callers can race on g.newStrategy, and this also violates the RWMutex contract (writes under read lock). Use g.strategyMu.Lock() while copying+clearing the slices (or use an atomic swap pattern) to make this thread-safe.

Copilot uses AI. Check for mistakes.
}
5 changes: 3 additions & 2 deletions plugin/gthulhu/gthulhu_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/Gthulhu/plugin/models"
reg "github.com/Gthulhu/plugin/plugin/internal/registry"
"github.com/Gthulhu/plugin/plugin/util"
)

// TestGthulhuPluginInstanceIsolation verifies that multiple GthulhuPlugin instances maintain independent state
Expand Down Expand Up @@ -103,7 +104,7 @@ func TestGthulhuPluginUpdateStrategyMap(t *testing.T) {
gthulhuPlugin := NewGthulhuPlugin(0, 0)

// Create test strategies
strategies := []SchedulingStrategy{
strategies := []util.SchedulingStrategy{
{PID: 100, Priority: true, ExecutionTime: 10000},
{PID: 200, Priority: false, ExecutionTime: 20000},
}
Expand Down Expand Up @@ -359,7 +360,7 @@ func TestGthulhuPluginRuntimeSimulation(t *testing.T) {
gthulhuPlugin = NewGthulhuPlugin(5000*1000, 500*1000) // Reset plugin

// Set up scheduling strategies
strategies := []SchedulingStrategy{
strategies := []util.SchedulingStrategy{
{PID: 100, Priority: true, ExecutionTime: 10000000}, // 10ms
{PID: 200, Priority: false, ExecutionTime: 20000000}, // 20ms
}
Expand Down
21 changes: 4 additions & 17 deletions plugin/gthulhu/strategy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,14 @@ import (
"io"
"log"
"time"
)

// SchedulingStrategy represents a strategy for process scheduling
type SchedulingStrategy struct {
Priority bool `json:"priority"` // If true, set vtime to minimum vtime
ExecutionTime uint64 `json:"execution_time"` // Time slice for this process in nanoseconds
PID int `json:"pid"` // Process ID to apply this strategy to
}

// SchedulingStrategiesResponse represents the response structure from the API
type SchedulingStrategiesResponse struct {
Success bool `json:"success"`
Message string `json:"message"`
Timestamp string `json:"timestamp"`
Scheduling []SchedulingStrategy `json:"scheduling"`
}
"github.com/Gthulhu/plugin/plugin/util"
)

const SCX_ENQ_PREEMPT = 1 << 32

// fetchSchedulingStrategies fetches scheduling strategies from the API server with JWT authentication
func fetchSchedulingStrategies(jwtClient *JWTClient, apiUrl string) ([]SchedulingStrategy, error) {
func fetchSchedulingStrategies(jwtClient *JWTClient, apiUrl string) ([]util.SchedulingStrategy, error) {
if jwtClient == nil {
return nil, fmt.Errorf("JWT client not initialized")
}
Expand All @@ -47,7 +34,7 @@ func fetchSchedulingStrategies(jwtClient *JWTClient, apiUrl string) ([]Schedulin
return nil, err
}

var response SchedulingStrategiesResponse
var response util.SchedulingStrategiesResponse
if err := json.Unmarshal(body, &response); err != nil {
return nil, err
}
Expand Down
3 changes: 3 additions & 0 deletions plugin/internal/registry/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"sync"

"github.com/Gthulhu/plugin/models"
"github.com/Gthulhu/plugin/plugin/util"
)

type Sched interface {
Expand All @@ -27,6 +28,8 @@ type CustomScheduler interface {
GetPoolCount() uint64
// SendMetrics sends custom metrics to the monitoring system
SendMetrics(interface{})
// GetChangedStrategies returns the list of scheduling strategies that have changed since the last call
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface comment doesn’t explain what each of the two returned slices from GetChangedStrategies represents. Please document the contract (e.g., first slice = added/updated strategies, second slice = removed strategies) so implementers and callers interpret the results consistently.

Suggested change
// GetChangedStrategies returns the list of scheduling strategies that have changed since the last call
// GetChangedStrategies returns the scheduling strategies that have changed since the last call.
// The first slice contains strategies that were added or updated.
// The second slice contains strategies that were removed.

Copilot uses AI. Check for mistakes.
GetChangedStrategies() ([]util.SchedulingStrategy, []util.SchedulingStrategy)
}

type Scheduler struct {
Expand Down
5 changes: 5 additions & 0 deletions plugin/plugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/Gthulhu/plugin/models"
"github.com/Gthulhu/plugin/plugin/util"
)

// TestRegisterNewPlugin tests the plugin registration functionality
Expand Down Expand Up @@ -360,3 +361,7 @@ func (m *mockScheduler) DetermineTimeSlice(s Sched, t *models.QueuedTask) uint64
func (m *mockScheduler) GetPoolCount() uint64 {
return 0
}

func (m *mockScheduler) GetChangedStrategies() ([]util.SchedulingStrategy, []util.SchedulingStrategy) {
return nil, nil
}
7 changes: 5 additions & 2 deletions plugin/simple/simple.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/Gthulhu/plugin/models"
reg "github.com/Gthulhu/plugin/plugin/internal/registry"
"github.com/Gthulhu/plugin/plugin/util"
)

func init() {
Expand Down Expand Up @@ -144,12 +145,10 @@ func (s *SimplePlugin) enqueueTask(queuedTask *models.QueuedTask) Task {

// Weighted vtime scheduling logic
if !s.fifoMode {

// Limit the amount of budget that an idling task can accumulate to one slice
if vtime < saturatingSub(s.vtimeNow, s.sliceDefault) {
vtime = saturatingSub(s.vtimeNow, s.sliceDefault)
}

}

// Ensure vtime is never 0 - use minimum value of 1 if needed
Expand Down Expand Up @@ -289,3 +288,7 @@ func (s *SimplePlugin) GetPoolStatus() (head, tail, count, capacity int) {
// For slice implementation: head=0, tail=len, count=len, capacity=cap
return 0, len(s.taskPool), len(s.taskPool), cap(s.taskPool)
}

func (s *SimplePlugin) GetChangedStrategies() ([]util.SchedulingStrategy, []util.SchedulingStrategy) {
return nil, nil
}
15 changes: 15 additions & 0 deletions plugin/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,21 @@ package util

import "time"

// SchedulingStrategy represents a strategy for process scheduling
type SchedulingStrategy struct {
Priority bool `json:"priority"` // If true, set vtime to minimum vtime
ExecutionTime uint64 `json:"execution_time"` // Time slice for this process in nanoseconds
PID int `json:"pid"` // Process ID to apply this strategy to
}

// SchedulingStrategiesResponse represents the response structure from the API
type SchedulingStrategiesResponse struct {
Success bool `json:"success"`
Message string `json:"message"`
Timestamp string `json:"timestamp"`
Scheduling []SchedulingStrategy `json:"scheduling"`
}

func Now() uint64 {
return uint64(time.Now().UnixNano())
}
Expand Down
Loading