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

disk usage percent fix #211

Merged
merged 2 commits into from
Sep 14, 2020
Merged

Conversation

will-moore
Copy link
Member

See https://forum.image.sc/t/omero-web-reports-disk-usage-incorrectly/42219

This rounds the usagePercent value before casting it to Int, to avoid strange JavaScript parseInt behaviour of ignoring exponentials for very small numbers (see example in link above).

To test: - probably not so easy to create multi-petabyte free disk space.
So might have to just check the code change against the examples in the link above.

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/omero-web-reports-disk-usage-incorrectly/42219/2

@will-moore will-moore changed the title round usagePercent before parseInt disk usage percent fix Sep 1, 2020
@pwalczysko
Copy link
Member

pwalczysko commented Sep 2, 2020

Screen Shot 2020-09-02 at 15 25 52

On merge-ci, this seems to be okay (?) Although how many TB do we have for our peruse there ?
https://merge-ci.openmicroscopy.org/web/webadmin/stats/
Even 1 TB seems excessive to the best of my knowledge, so not sure here.
The ratio counted seems correct, it is 3%

Edit: thinking about it a bit more, unless I count in some docker magic, it is sheerly not porrible that there are more than one TB of data on merge-ci ? cc @joshmoore

@sbesson explained that the number is plausible, see screenshot below (only the usage of the idr user) - the usage number is pushed up as this server is used for idr datasets testing

Screen Shot 2020-09-02 at 15 33 46

@joshmoore
Copy link
Member

joshmoore commented Sep 2, 2020

bash-4.2$ pwd
/uod/idr-scratch/merge-ci/omero-server-data
bash-4.2$ du -sh *
4.8G	BioFormatsCache
1.0K	DropBox
2.3G	Files
3.5G	FullText
66G	ManagedRepository
51G	Pixels
53M	Thumbnails

@pwalczysko
Copy link
Member

Thanks @joshmoore - the numbers in #211 (comment) seem to match more what I would expect from the top of my head.

Is the discrepancy between the web report (supported by @sbesson ) and the numbers in #211 (comment) caused by in-place imports ? And if yes, then actually, what is it that web reports there ? What is the "disk usage" of in-place imported data ? Possibly web does not take the in-place quality of the files into account ?

@joshmoore
Copy link
Member

joshmoore commented Sep 3, 2020

Confirmed separately. Free size comes from df. Used size comes from a query in the database (which ignores symlinks).

@will-moore
Copy link
Member Author

The web UI reports diskFree / (diskFree + usedSize) as a percentage.
If anyone wants to propose an update to the UI that tries to take into account the in-place imported data, please go ahead and open a new issue.
But I think the bug-fix here to the current functionality is working and can be merged?

@joshmoore
Copy link
Member

Certainly no objections for this PR: taking in-place into account is going to require server-side changes. 👍

@sbesson
Copy link
Member

sbesson commented Sep 4, 2020

As a minor doc RFE, is there some place in the client UI where the caveat about the in-place imports could be mentioned?

@pwalczysko
Copy link
Member

pwalczysko commented Sep 4, 2020

Agree with @sbesson about the explanation/warning in the UI.

Screen Shot 2020-09-04 at 11 38 23

Could an additional line under "Drive space usage" saying The in-place imported data are not counted correctly. The disk usage might be smaller than indicated.

do the job ?

@will-moore will-moore added this to the 5.8.0 milestone Sep 7, 2020
@will-moore
Copy link
Member Author

Added warning message:

Screenshot 2020-09-11 at 12 21 52

@manics manics merged commit 26d08d0 into ome:master Sep 14, 2020
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.

6 participants