Skip to content
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

Performance: Redundant chowning is being done in start.sh and ddev app.Start() #4942

Closed
rfay opened this issue May 22, 2023 · 2 comments
Closed
Milestone

Comments

@rfay
Copy link
Member

rfay commented May 22, 2023

Expected Behavior

We don't want to do redundant time-consuming chowns, especially when they are chowning large directories, or when they require an extra Exec (container exec).

Actual Behavior

We're chowning multiple things with multiple operations.

  • In ddev-webcontainer's start.sh, we sudo chown -R "$(id -u):$(id -g)" /mnt/ddev-global-cache/ /var/lib/php - this may be necessary in non-ddev context (where the image is being used outside ddev) but in general the ddev-global-cache is being done pre-emptively by app.Start(). (I did remove one instance of this so at least it's not being done twice now)
  • In app.Start() we have several duplications, and when we duplicate things it means more container starts.
    • Every call to CopyIntoVolume() does its own chown, meaning an additional Exec() call.
      c := fmt.Sprintf("chown -R %s %s", uid, targetSubdirFullPath)
    • We chown /var/lib/mysql and /var/lib/postgresql separately, but they could be combined into one exec, one RunSimpleContainer(). The mounts would have to be constructed in advance and the bash command also.
    • The project_mutagen volume is also separately chowned, and this may be time-consuming if it's large

Note that we may not even always have to chown, if it's already been done. For example, the project_mutagen volume really only needs to be done when the volume is created. But we could have a check in some of these places where we see if the ownership is already correct.

Steps To Reproduce

DDEV_DEBUG=true ddev start - look for the chowns.
ddev logs - look for the chowns

@rfay rfay changed the title Redundant chowning is being done in start.sh and ddev app.Start() Performance: Redundant chowning is being done in start.sh and ddev app.Start() May 22, 2023
@rfay
Copy link
Member Author

rfay commented May 23, 2023

In #4944 I attempted to fix the double-chown in start.sh, but it caused unknown problems with a few tests, so I removed that part and just fixed the too-late-mutagen.yml generation problem. So the redundant chowning is all still there for us.

@rfay rfay added this to the v1.23 milestone Jul 17, 2023
@rfay rfay modified the milestones: v1.23, v1.22.5 Oct 27, 2023
@rfay rfay modified the milestones: v1.22.5, v1.22.6 Nov 18, 2023
@stasadev stasadev modified the milestones: v1.22.6, v1.22.7 Dec 13, 2023
@rfay
Copy link
Member Author

rfay commented Feb 7, 2024

This is relatively hard to solve and relatively low impact. One part is solved in #5686

I think I'm willing to ignore this, closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants