#394 closed defect (fixed)
cancel a download if space is very tight
Reported by: | tomeu | Owned by: | humitos |
---|---|---|---|
Priority: | Urgent | Milestone: | |
Component: | Browse | Version: | Git as of bugdate |
Severity: | Critical | Keywords: | r+, olpc-test-pending |
Cc: | manuq, humitos, erikos, godiard | Distribution/OS: | Unspecified |
Bug Status: | Assigned |
Description
From (#172): For example stop before there's only 1MB free.
Attachments (8)
Change History (27)
comment:1 Changed 15 years ago by erikos
comment:2 Changed 15 years ago by erikos
- Bug Status changed from New to Assigned
- Milestone changed from 0.84 to 0.86
I think this needs proper handling with a dialog (string based).
comment:3 follow-up: ↓ 5 Changed 13 years ago by timClicks
Would it be possible to alter Browse's behaviour before making a download? This could be achieved through an HTTP HEAD request, then checking for available space.
comment:4 Changed 13 years ago by bernie
- Priority changed from Normal to Urgent
This bug occurs often in the field. Children download huge files, often in parallel, without thinking of the consequences. It's a very good way to screw up your datastore and make your system unbootable.
Thus, rising priority.
comment:5 in reply to: ↑ 3 Changed 13 years ago by bernie
- Cc bernie added
- Milestone changed from 0.86 to 0.90
Replying to timClicks:
Would it be possible to alter Browse's behaviour before making a download? This could be achieved through an HTTP HEAD request, then checking for available space.
Nope, because users download several things in parallel so space runs out in unpredictable ways. We should figure out a way to interrupt downloads before the filesystem becomes full.
comment:6 Changed 12 years ago by manuq
- Milestone changed from 0.90 to 0.98
comment:7 Changed 11 years ago by humitos
- Cc godiard humitos added
I tested it in build 18 and it is still happening.
I think this bug is important because it makes Sugar to not work anymore. I mean, there is no way to remove the file because a dialogue with the message "The Journal is full" with the button "Show Journal" is popped up all the time and it doesn't allow us to do anything.
I had to login with Ctrl + Alt + F2 and remove a video that I was downloading to make Sugar to work again.
How we should implement this? Should we check the free space on every "chunk" downloaded by Browse?
Changed 11 years ago by humitos
comment:8 Changed 11 years ago by humitos
Some discussion here:
http://lists.sugarlabs.org/archive/sugar-devel/2012-September/039516.html
comment:9 Changed 11 years ago by greenfeld
I am concerned that 50 MB might be too much space to save. A 2 GB XO-1.5 with an unedited OLPC build will only have about 170 MB free to start with.
A bootable Fedora SoaS USB with an overlay partition also might have limited space available.
Refusing to download with ~30% provided space available seems like a bit much in this case. Perhaps 10 MB or 20 MB would be better.
But then this might just be bikesheding and not relevant except to the few XO deployments that actually purchased 2 GB units.
comment:10 Changed 11 years ago by humitos
- Owner changed from erikos to humitos
- Status changed from new to accepted
comment:11 Changed 11 years ago by humitos
- Keywords patch added
Changed 11 years ago by humitos
This patch needs a patched version of sugar that implements "enough_space" method in Activity class
comment:12 Changed 11 years ago by humitos
- Cc erikos added
- Keywords r? added; patch removed
Simon, I've just uploaded the sugar-toolkit-gtk3 patches related to this one.
Changed 11 years ago by humitos
This patch puts the "enough_space" in the Download class inside the Browse Activity, so we don't have to modify sugar3.activity.Activity class in this cycle
comment:13 Changed 11 years ago by manuq
- Keywords r+ olpc-test-pending added; r? removed
- Resolution set to fixed
- Status changed from accepted to closed
Finally pushed f96848a9 . Thanks Manuel.
comment:14 follow-up: ↓ 15 Changed 11 years ago by greenfeld
- Keywords olpc-test-pending removed
- Resolution fixed deleted
- Status changed from closed to reopened
The patch seems to work in Browse-148, but has an order of operations issue in the error dialog.
The section
free_space_mb = self._free_available_space( path=self.temp_path) - SPACE_THRESHOLD \ / 1024.0 ** 2
likely should read
free_space_mb = (self._free_available_space( path=self.temp_path) - SPACE_THRESHOLD) \ / 1024.0 ** 2
comment:15 in reply to: ↑ 14 Changed 11 years ago by godiard
- Cc manuq added; bernie godiard removed
- Keywords r+ removed
Humitos, manuq, can you see at the last comment from Sam?
A comment in _free_available_space explaining what unit is used (apparently return bytes) should be good. Also enough_space method say the size parameter should be in kb,
but is not consistent with the use of _free_available_space inside it.
comment:16 Changed 11 years ago by godiard
- Cc godiard added
Changed 11 years ago by humitos
comment:17 Changed 11 years ago by humitos
- Keywords r? added
I attached a new patch to fix what Sam mentioned in the previous comment and this patch adds as well some comments about the units used in the code.
comment:18 Changed 11 years ago by manuq
- Keywords r+ olpc-test-pending added; r? removed
- Resolution set to fixed
- Status changed from reopened to closed
Good catch. Pushed humitos patch as f40101a4 .
The problem I see here, is that we would need some sort of notification that the download has been stopped - or been canceled due to space constraints. And as we are in string freeze...