-
Notifications
You must be signed in to change notification settings - Fork 59
image improvements #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
image improvements #202
Conversation
this introduces one small caviat: if you provide your own mount instead of a volume you need to make the folders and permissions yourself as well, not really ideal. gona see about extending the startup script to make the needed folders (that would run as the executing www-data user) to make the needed folders. could maybe do some other docker housekeeping tasks as well to help with container stability |
…p-fpm and larvel deployment optimalizations, and run koel:init and koel:doctor on startup
alright i kinda ended up down the rabithole with this one so let me know if this needs splitting into multiple PRs. and i now this is some big changes but all for major performance and size improvements. the docker image is now now 70% smaller. it also builds a bit faster but this is mainly dependant on internet speed cause it needs to download far less data. Could use a hand with that final test though, i have no idea why it claims the port isn't being listened to when the curl does succeed initial page load is 20% faster (in absolute numbers its 0.2s so there is a bit of variance between testing), numbers are bit less consistent with a larger library bit same ballpark. search and view loading seems snappier but harder to measure A bit of an overview of the changes:
some thoughts for future possible improvements:
|
@phanan could you take a look at this maybe? it majorly improves performance. only issue remaining seems goss seems to be doing something weird and not seeing apache listening on the port at all. but if i run the container myself it works fine and is listening on all port 80, all ports ![]() |
Thanks for the PR. As you said it yourself, the PR covers quite different points, which makes it harder (for me, near impossible) to review. Please consider splitting them into smaller ones. |
@phanan i've split most of this up in several MRs but left out the base image changes for now. while this is by far the biggest space saving and optimalization: it builds on top of all th other bigger changes and doing these first should make it a lot more managable to review, and not make it merge conflict hell. as it stands there might be some minor conflicts between some of the MRs but they are going to be very basic ones to resovle and i can deal with those as things get (hopefully) merged |
Please ping me when’re done with the PRs.
…On Sun, Aug 31, 2025 at 18:22 AEnterprise ***@***.***> wrote:
*AEnterprise* left a comment (koel/docker#202)
<#202 (comment)>
@phanan <https://github.com/phanan> i've split most of this up in several
MRs but left out the base image changes for now. while this is by far the
biggest space saving and optimalization: it builds on top of all th other
bigger changes and doing these first should make it a lot more managable to
review, and not make it merge conflict hell.
as it stands there might be some minor conflicts between some of the MRs
but they are going to be very basic ones to resovle and i can deal with
those as things get (hopefully) merged
—
Reply to this email directly, view it on GitHub
<#202 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB5O3UTNO44CR3ADU7NE2VT3QLLIVAVCNFSM6AAAAACD7576IGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTENBQGA3DKMJQGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@phanan i'm done with the PRs for now, these would need to be merged before i can do the rest since it would be full of conflicts otherwise |
noticed when upgrading koel that it needs to redownload a bunch of images, because they are stored in the container itself. This also bloats it quite a bit. The docker compose files already had a partial solution for it so i expanded it with a proper /cache folder. This also gives koel a central place for future expansions.
I considered moving the search index into it as well, people already have that mapped or data in the volume, and while koel can easily recover and re-download images, the search index going poof really breaks the application and needs manual intervention to rebuild
while updating that i also noticed it was doing 2 apt updates so merged them together