Skip to content

Commit

Permalink
feat: make build timeout adjustable (#397)
Browse files Browse the repository at this point in the history
* feat: make build timeout configurable

* chore: document timeout var

* feat: format sigkill errors

* feat: format build timeout errors
  • Loading branch information
x1unix authored Aug 2, 2024
1 parent f5c29c6 commit 77cf7fc
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 47 deletions.
12 changes: 11 additions & 1 deletion cmd/playground/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ func main() {
analyzer.SetLogger(logger)
defer logger.Sync() //nolint:errcheck

if err := cfg.Validate(); err != nil {
logger.Fatal("invalid server configuration", zap.Error(err))
}

goRoot, err := builder.GOROOT()
if err != nil {
logger.Fatal("Failed to find GOROOT environment variable value", zap.Error(err))
Expand All @@ -53,6 +57,7 @@ func main() {
func start(goRoot string, logger *zap.Logger, cfg *config.Config) error {
logger.Info("Starting service",
zap.String("version", Version), zap.Any("config", cfg))

analyzer.SetRoot(goRoot)
packages, err := analyzer.ReadPackagesFile(cfg.Build.PackagesFile)
if err != nil {
Expand Down Expand Up @@ -92,7 +97,12 @@ func start(goRoot string, logger *zap.Logger, cfg *config.Config) error {
Mount(apiRouter)

apiv2Router := apiRouter.PathPrefix("/v2").Subrouter()
server.NewAPIv2Handler(playgroundClient, buildSvc).Mount(apiv2Router)
server.NewAPIv2Handler(server.APIv2HandlerConfig{
Client: playgroundClient,
Builder: buildSvc,
BuildTimeout: cfg.Build.GoBuildTimeout,
}).Mount(apiv2Router)
//server.NewAPIv2Handler(playgroundClient, buildSvc).Mount(apiv2Router)

// Web UI routes
tplVars := server.TemplateArguments{
Expand Down
31 changes: 16 additions & 15 deletions docs/deployment/docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,22 @@ docker-compose ps

Playground server can be configured using environment variables described below.

| Environment Variable | Example | Description |
|------------------------|--------------------------------|----------------------------------------------------------------|
| `GOROOT` | `/usr/local/go` | Go root location. Uses `go env GOROOT` as fallback. |
| `APP_DEBUG` | `false` | Enables debug logging. |
| `APP_LOG_LEVEL` | `info` | Logger log level. `debug` requires `APP_DEBUG` env var. |
| `APP_LOG_FORMAT` | `console`, `json` | Log format |
| `APP_PLAYGROUND_URL` | `https://play.golang.org` | Official Go playground service URL. |
| `APP_GOTIP_URL` | `https://gotipplay.golang.org` | GoTip playground service URL. |
| `APP_BUILD_DIR` | `/var/cache/wasm` | Path to store cached WebAssembly builds. |
| `APP_CLEAN_INTERVAL` | `10m` | WebAssembly build files cache cleanup interval. |
| `APP_SKIP_MOD_CLEANUP` | `1` | Disables WASM builds cache cleanup. |
| `APP_PERMIT_ENV_VARS` | `GOSUMDB,GOPROXY` | Restricts list of environment variables passed to Go compiler. |
| `HTTP_READ_TIMEOUT` | `15s` | HTTP request read timeout. |
| `HTTP_WRITE_TIMEOUT` | `60s` | HTTP response timeout. |
| `HTTP_IDLE_TIMEOUT` | `90s` | HTTP keep alive timeout. |
| Environment Variable | Example | Description |
|------------------------|--------------------------------|--------------------------------------------------------------------------------------------------|
| `GOROOT` | `/usr/local/go` | Go root location. Uses `go env GOROOT` as fallback. |
| `APP_DEBUG` | `false` | Enables debug logging. |
| `APP_LOG_LEVEL` | `info` | Logger log level. `debug` requires `APP_DEBUG` env var. |
| `APP_LOG_FORMAT` | `console`, `json` | Log format |
| `APP_PLAYGROUND_URL` | `https://play.golang.org` | Official Go playground service URL. |
| `APP_GOTIP_URL` | `https://gotipplay.golang.org` | GoTip playground service URL. |
| `APP_BUILD_DIR` | `/var/cache/wasm` | Path to store cached WebAssembly builds. |
| `APP_CLEAN_INTERVAL` | `10m` | WebAssembly build files cache cleanup interval. |
| `APP_SKIP_MOD_CLEANUP` | `1` | Disables WASM builds cache cleanup. |
| `APP_PERMIT_ENV_VARS` | `GOSUMDB,GOPROXY` | Restricts list of environment variables passed to Go compiler. |
| `APP_GO_BUILD_TIMEOUT` | `40s` | Go WebAssembly program build timeout. Includes dependency download process via `go mod download` |
| `HTTP_READ_TIMEOUT` | `15s` | HTTP request read timeout. |
| `HTTP_WRITE_TIMEOUT` | `60s` | HTTP response timeout. |
| `HTTP_IDLE_TIMEOUT` | `90s` | HTTP keep alive timeout. |

## Building custom image

Expand Down
10 changes: 1 addition & 9 deletions internal/builder/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func (s BuildService) runGoTool(ctx context.Context, workDir string, args ...str
zap.Error(err), zap.Strings("cmd", cmd.Args), zap.Stringer("stderr", buff),
)

return newBuildErrorFromStdout(err, buff)
return formatBuildError(ctx, err, buff)
}

return nil
Expand Down Expand Up @@ -212,11 +212,3 @@ func (s BuildService) Clean(ctx context.Context) error {

return nil
}

func newBuildErrorFromStdout(err error, buff *bytes.Buffer) error {
if buff.Len() > 0 {
return newBuildError(buff.String())
}

return newBuildError("Process returned error: %s", err)
}
36 changes: 36 additions & 0 deletions internal/builder/error.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package builder

import (
"bytes"
"context"
"errors"
"fmt"
)
Expand Down Expand Up @@ -31,3 +33,37 @@ func IsBuildError(err error) bool {
dst := new(BuildError)
return errors.As(err, &dst)
}

func checkContextErrors(err error) (error, bool) {
if err == nil {
return nil, false
}

if errors.Is(err, context.Canceled) {
return err, true
}

if errors.Is(err, context.DeadlineExceeded) {
return newBuildError("Go program build timeout exceeded"), true
}

return nil, false
}

func formatBuildError(ctx context.Context, err error, buff *bytes.Buffer) error {
if buff.Len() > 0 {
return newBuildError(buff.String())
}

newErr, ok := checkContextErrors(err)
if ok {
return newErr
}

newErr, ok = checkContextErrors(ctx.Err())
if ok {
return newErr
}

return newBuildError("Build process returned an error: %s", err)
}
26 changes: 22 additions & 4 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import (
)

const (
DefaultWriteTimeout = 60 * time.Second
DefaultReadTimeout = 15 * time.Second
DefaultIdleTimeout = 90 * time.Second
DefaultWriteTimeout = 60 * time.Second
DefaultReadTimeout = 15 * time.Second
DefaultIdleTimeout = 90 * time.Second
DefaultGoBuildTimeout = 40 * time.Second
DefaultCleanInterval = 10 * time.Minute
)

type HTTPConfig struct {
Expand Down Expand Up @@ -71,6 +73,9 @@ type BuildConfig struct {
// CleanupInterval is WebAssembly build artifact cache clean interval
CleanupInterval time.Duration `envconfig:"APP_CLEAN_INTERVAL" json:"cleanupInterval"`

// GoBuildTimeout is Go program build timeout.
GoBuildTimeout time.Duration `envconfig:"APP_GO_BUILD_TIMEOUT" json:"goBuildTimeout"`

// SkipModuleCleanup disables Go module cache cleanup.
SkipModuleCleanup bool `envconfig:"APP_SKIP_MOD_CLEANUP" json:"skipModuleCleanup"`

Expand All @@ -85,7 +90,8 @@ func (cfg *BuildConfig) mountFlagSet(f *flag.FlagSet) {
f.StringVar(&cfg.PackagesFile, "f", "packages.json", "Path to packages index JSON file")
f.StringVar(&cfg.BuildDir, "wasm-build-dir", os.TempDir(), "Directory for WASM builds")
f.BoolVar(&cfg.SkipModuleCleanup, "skip-mod-clean", false, "Skip Go module cache cleanup")
f.DurationVar(&cfg.CleanupInterval, "clean-interval", 10*time.Minute, "Build directory cleanup interval")
f.DurationVar(&cfg.CleanupInterval, "clean-interval", DefaultCleanInterval, "Build directory cleanup interval")
f.DurationVar(&cfg.GoBuildTimeout, "go-build-timeout", DefaultGoBuildTimeout, "Go program build timeout.")
f.Var(cmdutil.NewStringsListValue(&cfg.BypassEnvVarsList), "permit-env-vars", "Comma-separated allow list of environment variables passed to Go compiler tool")
}

Expand All @@ -106,6 +112,18 @@ type Config struct {
Services ServicesConfig `json:"services"`
}

// Validate validates a config and returns error if config is invalid.
func (cfg Config) Validate() error {
if cfg.Build.GoBuildTimeout > cfg.HTTP.WriteTimeout {
return fmt.Errorf(
"go build timeout (%s) exceeds HTTP response timeout (%s)",
cfg.Build.GoBuildTimeout, cfg.HTTP.WriteTimeout,
)
}

return nil
}

// FromFlagSet returns config file which will read values from flags
// when flag.Parse will be called.
func FromFlagSet(f *flag.FlagSet) *Config {
Expand Down
50 changes: 50 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"flag"
"fmt"
"testing"
"time"

Expand All @@ -28,6 +29,7 @@ func TestFromFlags(t *testing.T) {
CleanupInterval: 1 * time.Hour,
BypassEnvVarsList: []string{"FOO", "BAR"},
SkipModuleCleanup: true,
GoBuildTimeout: 4 * time.Second,
},
Services: ServicesConfig{GoogleAnalyticsID: "GA-123456"},
Log: LogConfig{
Expand Down Expand Up @@ -60,6 +62,7 @@ func TestFromFlags(t *testing.T) {
"-http-read-timeout=7s",
"-http-write-timeout=6s",
"-http-idle-timeout=3s",
"-go-build-timeout=4s",
}

fl := flag.NewFlagSet("app", flag.PanicOnError)
Expand Down Expand Up @@ -118,6 +121,7 @@ func TestFromEnv(t *testing.T) {
CleanupInterval: 1 * time.Hour,
BypassEnvVarsList: []string{"FOO", "BAR"},
SkipModuleCleanup: true,
GoBuildTimeout: time.Hour,
},
Services: ServicesConfig{GoogleAnalyticsID: "GA-123456"},
Log: LogConfig{
Expand Down Expand Up @@ -151,6 +155,7 @@ func TestFromEnv(t *testing.T) {
"HTTP_READ_TIMEOUT": "21s",
"HTTP_WRITE_TIMEOUT": "22s",
"HTTP_IDLE_TIMEOUT": "23s",
"APP_GO_BUILD_TIMEOUT": "1h",
},
},
}
Expand All @@ -173,3 +178,48 @@ func TestFromEnv(t *testing.T) {
})
}
}

func TestConfig_Validate(t *testing.T) {
cases := map[string]struct {
cfg func(t *testing.T) Config
expectErr string
}{
"default config is valid": {
cfg: func(t *testing.T) Config {
fset := flag.NewFlagSet("foo", flag.PanicOnError)
cfg := FromFlagSet(fset)
require.NoError(t, fset.Parse([]string{"foo"}))
require.NotNil(t, cfg)
return *cfg
},
},
"go build timeout": {
expectErr: fmt.Sprintf(
"go build timeout (%s) exceeds HTTP response timeout (%s)",
time.Hour, time.Second,
),
cfg: func(_ *testing.T) Config {
return Config{
Build: BuildConfig{
GoBuildTimeout: time.Hour,
},
HTTP: HTTPConfig{
WriteTimeout: time.Second,
},
}
},
},
}

for n, c := range cases {
t.Run(n, func(t *testing.T) {
err := c.cfg(t).Validate()
if c.expectErr == "" {
require.NoError(t, err)
return
}

require.EqualError(t, err, c.expectErr)
})
}
}
49 changes: 31 additions & 18 deletions internal/server/handler_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,31 @@ import (

var ErrEmptyRequest = errors.New("empty request")

type APIv2HandlerConfig struct {
Client *goplay.Client
Builder builder.BuildService
BuildTimeout time.Duration
}

func (cfg APIv2HandlerConfig) buildContext(parentCtx context.Context) (context.Context, context.CancelFunc) {
if cfg.BuildTimeout == 0 {
return parentCtx, func() {}
}

return context.WithDeadline(parentCtx, time.Now().Add(cfg.BuildTimeout))
}

type APIv2Handler struct {
logger *zap.Logger
compiler builder.BuildService
client *goplay.Client
limiter *rate.Limiter
logger *zap.Logger
limiter *rate.Limiter
cfg APIv2HandlerConfig
}

func NewAPIv2Handler(client *goplay.Client, builder builder.BuildService) *APIv2Handler {
func NewAPIv2Handler(cfg APIv2HandlerConfig) *APIv2Handler {
return &APIv2Handler{
logger: zap.L().Named("api.v2"),
compiler: builder,
client: client,
limiter: rate.NewLimiter(rate.Every(frameTime), compileRequestsPerFrame),
logger: zap.L().Named("api.v2"),
cfg: cfg,
limiter: rate.NewLimiter(rate.Every(frameTime), compileRequestsPerFrame),
}
}

Expand All @@ -38,7 +50,7 @@ func (h *APIv2Handler) HandleGetSnippet(w http.ResponseWriter, r *http.Request)
vars := mux.Vars(r)
snippetID := vars["id"]

snippet, err := h.client.GetSnippet(r.Context(), snippetID)
snippet, err := h.cfg.Client.GetSnippet(r.Context(), snippetID)
if err != nil {
if errors.Is(err, goplay.ErrSnippetNotFound) {
return Errorf(http.StatusNotFound, "snippet %q not found", snippetID)
Expand Down Expand Up @@ -76,7 +88,7 @@ func (h *APIv2Handler) HandleShare(w http.ResponseWriter, r *http.Request) error
return err
}

snippetID, err := h.client.Share(ctx, payload.Reader())
snippetID, err := h.cfg.Client.Share(ctx, payload.Reader())
if err != nil {
return err
}
Expand Down Expand Up @@ -108,7 +120,7 @@ func (h *APIv2Handler) HandleFormat(w http.ResponseWriter, r *http.Request) erro
return err
}

rsp, err := h.client.GoImports(ctx, payload.Bytes(), backend)
rsp, err := h.cfg.Client.GoImports(ctx, payload.Bytes(), backend)
if err != nil {
if isContentLengthError(err) {
return ErrSnippetTooLarge
Expand Down Expand Up @@ -148,7 +160,7 @@ func (h *APIv2Handler) HandleRun(w http.ResponseWriter, r *http.Request) error {
return NewBadRequestError(err)
}

res, err := h.client.Evaluate(ctx, goplay.CompileRequest{
res, err := h.cfg.Client.Evaluate(ctx, goplay.CompileRequest{
Version: goplay.DefaultVersion,
WithVet: params.Vet,
Body: snippet,
Expand All @@ -172,7 +184,7 @@ func (h *APIv2Handler) HandleRun(w http.ResponseWriter, r *http.Request) error {
// HandleCompile handles WebAssembly compile requests.
func (h *APIv2Handler) HandleCompile(w http.ResponseWriter, r *http.Request) error {
// Limit for request timeout
ctx, cancel := context.WithDeadline(r.Context(), time.Now().Add(maxBuildTimeDuration))
ctx, cancel := h.cfg.buildContext(r.Context())
defer cancel()

// Wait for our queue in line for compilation
Expand All @@ -185,11 +197,12 @@ func (h *APIv2Handler) HandleCompile(w http.ResponseWriter, r *http.Request) err
return err
}

result, err := h.compiler.Build(ctx, files)
if builder.IsBuildError(err) {
return NewHTTPError(http.StatusBadRequest, err)
}
result, err := h.cfg.Builder.Build(ctx, files)
if err != nil {
if builder.IsBuildError(err) || errors.Is(err, context.Canceled) {
return NewHTTPError(http.StatusBadRequest, err)
}

return err
}

Expand Down

0 comments on commit 77cf7fc

Please sign in to comment.