refactor to using name instead of moduleID

Author: henderkes

caddy/caddy.go

@@ -4,11 +4,10 @@
 package caddy
 
 import (
-	"crypto/sha256"
-	"encoding/binary"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/davecgh/go-spew/spew"
 	"net/http"
 	"path/filepath"
 	"strconv"
@@ -53,7 +52,7 @@ func init() {
 }
 
 type workerConfig struct {
-	// Name for the worker
+	// Name for the worker. Default: the filename for FrankenPHPApp workers, filename + environment variables for FrankenPHPModule workers.
 	Name string `json:"name,omitempty"`
 	// FileName sets the path to the worker script.
 	FileName string `json:"file_name,omitempty"`
@@ -63,8 +62,6 @@ type workerConfig struct {
 	Env map[string]string `json:"env,omitempty"`
 	// Directories to watch for file changes
 	Watch []string `json:"watch,omitempty"`
-	// ModuleID identifies which module created this worker
-	ModuleID uint64 `json:"module_id,omitempty"`
 }
 
 type FrankenPHPApp struct {
@@ -119,20 +116,17 @@ func (f *FrankenPHPApp) Start() error {
 		frankenphp.WithMaxWaitTime(f.MaxWaitTime),
 	}
 	// Add workers from FrankenPHPApp configuration
-	for _, w := range f.Workers {
-		opts = append(opts, frankenphp.WithWorkers(w.Name, repl.ReplaceKnown(w.FileName, ""), w.Num, w.Env, w.Watch, w.ModuleID))
-	}
-
-	// Add workers from FrankenPHPModule configurations
-	for _, w := range sharedState.Workers {
-		opts = append(opts, frankenphp.WithWorkers(w.Name, repl.ReplaceKnown(w.FileName, ""), w.Num, w.Env, w.Watch, w.ModuleID))
+	for _, w := range append(f.Workers, sharedState.Workers...) {
+		opts = append(opts, frankenphp.WithWorkers(w.Name, repl.ReplaceKnown(w.FileName, ""), w.Num, w.Env, w.Watch))
 	}
 
 	frankenphp.Shutdown()
 	if err := frankenphp.Init(opts...); err != nil {
 		return err
 	}
 
+	caddy.Log().Warn(fmt.Sprintf("FrankenPHPApp started with workers: %s", spew.Sdump(append(f.Workers, sharedState.Workers...))))
+
 	return nil
 }
 
@@ -360,8 +354,6 @@ type FrankenPHPModule struct {
 	ResolveRootSymlink *bool `json:"resolve_root_symlink,omitempty"`
 	// Env sets an extra environment variable to the given value. Can be specified more than once for multiple environment variables.
 	Env map[string]string `json:"env,omitempty"`
-	// ModuleID is the module ID that created this request.
-	ModuleID uint64 `json:"-"`
 	// Workers configures the worker scripts to start.
 	Workers []workerConfig `json:"workers,omitempty"`
 
@@ -436,17 +428,6 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
 	}
 
 	if len(f.Workers) > 0 {
-		envString := ""
-		for k, v := range f.Env {
-			envString += k + "=" + v + ","
-		}
-		data := []byte(f.Root + envString)
-		hash := sha256.Sum256(data)
-		f.ModuleID = binary.LittleEndian.Uint64(hash[:8])
-
-		for i := range f.Workers {
-			f.Workers[i].ModuleID = f.ModuleID
-		}
 		sharedState.Workers = append(sharedState.Workers, f.Workers...)
 	}
 
@@ -479,13 +460,20 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 		}
 	}
 
+	workerNames := make([]string, len(f.Workers))
+	for i, w := range f.Workers {
+		workerNames[i] = w.Name
+	}
+
+	caddy.Log().Info(fmt.Sprintf("ServeHTTP module has workers: %s", spew.Sdump(f.Workers)))
+
 	fr, err := frankenphp.NewRequestWithContext(
 		r,
 		documentRootOption,
 		frankenphp.WithRequestSplitPath(f.SplitPath),
 		frankenphp.WithRequestPreparedEnv(env),
 		frankenphp.WithOriginalRequest(&origReq),
-		frankenphp.WithModuleID(f.ModuleID),
+		frankenphp.WithWorkerNames(workerNames),
 	)
 
 	if err != nil {
@@ -637,11 +625,32 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 					}
 				}
 
+				if wc.Name == "" {
+					name, _ := fastabs.FastAbs(wc.FileName)
+					if name == "" {
+						name = wc.FileName
+					}
+					wc.Name = name
+
+					if len(wc.Env) > 0 {
+						envString := ""
+						for k, v := range wc.Env {
+							envString += k + "=" + v + ","
+						}
+						wc.Name += "#" + envString
+					}
+				}
+				if !strings.HasPrefix(wc.Name, "m#") {
+					wc.Name = "m#" + wc.Name
+				}
+
 				f.Workers = append(f.Workers, wc)
 			}
 		}
 	}
 
+	caddy.Log().Warn(fmt.Sprintf("FrankenPHPModule UnmarshalCaddyfile with workers: %s", spew.Sdump(f.Workers)))
+
 	return nil
 }
 

caddy/caddy_test.go

@@ -161,7 +161,7 @@ func TestGlobalAndLocalWorker(t *testing.T) {
 		}
 		`, "caddyfile")
 
-	for i := 0; i < 100; i++ {
+	for i := 0; i < 2; i++ {
 		wg.Add(1)
 
 		go func(i int) {

context.go

@@ -23,6 +23,7 @@ type frankenPHPContext struct {
 	pathInfo       string
 	scriptName     string
 	scriptFilename string
+	workerNames    []string
 
 	// Whether the request is already closed by us
 	isDone bool
@@ -31,9 +32,6 @@ type frankenPHPContext struct {
 
 	done      chan interface{}
 	startedAt time.Time
-
-	// The module ID that created this request
-	moduleID uint64
 }
 
 // fromContext extracts the frankenPHPContext from a context.
@@ -42,8 +40,7 @@ func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) {
 	return
 }
 
-// NewRequestWithContext creates a new FrankenPHP request context.
-func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) {
+func newFrankenPHPContext(r *http.Request, opts ...RequestOption) (*frankenPHPContext, error) {
 	fc := &frankenPHPContext{
 		done:      make(chan interface{}),
 		startedAt: time.Now(),
@@ -94,7 +91,15 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques
 
 	// SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
 	fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName)
+	return fc, nil
+}
 
+// NewRequestWithContext creates a new FrankenPHP request context.
+func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) {
+	fc, err2 := newFrankenPHPContext(r, opts...)
+	if err2 != nil {
+		return nil, err2
+	}
 	c := context.WithValue(r.Context(), contextKey, fc)
 
 	return r.WithContext(c), nil

frankenphp.go

@@ -32,6 +32,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"github.com/davecgh/go-spew/spew"
 	"io"
 	"net/http"
 	"os"
@@ -292,6 +293,8 @@ func Init(options ...Option) error {
 		return err
 	}
 
+	logger.Warn(fmt.Sprintf("FrankenPHP initialised workers: %s", spew.Sdump(opt.workers)))
+
 	initAutoScaling(mainThread)
 
 	if c := logger.Check(zapcore.InfoLevel, "FrankenPHP started 🐘"); c != nil {
@@ -404,14 +407,20 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error
 		return nil
 	}
 
-	// Detect if a worker is available to handle this request
-	if worker := getWorkerForContext(fc); worker != nil {
-		worker.handleRequest(fc)
-	} else {
-		// If no worker was available, send the request to non-worker threads
-		handleRequestWithRegularPHPThreads(fc)
+	workerNames := append(fc.workerNames, "")
+	fc.logger.Warn(fmt.Sprintf("ServeHTTP Worker Names: %s", spew.Sdump(workerNames)))
+	for _, workerName := range workerNames {
+		// Detect if a worker is available to handle this request
+		workername := workerName + fc.scriptFilename
+		if worker := getWorkerForName(workername); worker != nil {
+			fc.logger.Info(fmt.Sprintf("Found worker: %s", workername))
+			worker.handleRequest(fc)
+			return nil
+		}
 	}
 
+	// If no worker was available, send the request to non-worker threads
+	handleRequestWithRegularPHPThreads(fc)
 	return nil
 }
 

frankenphp_test.go

@@ -65,7 +65,7 @@ func runTest(t *testing.T, test func(func(http.ResponseWriter, *http.Request), *
 
 	initOpts := []frankenphp.Option{frankenphp.WithLogger(opts.logger)}
 	if opts.workerScript != "" {
-		initOpts = append(initOpts, frankenphp.WithWorkers("workerName", testDataDir+opts.workerScript, opts.nbWorkers, opts.env, opts.watch, 0))
+		initOpts = append(initOpts, frankenphp.WithWorkers("workerName", testDataDir+opts.workerScript, opts.nbWorkers, opts.env, opts.watch))
 	}
 	initOpts = append(initOpts, opts.initOpts...)
 	if opts.phpIni != nil {

options.go

@@ -28,7 +28,6 @@ type workerOpt struct {
 	num      int
 	env      PreparedEnv
 	watch    []string
-	moduleID uint64
 }
 
 // WithNumThreads configures the number of PHP threads to start.
@@ -56,10 +55,10 @@ func WithMetrics(m Metrics) Option {
 	}
 }
 
-// WithWorkers configures the PHP workers to start, moduleID is used to identify the worker for a specific domain
-func WithWorkers(name string, fileName string, num int, env map[string]string, watch []string, moduleID uint64) Option {
+// WithWorkers configures the PHP workers to start
+func WithWorkers(name string, fileName string, num int, env map[string]string, watch []string) Option {
 	return func(o *opt) error {
-		o.workers = append(o.workers, workerOpt{name, fileName, num, PrepareEnv(env), watch, moduleID})
+		o.workers = append(o.workers, workerOpt{name, fileName, num, PrepareEnv(env), watch})
 
 		return nil
 	}

phpmainthread_test.go

@@ -96,8 +96,8 @@ func TestTransitionThreadsWhileDoingRequests(t *testing.T) {
 
 	assert.NoError(t, Init(
 		WithNumThreads(numThreads),
-		WithWorkers(worker1Name, worker1Path, 1, map[string]string{"ENV1": "foo"}, []string{}, 0),
-		WithWorkers(worker2Name, worker2Path, 1, map[string]string{"ENV1": "foo"}, []string{}, 0),
+		WithWorkers(worker1Name, worker1Path, 1, map[string]string{"ENV1": "foo"}, []string{}),
+		WithWorkers(worker2Name, worker2Path, 1, map[string]string{"ENV1": "foo"}, []string{}),
 		WithLogger(zap.NewNop()),
 	))
 
@@ -182,7 +182,7 @@ func TestFinishBootingAWorkerScript(t *testing.T) {
 
 func getDummyWorker(fileName string) *worker {
 	if workers == nil {
-		workers = make(map[string][]*worker)
+		workers = make(map[string]*worker)
 	}
 	worker, _ := newWorker(workerOpt{
 		fileName: testDataPath + "/" + fileName,
@@ -214,9 +214,9 @@ func allPossibleTransitions(worker1Path string, worker2Path string) []func(*phpT
 				thread.boot()
 			}
 		},
-		func(thread *phpThread) { convertToWorkerThread(thread, workers[worker1Path][0]) },
+		func(thread *phpThread) { convertToWorkerThread(thread, workers[worker1Path]) },
 		convertToInactiveThread,
-		func(thread *phpThread) { convertToWorkerThread(thread, workers[worker2Path][0]) },
+		func(thread *phpThread) { convertToWorkerThread(thread, workers[worker2Path]) },
 		convertToInactiveThread,
 	}
 }

request_options.go

@@ -124,10 +124,10 @@ func WithRequestLogger(logger *zap.Logger) RequestOption {
 	}
 }
 
-// WithModuleID sets the module ID associated with the current request
-func WithModuleID(moduleID uint64) RequestOption {
+// WithRequestLogger sets the logger associated with the current request
+func WithWorkerNames(names []string) RequestOption {
 	return func(o *frankenPHPContext) error {
-		o.moduleID = moduleID
+		o.workerNames = names
 
 		return nil
 	}

scaling.go

@@ -160,7 +160,7 @@ func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext,
 			}
 
 			// if the request has been stalled long enough, scale
-			if worker := getWorkerForContext(fc); worker != nil {
+			if worker := getWorkerForName(fc.scriptFilename); worker != nil {
 				scaleWorkerThread(worker)
 			} else {
 				scaleRegularThread()

scaling_test.go

@@ -36,14 +36,14 @@ func TestScaleAWorkerThreadUpAndDown(t *testing.T) {
 	assert.NoError(t, Init(
 		WithNumThreads(2),
 		WithMaxThreads(3),
-		WithWorkers(workerName, workerPath, 1, map[string]string{}, []string{}, 0),
+		WithWorkers(workerName, workerPath, 1, map[string]string{}, []string{}),
 		WithLogger(zap.NewNop()),
 	))
 
 	autoScaledThread := phpThreads[2]
 
 	// scale up
-	scaleWorkerThread(workers[workerPath][0])
+	scaleWorkerThread(workers[workerPath])
 	assert.Equal(t, stateReady, autoScaledThread.state.get())
 
 	// on down-scale, the thread will be marked as inactive