#2074 closed defect (fixed)
Activity data directory should be deleted when an activity is deleted
Reported by: | bernie | Owned by: | tch |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | Sugar | Version: | Unspecified |
Severity: | Unspecified | Keywords: | sugar-love r+ dextrose |
Cc: | garycmartin, wade | Distribution/OS: | Unspecified |
Bug Status: | Assigned |
Description
Some ill-behaved activities generate large amounts of data in their data directories. For example, Browse can easily put 50MB of cache in there, and I've seen a laptop with Wine using up 200MB of disk space. As the user installs activities to try them out, this junk accumulates permanently in the filesystem, with no way to get rid of it. I've seen several laptops with an empty journal and no free space.
I propose that deleting an activity should also automatically delete its associated data directory.
There's a data loss concern for activities that don't use the journal, but the current behavior causes an even worse data loss scenario: it quickly makes the system so unusable that it needs to be reinstalled, thus loosing *all* data.
Attachments (11)
Change History (33)
comment:1 Changed 13 years ago by bernie
- Owner changed from jasg to tch
- Status changed from new to assigned
Changed 13 years ago by tch
comment:2 Changed 13 years ago by tch
- Keywords r? added
comment:3 Changed 13 years ago by erikos
- Component changed from sugar to sugar-toolkit
- Milestone changed from Unspecified by Release Team to 0.90
comment:4 follow-up: ↓ 8 Changed 13 years ago by erikos
- Cc garycmartin wade added
About the concern for activities that don't use the journal losing data. An activity could store configurations in the bundle profile even though it does write to the Journal. Maybe the activity team can give his opinion on how the activities behave to evaluate what the bigger issue is.
For the code, I would call it bundle_profile_path.
bundle_profile_path = env.get_profile_path(self._bundle_id)
comment:5 Changed 13 years ago by erikos
- Bug Status changed from Unconfirmed to Assigned
- Keywords r! added; r? removed
- Priority changed from Unspecified by Maintainer to Normal
Changed 13 years ago by tch
Changed 13 years ago by tch
comment:6 Changed 13 years ago by tch
Added 2 new patches, for sugar and sugar-toolkit. To differ the delete and upgrade scenarios when uninstalling an ActivityBundle.
comment:7 Changed 13 years ago by tch
- Keywords r? added; r! removed
comment:8 in reply to: ↑ 4 Changed 13 years ago by garycmartin
Replying to erikos:
About the concern for activities that don't use the journal losing data. An activity could store configurations in the bundle profile even though it does write to the Journal. Maybe the activity team can give his opinion on how the activities behave to evaluate what the bigger issue is.
As a concrete if trivial example, the Moon activity stores some minimal UI state in its data directory in addition to the Journal entry. The main use case is for folks using Moon in the Southern hemisphere and their preference for viewing the moon rotated so that the its south pole is facing up. Storing this in the data directory allows future new sessions to start with the correct view orientation.
If an activity is specifically erased by a user then it is understandable that this data may be removed (for the sake of maximising available storage space), though it would be frustrating if this was triggered unexpectedly by other less obvious Sugar actions (upgrading; deleting an .xo bundle from Journal; Backup/restore). Not sure how much sensitive data is kept around by other activities, other than TurtleArt using it for caching brick images (has to re render them all at launch time if deleted).
comment:9 Changed 13 years ago by tch
Why is an activity being uninstalled when deleting the bundle from the journal? Is that the desired behavior? or is just a side effect of a simpler logic?
Now i see there are many other scenarios. But i think most of them are handleable, because only 1 of them requires a profile data deletion.
I think this discussion should be moved to the mailing list, but even when the solution would require further discussion and a redesign process, Ill try to fix as it currently is, because we consider that having those invisible storage eaters is a bigger problem than loosing cache for browse or Turtle art.
Changed 13 years ago by tch
Changed 13 years ago by tch
comment:10 Changed 13 years ago by tch
Now it will delete the profile data only when the user explicitly erase the activity from the activities list.
The patch is even more simple now, because it behaves normally for any other scenario.
comment:11 follow-up: ↓ 12 Changed 13 years ago by erikos
Thanks martin for the new patch. Just one discussion to consider. There was the general issue with kids removing the activities and then technicians had to bring them back. Is that solved in your deployment? http://lists.sugarlabs.org/archive/sugar-devel/2009-May/014156.html
comment:12 in reply to: ↑ 11 Changed 13 years ago by tch
Replying to erikos:
Thanks martin for the new patch. Just one discussion to consider. There was the general issue with kids removing the activities and then technicians had to bring them back. Is that solved in your deployment? http://lists.sugarlabs.org/archive/sugar-devel/2009-May/014156.html
Yes it is, now i have improved the patch so it can be applied to master branch.
http://bugs.sugarlabs.org/ticket/2087
comment:13 Changed 13 years ago by tch
- Keywords dextrose added
Please also review this one:
Protected activities patch; http://bugs.sugarlabs.org/ticket/2087
Deleting .xo entries from journal corner case: http://bugs.sugarlabs.org/ticket/1512
comment:14 follow-up: ↓ 15 Changed 13 years ago by FGrose
See this posting, http://lists.sugarlabs.org/archive/sugar-devel/2010-July/025769.html
To prevent the any unexpected loss of Activity profile data or other content that may be stored in the Activity directory, an option checkbox should be added to the confirmation dialog, such as, (check) erase all associated data, which could default to yes (checked).
Changed 13 years ago by tch
Changed 13 years ago by tch
comment:15 in reply to: ↑ 14 Changed 13 years ago by tch
Replying to FGrose:
See this posting, http://lists.sugarlabs.org/archive/sugar-devel/2010-July/025769.html
To prevent the any unexpected loss of Activity profile data or other content that may be stored in the Activity directory, an option checkbox should be added to the confirmation dialog, such as, (check) erase all associated data, which could default to yes (checked).
Done :)
Changed 13 years ago by tch
Changed 13 years ago by tch
comment:16 Changed 13 years ago by tch
http://lists.sugarlabs.org/archive/sugar-devel/2010-August/026219.html
Including the checkbutton generated a new discussion about this topic, and the consensus was to not include the checkbutton. Code review (on the fifth version) will be appreciated.
comment:17 Changed 13 years ago by erikos
- Keywords r+ added; r? removed
Code looks good. Thanks for reaching a consensus on the mailing list, this was probably the harder part of the fix :)
Please cite the ticket in the commit line like #2074. Thanks!
comment:18 Changed 13 years ago by erikos
- Component changed from sugar-toolkit to sugar-datastore
Please land this today, so we have it in tonight's tarballs!
Changed 13 years ago by tch
Changed 13 years ago by tch
comment:19 Changed 13 years ago by tch
pong is that ok?
comment:20 Changed 13 years ago by tch
- Resolution set to fixed
- Status changed from assigned to closed
comment:21 Changed 13 years ago by sascha_silbe
- Component changed from sugar-datastore to sugar
The attached patch is against sugar, so I assume the component change was accidental.
Please provide a link to the commit(s).
Attached patch for deleting profile path