Opened 4 years ago

Closed 4 years ago

Last modified 12 months ago

#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.

Change History (33)

comment:1 Changed 4 years ago by bernie

  • Owner changed from jasg to tch
  • Status changed from new to assigned

comment:2 Changed 4 years ago by tch

  • Keywords r? added

Attached patch for deleting profile path

comment:3 Changed 4 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: Changed 4 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 4 years ago by erikos

  • Bug Status changed from Unconfirmed to Assigned
  • Keywords r! added; r? removed
  • Priority changed from Unspecified by Maintainer to Normal

comment:6 Changed 4 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 4 years ago by tch

  • Keywords r? added; r! removed

comment:8 in reply to: ↑ 4 Changed 4 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 4 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.

comment:10 Changed 4 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: Changed 4 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 4 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 4 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: Changed 4 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).

comment:15 in reply to: ↑ 14 Changed 4 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 :)

comment:16 Changed 4 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 4 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 4 years ago by erikos

  • Component changed from sugar-toolkit to sugar-datastore

Please land this today, so we have it in tonight's tarballs!

comment:19 Changed 4 years ago by tch

pong is that ok?

comment:20 Changed 4 years ago by tch

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:21 Changed 4 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).

comment:22 Changed 12 months ago by dnarvaez

  • Milestone 0.90 deleted

Milestone 0.90 deleted

Note: See TracTickets for help on using tickets.