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

fix: corrected diskspace calculation in case of docker usage #864

Merged
merged 8 commits into from
Apr 5, 2019

Conversation

paschaef
Copy link
Contributor

As mentioned in #844 during backup in the ncp-update process there is the possiblity that no diskspace is available for the backup. Also it was mentioned that the backup is dataless and does not require a lot of space.
However, running in a docker configuration the data-directory is a subdirectory of the basedir and hence size of datadirgets counted too eventhough includedata is disabled.
This pull request aimed to correct the calculation for the free/occupied diskspace in docker configuration. If not in docker configuration calculation is done as before.

Tanarri and others added 5 commits March 26, 2019 22:27
'nc-backup' is the name of the program
The info text says, that a snapshot will be "taken". That is wrong!
We are in restore-mode --> a snapshot will be "restored"
@nachoparker
Copy link
Member

good catch

@nachoparker
Copy link
Member

related to conversation here #858 (comment)

@nachoparker
Copy link
Member

This should not be tied to /.docker-image. If we are not using INCLUDEDATA, then we should exclude the size of the datadir from the calculation.

Also we should add some more space for the database dump, as per the conversation above.

Thanks!

@paschaef
Copy link
Contributor Author

paschaef commented Apr 1, 2019

So for clarification of the directory structre (because I am only using the dockerized version). The directory structure looks as follows for both (docker, non-docker) cases:

/var/www/nextcloud/
/var/www/nextcloud/$(datadir)

Am I right?

Then the proposal is to remove the outer most if condition with /.docker-image and to do only this part:

  if [[ "$includedata" == "yes" ]]
  then
    size=$((nsize + 100*1024))
  else #datadir is inside $basedir/nextcloud therefor substract
    size=$((nsize - dsize + 100*1024))
  fi

Any suggestions on how to estiamte the size of the database dump? Or would you like to make a (larger) hardcoded estimate as it is now?

@nachoparker
Copy link
Member

hi

Yes, the dirs are "$BASEDIR/nextcloud and "$BASEDIR"/nextcloud/data in the docker or regular case.

Your proposed solution looks good to me.

About the db dump... it's hard to tell. My dbdir is 1GiB and my dbdump is 300MiB, so I think that the easiest is to go from 100 to 500MiB extra

@paschaef
Copy link
Contributor Author

paschaef commented Apr 2, 2019

Here we are.

@nachoparker
Copy link
Member

nachoparker commented Apr 3, 2019

Looks good! Now we only need to tweak the calculations like in this commit c18273a, which was fixed after you started working on this.

@paschaef
Copy link
Contributor Author

paschaef commented Apr 3, 2019

This is done! Hope it is correct in this version now.

@nachoparker
Copy link
Member

looks perfect now :)

@paschaef
Copy link
Contributor Author

paschaef commented Apr 4, 2019

great :)

@nachoparker nachoparker changed the base branch from master to devel April 5, 2019 01:06
@nachoparker nachoparker merged commit 85f9401 into nextcloud:devel Apr 5, 2019
@nachoparker
Copy link
Member

thanks!

@hunhejj
Copy link

hunhejj commented Nov 14, 2020

I still have the above issue @nachoparker & @paschaef .

Nextcloudpi version: 1.31.0., running in docker.

Trying to upgrade nextcloud from 18.0.7.1 to 19.0.4.

Fails with the same error as mentioned in #858: free space check failed. Need 873366317 Bytes. Checked and there is way more space than that.

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

Successfully merging this pull request may close these issues.

4 participants