compress qlogs when the QUIC connection is closed #193
Conversation
tracer.go
Outdated
| if err := l.f.Close(); err != nil { | ||
| return err | ||
| } | ||
| if err := os.Remove(l.f.Name()); err != nil { |
There was a problem hiding this comment.
Isn't this error handling beautiful? 🤢
|
Seems to work: |
| WriteCloser: newBufferedWriteCloser(bufio.NewWriter(gz), gz), | ||
| f: f, | ||
| filename: finalFilename, | ||
| Writer: bufio.NewWriter(f), | ||
| } | ||
| } | ||
|
|
||
| func (l *qlogger) Close() error { |
There was a problem hiding this comment.
Can we add some defers for closing/deleting here? Ideally, close would return after closing everything, even if something fails along the way.
There was a problem hiding this comment.
Makes sense, although using defer means that we won't be able to handle the error (without adding a ton of boilerplate code).
There was a problem hiding this comment.
A helper Compress function to compress/copy would help a bit. Also, you may find https://pkg.go.dev/go.uber.org/multierr#Append useful.
But I don't care strongly any which way.
There was a problem hiding this comment.
I think it's ok that we're ignoring the error here. If closing the file fails, there's really nothing we can do about it anyway.
marten-seemann
force-pushed
the
deferred-qlog-compression
branch
3 times, most recently
from
baa1bb1
to
40fecb3
Compare
marten-seemann
force-pushed
the
deferred-qlog-compression
branch
from
40fecb3
to
fe226b7
Compare
Fixes #191. Fixes #192.
I hate that we to make this change, but I don't see any other solution. Compression algorithms keep a lot of internal state, so we shouldn't have too many of them running in parallel. This was already an issue when using gzip (see #192), which got exacerbated by using zstd.
The alternative is to use the file system as a buffer: We qlog into a temporary file
.<name>.qlog.swp. Once the QUIC connection is closed, we compress that file to<name>.qlog.zst.