Block scale to zero during recording, allow multiple recordings #29
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| func (c *unikraftCloudController) Disable(ctx context.Context) error { | ||
| return c.write(ctx, "+") | ||
| } | ||
|
|
||
| func (c *unikraftCloudController) Enable(ctx context.Context) error { | ||
| return c.write(ctx, "-") | ||
| } | ||
|
|
||
| func (c *unikraftCloudController) write(ctx context.Context, char string) error { | ||
| if _, err := os.Stat(c.path); err != nil { | ||
| if os.IsNotExist(err) { | ||
| return nil | ||
| } | ||
| logger.FromContext(ctx).Error("failed to stat scale-to-zero control file", "path", c.path, "err", err) | ||
| return err | ||
| } | ||
|
|
||
| f, err := os.OpenFile(c.path, os.O_WRONLY|os.O_TRUNC, 0) | ||
| if err != nil { | ||
| logger.FromContext(ctx).Error("failed to open scale-to-zero control file", "path", c.path, "err", err) | ||
| return err | ||
| } | ||
| defer f.Close() | ||
| if _, err := f.Write([]byte(char)); err != nil { | ||
| logger.FromContext(ctx).Error("failed to write scale-to-zero control file", "path", c.path, "err", err) | ||
| return err | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Sayan-
force-pushed
the
sayan/scaletozero
branch
from
ba5206f to
0908bb9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
removal of |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Sayan-
force-pushed
the
sayan/scaletozero
branch
from
c026bc5 to
346376e
Compare
Sayan-
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
🚨 BugBot couldn't runSomething went wrong. Try again by commenting "bugbot run", or contact support (requestId: serverGenReqId_1eefc081-5ee3-4a1a-9905-acec10813503). |
🚨 BugBot couldn't runSomething went wrong. Try again by commenting "bugbot run", or contact support (requestId: serverGenReqId_ef950486-8c2c-4276-9203-59d76ad18550). |
There was a problem hiding this comment.
Bug: Recorder List Method Misnamed
The FFmpegManager.ListActiveRecorders method is misnamed. It returns all registered recorders instead of only active (currently recording) ones, as its name implies. This is a regression, as the previous implementation correctly filtered for active recorders using IsRecording(). This mismatch between the method's name and its behavior breaks its implied contract and can lead to developer confusion.
server/lib/recorder/ffmpeg.go#L424-L434
Was this report helpful? Give feedback by reacting with 👍 or 👎
Description
As we've stress tested the recording server more we've noticed the behavior from pause <> resume to be a bit more unpredictable than we'd like. Instead we're opt'ing to leverage the pseudo file detailed in the unikraft docs to disable scale to zero while we're recording.
Stopbut the browser instance will stay live + recording for the duration between[Start, Stop]ffmpegis happy with this in both runtimes. Note: we expect names to be unique and we will reject any duplicates.ffmpegthat will work in conjunction with the existing max size as a guardrail.The main thing to call out here is I've pushed the implementation of the scale to zero down into the recorder so it will be re-enabled as soon as possible (e.g. the moment the recording is complete or the user has explicitly called to
Stoprecording. These may race which is why I've wrapped this in a sync.Once to make it more forgiving)Testing
Tested with the end to end automation suite. The expected behavior that the instance would stay in
runninguntil shutdown was explicitly called was confirmed