Skip to content
Snippets Groups Projects
Commit 9b98413f authored by lennart.kramer's avatar lennart.kramer
Browse files

improve error handling and logging

Sometimes under load, the internal timer in the server would
executed substantially later than the pipe reads/writes and
so the deadlines from the read/writes would end first and
this error was not handled and returned an 500 error.
This was changed so that it now properly returns an 416 error.

Additionally, for some error logging, the input and timeout is
also given.
parent 4416828c
No related branches found
No related tags found
No related merge requests found
...@@ -4,6 +4,7 @@ import ( ...@@ -4,6 +4,7 @@ import (
"archive/zip" "archive/zip"
"bytes" "bytes"
"bufio" "bufio"
"errors"
"fmt" "fmt"
"path/filepath" "path/filepath"
"io" "io"
...@@ -191,14 +192,14 @@ func (p *ChildProcess) eval_command(command string, timeout uint64) (*bytes.Buff ...@@ -191,14 +192,14 @@ func (p *ChildProcess) eval_command(command string, timeout uint64) (*bytes.Buff
in_err := make(chan error, 1) in_err := make(chan error, 1)
// write to stdin in separate goroutine to prevent deadlocks // write to stdin in separate goroutine to prevent deadlocks
go func() { go func() {
p.Input.SetWriteDeadline(time.Now().Add(time.Duration(timeout)*time.Millisecond)) p.Input.SetWriteDeadline(time.Now().Add(time.Duration(timeout + 10)*time.Millisecond))
_, err := io.Copy(p.Input, strings.NewReader(command)) _, err := io.Copy(p.Input, strings.NewReader(command))
p.Input.Close() p.Input.Close()
in_err<-err in_err<-err
}() }()
var outbuf bytes.Buffer var outbuf bytes.Buffer
// read from stdout // read from stdout
p.Outfile.SetReadDeadline(time.Now().Add(time.Duration(timeout)*time.Millisecond)) p.Outfile.SetReadDeadline(time.Now().Add(time.Duration(timeout + 10)*time.Millisecond))
_, err := io.Copy(&outbuf, p.Output) _, err := io.Copy(&outbuf, p.Output)
p.Outfile.Close() p.Outfile.Close()
input_err := <-in_err input_err := <-in_err
...@@ -286,7 +287,20 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess, ...@@ -286,7 +287,20 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess,
proc := <-queue proc := <-queue
metrics.QueueLen.Dec() metrics.QueueLen.Dec()
user := proc.User user := proc.User
log_with_input := func(format string, a ...interface{}) {
msg := fmt.Sprintf(format, a...)
log.Printf("%s - input: `%s`, timeout: %d", msg, input, timeout);
}
debugf("Debug: input (%d): %s", user.Id, input) debugf("Debug: input (%d): %s", user.Id, input)
write_timeout_err := func() {
w.WriteHeader(http.StatusRequestedRangeNotSatisfiable)
fmt.Fprint(w, "416 - timeout\n")
log_with_input("Warn: Process %s had timeout", user.Name)
metrics.ResponseTime.Observe(float64(timeout))
metrics.NumTimeout.Inc()
}
defer process_cleanup(user, user_queue, proc.TempDir) defer process_cleanup(user, user_queue, proc.TempDir)
proc_out := make(chan struct {buf *bytes.Buffer; time float64; err error}, 1) proc_out := make(chan struct {buf *bytes.Buffer; time float64; err error}, 1)
go func() { go func() {
...@@ -299,10 +313,15 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess, ...@@ -299,10 +313,15 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess,
outbuf := outstr.buf outbuf := outstr.buf
tim := outstr.time tim := outstr.time
err := outstr.err err := outstr.err
if errors.Is(err, os.ErrDeadlineExceeded) {
debugf("Timeout with I/O pipe timeout")
write_timeout_err()
return
}
if err != nil { if err != nil {
write_500(w) write_500(w)
metrics.NumIntError.Inc() metrics.NumIntError.Inc()
log.Printf("Error: Communicating with maxima failed: %s", err) log_with_input("Error: Communicating with maxima failed: %s", err)
return return
} }
if health { if health {
...@@ -316,7 +335,7 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess, ...@@ -316,7 +335,7 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess,
} else { } else {
write_500(w) write_500(w)
metrics.NumIntError.Inc() metrics.NumIntError.Inc()
log.Printf("Error: Healthcheck did not pass") log.Printf("Error: Healthcheck did not pass, output: %s", outbuf)
return return
} }
} }
...@@ -328,7 +347,7 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess, ...@@ -328,7 +347,7 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess,
// just return text if directory could not be read and assume no plots were generated // just return text if directory could not be read and assume no plots were generated
w.Header().Set("Content-Type", "text/plain;charset=UTF-8") w.Header().Set("Content-Type", "text/plain;charset=UTF-8")
outbuf.WriteTo(w) outbuf.WriteTo(w)
log.Printf("Warn: could not read temp directory of maxima process: %s", err) log_with_input("Warn: could not read temp directory of maxima process: %s", err)
metrics.ResponseTime.Observe(tim) metrics.ResponseTime.Observe(tim)
metrics.NumSuccess.Inc() metrics.NumSuccess.Inc()
} }
...@@ -345,7 +364,7 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess, ...@@ -345,7 +364,7 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess,
if err != nil { if err != nil {
write_500(w) write_500(w)
metrics.NumIntError.Inc() metrics.NumIntError.Inc()
log.Printf("Error: Could not add OUTPUT to zip archive: %s", err) log_with_input("Error: Could not add OUTPUT to zip archive: %s", err)
return return
} }
outbuf.WriteTo(out) outbuf.WriteTo(out)
...@@ -357,14 +376,14 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess, ...@@ -357,14 +376,14 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess,
if err != nil { if err != nil {
write_500(w) write_500(w)
metrics.NumIntError.Inc() metrics.NumIntError.Inc()
log.Printf("Error: could not open plot %s: %s", file.Name(), err) log_with_input("Error: could not open plot %s: %s", file.Name(), err)
return return
} }
fzipput, err := zipfile.Create("/" + file.Name()) fzipput, err := zipfile.Create("/" + file.Name())
if err != nil { if err != nil {
write_500(w) write_500(w)
metrics.NumIntError.Inc() metrics.NumIntError.Inc()
log.Printf("Error: could not add file %s to zip archive: %s", file.Name(), err) log_with_input("Error: could not add file %s to zip archive: %s", file.Name(), err)
return return
} }
io.Copy(fzipput, ffilein) io.Copy(fzipput, ffilein)
...@@ -374,11 +393,9 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess, ...@@ -374,11 +393,9 @@ func handler(w http.ResponseWriter, r *http.Request, queue <-chan *ChildProcess,
metrics.ResponseTime.Observe(tim) metrics.ResponseTime.Observe(tim)
metrics.NumSuccess.Inc() metrics.NumSuccess.Inc()
case <-time.After(time.Duration(timeout) * time.Millisecond): case <-time.After(time.Duration(timeout) * time.Millisecond):
w.WriteHeader(http.StatusRequestedRangeNotSatisfiable) debugf("Timeout with internal timer")
fmt.Fprint(w, "416 - timeout\n") write_timeout_err()
log.Printf("Warn: Process %s had timeout", user.Name) return
metrics.ResponseTime.Observe(float64(timeout))
metrics.NumTimeout.Inc()
} }
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment