diff --git a/pkg/cmd/streamplace.go b/pkg/cmd/streamplace.go index fcdfdde9..eddd91a2 100644 --- a/pkg/cmd/streamplace.go +++ b/pkg/cmd/streamplace.go @@ -562,6 +562,15 @@ func runMain(ctx context.Context, build *config.BuildFlags, platformJobs []jobFu return storage.StartSegmentCleaner(ctx, ldb, cli) }) + // Salvage sweep for debug recordings: uploads any local spools the live + // upload path couldn't commit (stalled S3, crashed worker) — including + // recordings from before the spool existed — then deletes them locally. + if cli.S3Configured() { + group.Go(func() error { + return cli.DebugRecordingSweeper(ctx) + }) + } + if cli.LegacySegmentCleaner { group.Go(func() error { return ldb.StartSegmentCleaner(ctx) diff --git a/pkg/config/config.go b/pkg/config/config.go index 107157e3..d432742f 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1193,14 +1193,21 @@ func (cli *CLI) DataFilePath(fpath []string) string { if cli.DataDir == "" { panic("no data dir configured") } - // windows does not like colons - safe := []string{} + fpath = append([]string{cli.DataDir}, sanitizePathParts(fpath)...) + fdpath := filepath.Join(fpath...) + return fdpath +} + +// sanitizePathParts replaces ":" with "-" in each path component (windows does +// not like colons). Debug-recording S3 keys use the same sanitization so the +// bucket layout matches the on-disk layout byte for byte — that's what lets +// the salvage sweep map a leftover local file straight to its object key. +func sanitizePathParts(fpath []string) []string { + safe := make([]string, 0, len(fpath)) for _, p := range fpath { safe = append(safe, strings.ReplaceAll(p, ":", "-")) } - fpath = append([]string{cli.DataDir}, safe...) - fdpath := filepath.Join(fpath...) - return fdpath + return safe } // does a file exist in our data dir? @@ -1460,22 +1467,33 @@ type DebugRecordingFile interface { } // DebugRecordingCreate opens a write target for a debug recording (RTMP/MKV -// dumps, WHIP rtcrec sessions). When S3 is configured the recording streams to -// an S3 object at the key formed by joining fpath with "/" (so the bucket -// mirrors the on-disk debug-recordings// layout); otherwise it falls -// back to a local file under DataDir — the dev default. The returned value must -// be Closed to finalize (Close commits the S3 upload). overwrite only affects -// the local-disk path (S3 puts always overwrite). +// dumps, WHIP rtcrec sessions). The recording always lands in a local file +// under DataDir first — the source of truth. When S3 is configured, a copy +// also streams best-effort to an object whose key mirrors the on-disk +// debug-recordings// layout (same ":"-sanitization); Close commits +// that upload and removes the local spool on success. Any S3 failure — at +// open, mid-stream, or at commit — leaves the local file in place for +// SweepDebugRecordings to upload later, so a stalled or dead S3 never loses a +// recording. overwrite only affects the local file (S3 puts always overwrite). func (cli *CLI) DebugRecordingCreate(ctx context.Context, fpath []string, contentType string, overwrite bool) (DebugRecordingFile, error) { - if cli.S3Configured() { - key := strings.Join(fpath, "/") - // The recording outlives the ingest session's ctx: Close commits the upload - // during teardown, after that ctx is typically cancelled — a cancelled ctx - // here would abort the upload and lose the object. Callers bound the commit - // with their own finalize waits instead. - return s3.NewUploadWriter(context.WithoutCancel(ctx), s3.NewClient(cli.S3Config()), cli.S3Bucket, key, contentType) + local, err := cli.DataFileCreate(fpath, overwrite) + if err != nil { + return nil, err + } + if !cli.S3Configured() { + return local, nil + } + key := strings.Join(sanitizePathParts(fpath), "/") + // The recording outlives the ingest session's ctx: Close commits the upload + // during teardown, after that ctx is typically cancelled — a cancelled ctx + // here would abort the upload. Callers bound the commit with their own + // finalize waits; per-op timeouts in pkg/s3 bound a genuine stall. + s3w, err := s3.NewUploadWriter(context.WithoutCancel(ctx), s3.NewClient(cli.S3Config()), cli.S3Bucket, key, contentType) + if err != nil { + log.Error(ctx, "debug recording S3 upload could not start; recording to local spool only", "path", local.Name(), "error", err) + return local, nil } - return cli.DataFileCreate(fpath, overwrite) + return &spooledRecording{ctx: context.WithoutCancel(ctx), local: local, s3w: s3w}, nil } func (cli *CLI) ShouldSyndicate(did string) bool { diff --git a/pkg/config/debug_recording.go b/pkg/config/debug_recording.go new file mode 100644 index 00000000..d8a1f5b1 --- /dev/null +++ b/pkg/config/debug_recording.go @@ -0,0 +1,169 @@ +package config + +import ( + "context" + "io" + "os" + "path/filepath" + "strings" + "time" + + "stream.place/streamplace/pkg/log" + "stream.place/streamplace/pkg/s3" +) + +// spooledRecording is the S3-configured DebugRecordingFile: every write lands +// in the local spool file (the source of truth) and streams best-effort to S3. +// Close commits the upload and removes the spool on success; on any S3 failure +// the spool survives for SweepDebugRecordings to upload later. S3 trouble is +// therefore never a write/Close error — the recording is safe on disk. +type spooledRecording struct { + ctx context.Context + local *os.File + s3w *s3.UploadWriter + s3err error // first S3 failure; once set, S3 is out of the picture +} + +func (w *spooledRecording) Name() string { return w.local.Name() } + +func (w *spooledRecording) Write(p []byte) (int, error) { + if w.s3err == nil { + if _, err := w.s3w.Write(p); err != nil { + w.s3err = err + log.Error(w.ctx, "debug recording S3 upload failed mid-stream; keeping local spool", "path", w.local.Name(), "error", err) + // The partial multipart upload is useless — the sweep re-uploads the + // whole spool later. Abort is bounded by pkg/s3's per-op timeouts. + if aerr := w.s3w.Abort(); aerr != nil { + log.Error(w.ctx, "abort of failed debug recording upload", "error", aerr) + } + } + } + return w.local.Write(p) +} + +func (w *spooledRecording) Close() error { + lerr := w.local.Close() + if w.s3err == nil { + w.s3err = w.s3w.Close() + } + if w.s3err != nil { + log.Error(w.ctx, "debug recording not committed to S3; keeping local spool for the salvage sweep", "path", w.local.Name(), "error", w.s3err) + return lerr + } + // Committed to S3 — the spool has served its purpose. (Even if lerr != nil: + // os.File writes are unbuffered syscalls, so a Close error doesn't mean the + // S3 copy is short; keeping the spool would just make the sweep re-upload.) + if err := os.Remove(w.local.Name()); err != nil { + log.Warn(w.ctx, "could not remove committed debug recording spool", "path", w.local.Name(), "error", err) + } + log.Log(w.ctx, "debug recording committed to S3", "key", w.s3w.Name()) + return lerr +} + +// debugRecordingSweepInterval is how often the sweeper looks for leftovers; +// debugRecordingSweepIdle is how long a file must sit unmodified before it's +// considered dead. An active recording's mtime advances with every write, and +// wedged workers are torn down by their watchdogs well inside the idle window, +// so anything idle this long is a leftover: a stalled/failed live upload, a +// crashed worker, or a recording from the era when S3 upload didn't work. +const ( + debugRecordingSweepInterval = 15 * time.Minute + debugRecordingSweepIdle = 15 * time.Minute +) + +// DebugRecordingSweeper runs SweepDebugRecordings periodically (and once at +// startup) until ctx is done. Run it in main when S3 is configured — this is +// the salvage half of the local-spool durability story: whatever the live +// upload path couldn't commit, the sweep eventually does. +func (cli *CLI) DebugRecordingSweeper(ctx context.Context) error { + ticker := time.NewTicker(debugRecordingSweepInterval) + defer ticker.Stop() + for { + cli.SweepDebugRecordings(ctx) + select { + case <-ctx.Done(): + return nil + case <-ticker.C: + } + } +} + +// SweepDebugRecordings uploads leftover local debug recordings to S3 and +// removes them once committed. The on-disk layout under +// DataDir/debug-recordings mirrors the object-key layout exactly (both are +// ":"-sanitized), so a file's path relative to DataDir IS its key. Files +// modified within debugRecordingSweepIdle are skipped — they may still be +// written by an active worker. +func (cli *CLI) SweepDebugRecordings(ctx context.Context) { + root := cli.DataFilePath([]string{"debug-recordings"}) + entries := []string{} + err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() { + return err + } + info, ierr := d.Info() + if ierr != nil { + return ierr + } + if time.Since(info.ModTime()) < debugRecordingSweepIdle { + return nil + } + entries = append(entries, path) + return nil + }) + if err != nil { + if !os.IsNotExist(err) { + log.Error(ctx, "debug recording sweep: walk", "root", root, "error", err) + } + return + } + for _, path := range entries { + if ctx.Err() != nil { + return + } + if err := cli.sweepOneRecording(ctx, path); err != nil { + log.Error(ctx, "debug recording sweep: upload failed; leaving spool for the next sweep", "path", path, "error", err) + } + } +} + +func (cli *CLI) sweepOneRecording(ctx context.Context, path string) error { + rel, err := filepath.Rel(cli.DataFilePath(nil), path) + if err != nil { + return err + } + key := filepath.ToSlash(rel) + fd, err := os.Open(path) + if err != nil { + return err + } + defer fd.Close() + w, err := s3.NewUploadWriter(ctx, s3.NewClient(cli.S3Config()), cli.S3Bucket, key, debugRecordingContentType(path)) + if err != nil { + return err + } + if _, err := io.Copy(w, fd); err != nil { + if aerr := w.Abort(); aerr != nil { + log.Error(ctx, "debug recording sweep: abort", "key", key, "error", aerr) + } + return err + } + if err := w.Close(); err != nil { + return err + } + if err := os.Remove(path); err != nil { + return err + } + log.Log(ctx, "debug recording sweep: salvaged recording to S3", "key", key) + return nil +} + +func debugRecordingContentType(path string) string { + switch strings.ToLower(filepath.Ext(path)) { + case ".mkv": + return "video/x-matroska" + case ".cbor": + return "application/cbor" + } + return "application/octet-stream" +} diff --git a/pkg/config/debug_recording_test.go b/pkg/config/debug_recording_test.go new file mode 100644 index 00000000..72c7c766 --- /dev/null +++ b/pkg/config/debug_recording_test.go @@ -0,0 +1,164 @@ +package config + +import ( + "context" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// fakeS3 speaks just enough path-style multipart-upload S3 for UploadWriter: +// initiate → parts → complete. failComplete makes CompleteMultipartUpload 500, +// simulating an S3 that took the parts but won't commit. +type fakeS3 struct { + mu sync.Mutex + parts map[string][]byte + objects map[string][]byte + failComplete bool +} + +func newFakeS3() *fakeS3 { + return &fakeS3{parts: map[string][]byte{}, objects: map[string][]byte{}} +} + +func (f *fakeS3) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + path := strings.TrimPrefix(r.URL.Path, "/") + q := r.URL.Query() + switch { + case r.Method == "POST" && q.Has("uploads"): + fmt.Fprintf(w, `test-upload`) + case r.Method == "PUT" && q.Has("partNumber"): + body, _ := io.ReadAll(r.Body) + f.parts[path+"#"+q.Get("partNumber")] = body + w.Header().Set("ETag", `"part-`+q.Get("partNumber")+`"`) + case r.Method == "POST" && q.Has("uploadId"): + if f.failComplete { + w.WriteHeader(http.StatusInternalServerError) + return + } + var buf []byte + for i := 1; ; i++ { + part, ok := f.parts[fmt.Sprintf("%s#%d", path, i)] + if !ok { + break + } + buf = append(buf, part...) + } + f.objects[path] = buf + fmt.Fprintf(w, `%s`, path) + case r.Method == "DELETE": + // AbortMultipartUpload + default: + w.WriteHeader(http.StatusBadRequest) + } +} + +func (f *fakeS3) object(path string) ([]byte, bool) { + f.mu.Lock() + defer f.mu.Unlock() + b, ok := f.objects[path] + return b, ok +} + +func newS3TestCLI(t *testing.T, endpoint string) *CLI { + return &CLI{ + DataDir: t.TempDir(), + S3Endpoint: endpoint, + S3Bucket: "bkt", + S3AccessKeyID: "test-access", + S3SecretAccessKey: "test-secret", + S3Region: "auto", + } +} + +// TestDebugRecordingSpoolCommit: happy path — the recording streams to S3 (at +// the sanitized key mirroring the on-disk layout) and the local spool is +// removed once the upload commits. +func TestDebugRecordingSpoolCommit(t *testing.T) { + fake := newFakeS3() + srv := httptest.NewServer(fake) + defer srv.Close() + cli := newS3TestCLI(t, srv.URL) + + fpath := []string{"debug-recordings", "did:key:zTest", "rec.rtmp.mkv"} + f, err := cli.DebugRecordingCreate(context.Background(), fpath, "video/x-matroska", false) + require.NoError(t, err) + data := []byte("pretend this is mkv data") + _, err = f.Write(data) + require.NoError(t, err) + require.NoError(t, f.Close()) + + got, ok := fake.object("bkt/debug-recordings/did-key-zTest/rec.rtmp.mkv") + require.True(t, ok, "object committed at the sanitized key") + require.Equal(t, data, got) + _, err = os.Stat(cli.DataFilePath(fpath)) + require.True(t, os.IsNotExist(err), "spool removed after commit") +} + +// TestDebugRecordingSpoolSurvivesFailedCommit: when S3 won't commit, Close is +// not an error — the local spool survives, intact, for the sweep. +func TestDebugRecordingSpoolSurvivesFailedCommit(t *testing.T) { + fake := newFakeS3() + fake.failComplete = true + srv := httptest.NewServer(fake) + defer srv.Close() + cli := newS3TestCLI(t, srv.URL) + + fpath := []string{"debug-recordings", "did:key:zTest", "rec.rtmp.mkv"} + f, err := cli.DebugRecordingCreate(context.Background(), fpath, "video/x-matroska", false) + require.NoError(t, err) + data := []byte("pretend this is mkv data") + _, err = f.Write(data) + require.NoError(t, err) + require.NoError(t, f.Close(), "a failed S3 commit is not a recording error") + + got, err := os.ReadFile(cli.DataFilePath(fpath)) + require.NoError(t, err, "spool survives the failed commit") + require.Equal(t, data, got) + _, ok := fake.object("bkt/debug-recordings/did-key-zTest/rec.rtmp.mkv") + require.False(t, ok, "no object committed") +} + +// TestSweepDebugRecordings: idle leftovers get uploaded at the key their path +// dictates and deleted; a freshly-written file (possibly still being recorded) +// is left alone. +func TestSweepDebugRecordings(t *testing.T) { + fake := newFakeS3() + srv := httptest.NewServer(fake) + defer srv.Close() + cli := newS3TestCLI(t, srv.URL) + + dir := cli.DataFilePath([]string{"debug-recordings", "did-key-zOld"}) + require.NoError(t, os.MkdirAll(dir, 0755)) + stale := filepath.Join(dir, "stale.rtmp.mkv") + staleData := []byte("leftover recording bytes") + require.NoError(t, os.WriteFile(stale, staleData, 0644)) + old := time.Now().Add(-time.Hour) + require.NoError(t, os.Chtimes(stale, old, old)) + + fresh := filepath.Join(dir, "fresh.rtmp.mkv") + require.NoError(t, os.WriteFile(fresh, []byte("still being written"), 0644)) + + cli.SweepDebugRecordings(context.Background()) + + got, ok := fake.object("bkt/debug-recordings/did-key-zOld/stale.rtmp.mkv") + require.True(t, ok, "stale spool salvaged to S3") + require.Equal(t, staleData, got) + _, err := os.Stat(stale) + require.True(t, os.IsNotExist(err), "salvaged spool removed") + _, err = os.Stat(fresh) + require.NoError(t, err, "fresh file untouched") + _, ok = fake.object("bkt/debug-recordings/did-key-zOld/fresh.rtmp.mkv") + require.False(t, ok, "fresh file not uploaded") +} diff --git a/pkg/media/ingest_worker_test.go b/pkg/media/ingest_worker_test.go index 74336d8f..0c8aa114 100644 --- a/pkg/media/ingest_worker_test.go +++ b/pkg/media/ingest_worker_test.go @@ -327,9 +327,11 @@ func TestRunMKVIngestWorkerRecordsToS3(t *testing.T) { return ok && bytes.Equal(got, mkv) }, 10*time.Second, 25*time.Millisecond, "worker streams the recording to the S3 bucket verbatim") - // And nothing fell back to local disk. + // And the local spool was cleaned up after the commit (the recording spools + // to disk while live — durability against a stalled S3 — but a committed + // upload leaves nothing behind). matches, _ := filepath.Glob(filepath.Join(dataDir, "debug-recordings", "*", "*")) - require.Empty(t, matches, "recording must go to S3, not DataDir") + require.Empty(t, matches, "spool removed after S3 commit") } // TestRunMKVIngestWorkerSelfWatchdog proves the worker's OWN watchdog contains a diff --git a/pkg/s3/multipart_writer.go b/pkg/s3/multipart_writer.go index 9891030c..da388ec6 100644 --- a/pkg/s3/multipart_writer.go +++ b/pkg/s3/multipart_writer.go @@ -379,6 +379,12 @@ func (w *UploadWriter) Write(p []byte) (int, error) { return w.mw.Write(p) } // should Close exactly once). func (w *UploadWriter) Close() error { return w.mw.Complete() } +// Abort discards the upload instead of committing it — for callers that know +// the object is no longer wanted, e.g. a spooled debug recording whose S3 copy +// already failed mid-stream and will be re-uploaded whole from the local spool +// later. Safe to call after a failed Close. +func (w *UploadWriter) Abort() error { return w.mw.Abort() } + // Name reports the object key, mirroring *os.File.Name() so callers can log a // destination uniformly whether they got a file or an S3 upload. func (w *UploadWriter) Name() string { return w.key }