Repository navigation
Conversation
… the shutdown drain in compose Direct /video/convert work defaulted to one conversion at a time on any machine. The default is now half the usable CPUs (container limit aware), between 1 and 4, and MEDIA_SERVER_MAX_CONCURRENT_DIRECT_CONVERSIONS still overrides it. The compose files set MEDIA_SERVER_SHUTDOWN_GRACE_MS to 25s under a 30s stop_grace_period so main's drain lets active work finish on docker compose stop.
|
@07rjain is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const MAX_DEFAULT_DIRECT_VIDEO_PROCESSES = 4; | ||
|
|
||
| export function getDefaultMaxConcurrentDirectVideoProcesses( | ||
| cpuCapacity: number = getContainerCpuLimit() || os.availableParallelism(), |
There was a problem hiding this comment.
If a container has a finite CPU quota larger than its assigned CPU set, getContainerCpuLimit() || os.availableParallelism() ignores the smaller available count. An eight-CPU quota with a two-CPU set selects four conversions instead of one, making conversions compete for the same CPUs. Use the smaller positive quota and os.availableParallelism() value before halving it, and test that combination.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/media-server/src/lib/video-capacity.ts
Line: 16
Comment:
**CPU limits can overcount**
If a container has a finite CPU quota larger than its assigned CPU set, `getContainerCpuLimit() || os.availableParallelism()` ignores the smaller available count. An eight-CPU quota with a two-CPU set selects four conversions instead of one, making conversions compete for the same CPUs. Use the smaller positive quota and `os.availableParallelism()` value before halving it, and test that combination.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| stop_grace_period: 30s | ||
| environment: | ||
| # Finish active work for up to 25s on stop (below stop_grace_period). | ||
| MEDIA_SERVER_SHUTDOWN_GRACE_MS: "25000" |
There was a problem hiding this comment.
If a job outlasts the 25-second drain and its webhook is slow or unavailable, only five seconds remain before Docker kills the container. Cleanup starts after the drain, and abortAllJobs() can spend 16.5 seconds sending cancellation webhooks before the server reaches media and temporary-file cleanup. Docker can cut off both notifications and cleanup.
Increase stop_grace_period to cover both stages, or bound cleanup within the remaining time. Apply the same fix to docker-compose.template.yml and docker-compose.coolify.yml.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docker-compose.yml
Line: 52-55
Comment:
**Docker cuts cleanup short**
If a job outlasts the 25-second drain and its webhook is slow or unavailable, only five seconds remain before Docker kills the container. Cleanup starts after the drain, and `abortAllJobs()` can spend 16.5 seconds sending cancellation webhooks before the server reaches media and temporary-file cleanup. Docker can cut off both notifications and cleanup.
Increase `stop_grace_period` to cover both stages, or bound cleanup within the remaining time. Apply the same fix to `docker-compose.template.yml` and `docker-compose.coolify.yml`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| The media server converts uploads in parallel based on the CPUs it can use: half the available CPUs, at least 1 and at most 4. It still turns work away when CPU or memory pressure is high. Override this with: | ||
|
|
||
| ```bash | ||
| MEDIA_SERVER_MAX_CONCURRENT_DIRECT_CONVERSIONS=2 # direct /video/convert requests | ||
| MEDIA_SERVER_MAX_CONCURRENT_VIDEO_PROCESSES=4 # all video jobs | ||
| ``` |
There was a problem hiding this comment.
This page tells Compose users to put custom settings in .env, but none of the three Compose services forwards either tuning variable or loads that file with env_file. Adding the shown values to .env therefore leaves conversion limits unchanged. Show how to add them to the media-server service's environment mapping and recreate the container.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/content/docs/self-hosting.mdx
Line: 111-116
Comment:
**Compose ignores these tuning settings**
This page tells Compose users to put custom settings in `.env`, but none of the three Compose services forwards either tuning variable or loads that file with `env_file`. Adding the shown values to `.env` therefore leaves conversion limits unchanged. Show how to add them to the media-server service's `environment` mapping and recreate the container.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…cel webhooks and cleanup A 25s drain plus webhooks for jobs aborted at shutdown (an in-flight attempt up to 5s, then 3 x 5s with 1.5s backoff) needs 46.5s before cleanup, more than the 30s stop_grace_period, so Docker could SIGKILL mid-cleanup. Raise stop_grace_period to 60s in all three compose files and document the budget.
|
@greptileai please re-review the latest changes. |
Split out of #2580 (shutdown draining and capacity).
Direct conversions default to half the usable CPUs, bounded between one and four. The default uses the smaller positive container quota and assigned CPU count, so an eight-CPU quota with a two-CPU set permits one direct conversion.
MEDIA_SERVER_MAX_CONCURRENT_DIRECT_CONVERSIONSstill overrides this default, and the existing shared CPU/memory admission checks remain in effect.All three Compose variants enable main's existing drain with
MEDIA_SERVER_SHUTDOWN_GRACE_MS=25000andstop_grace_period: 60s. This allows time after draining for cancellation webhooks and cleanup. They forward both conversion tuning variables from.env; the guide includes the standard/template commands and the Coolify redeployment instructions. The audio-level capacity regression occupies every direct slot.The branch includes main at
3614bdb692af7317c30c0ad7afaa496a6974c2eb, integrating the 174 upstream commits missing from the previous head without conflicts.Validation:
exclude_from_hcfield.bun run typecheck, scoped Biome checks and media-server bundling passed. Biome does not process the MDX guide; its commands were checked against the actual service names.All three findings in the displayed Greptile review have been addressed since that review. Its 4/5 score was produced on
a9cebd578; a fresh score and maintainer approval for fork CI are still pending.
The PR appears safe to merge; all three previous findings are addressed.Summary
Direct conversions now default to half the usable CPUs, bounded between one and four.
All three previous findings are addressed. No new actionable issues were found.
Reviews (2) · Last reviewed commit: "docs(self-host): correct media-server tu..." · Reviewed by Greptile