Ticket #7588 (closed defect: fixed)

Opened 6 years ago

Last modified 19 months ago

Sugar should provide messages indicating when NAND is "getting full" and "critically full"

Reported by: cscott Owned by: kimquirk
Priority: blocker Milestone: 8.2.0 (was Update.2)
Component: sugar Version: not specified
Keywords: blocks:8.2.0 r+ Cc: Eben, godiard, humitos
Action Needed: test in release Verified: no
Deployments affected: Blocked By:
Blocking: #7125

Description

When the NAND is full, Sugar should suggest that the child clean up the journal.

Attachments

full_journal_alert.jpg (88.2 kB) - added by Eben 6 years ago.
7588-critical_space.patch (6.1 kB) - added by erikos 6 years ago.
first go for the critical space patch
7588-critical_space-2.patch (6.4 kB) - added by erikos 6 years ago.
new version with better layout

Change History

  Changed 6 years ago by cscott

  • blocking 7125 added

(In #7125) Here's a list of tasks associated with this general bug, and trac #s for them:

  • the initscripts should be sure to unfreeze the dcon if/when X fails to start. This ensures that the system is obviously recoverable (you can recover by rebooting with the check key held down, but this is not obvious!). (#7586)
  • sugar should, ideally, start even if flash is full. It is currently failing when writing to ~olpc/.boot_time or some such, and crashing. (#7587)
  • once sugar starts, there should be a message indicating that the NAND is critically full. (#7588)
  • trying to save new content to the journal should also give an obvious message that the NAND is full. (#7589)
  • removing content from the journal should work even if NAND is full. (#7590)
  • automatically remove content from the journal is NAND is full? (controversial) (#7591)
  • Jffs2 is slow when it fills/root should have reserved space (#5317)

  Changed 6 years ago by gregorio

  • keywords blocks:8.2.0 added
  • priority changed from normal to blocker
  • next_action changed from never set to design

  Changed 6 years ago by marco

  • cc Eben added

This will break the string freeze once again. Eben, can you please spec the UI?

Changed 6 years ago by Eben

  Changed 6 years ago by Eben

I've attached a sample of what this alert should look like. Note that this is the first real example of the "modal alert" specification, which should ultimately be separate from "modal panels" (such as the object chooser, control panel, etc.) The API should look very similar to that for non-modal alerts, accepting an icon, a title, a message, and an array of buttons.

The icon should be rendered in the large size; all text is 2x normal size and centered; the title is bold while the message is not; the message box should fill 75% of the window width; everything is centered both horizontally and vertically.

I think that this portion of this ticket can be implemented sooner than later, and we can save the less obtrusive notifications until that system is more robust. In the meantime, we may want to choose a threshold for revealing this alert that is slightly more conservative in the amount of free space remaining, so that it gets handled before an emergency arises in most cases.

  Changed 6 years ago by erikos

  • owner changed from marco to erikos

  Changed 6 years ago by marco

Status meeting agreement is that we should do this and the related #7589 even if we can't do it in a way that cover all cases. Think of the "reserving" 50 MB space idea, for example.

Some combination of Tomeu, Marco, Eben, Simon should discuss a simple design for this in 8.2, and we can refine it later.

  Changed 6 years ago by erikos

Conditions: every time the journal get focus or an item is added or boot (or the journal is started) we check free space and display the dialog if it's low (50MB).

It is ok if this is a bit annoying in the case where the user does not want to clean up after all.

For the design see the mockup which is attached here.

  Changed 6 years ago by marco

  • next_action changed from design to code

Changed 6 years ago by erikos

first go for the critical space patch

follow-up: ↓ 10   Changed 6 years ago by erikos

  • keywords r? added
  • next_action changed from code to review

The cases this patch handles are:

a) sugar boots (we check the space at journal start)

b) create/update datastore object

c) when the journal is in focus

With the latter case the current implementation faces a race. For example - in case (a) after the alert is displayed and 'Show journal' is clicked we focus the journal and another alert is displayed. Maybe we can even leave the show-when-focus case out.

Layout nitpicks:

  • i added the SugarModalAlert gtype to the sugar theme which gives the black background color and the white font but does not produce the grey border, any idea someone?
  • the button in the alert takes all the available size even when I try to tell him not to self._vbox.pack_start(self._show_journal, False)

For the moment I use 50MB as a threshold (spelled wrong in the current patch :).

in reply to: ↑ 9   Changed 6 years ago by tomeu

Replying to erikos:

c) when the journal is in focus With the latter case the current implementation faces a race. For example - in case (a) after the alert is displayed and 'Show journal' is clicked we focus the journal and another alert is displayed. Maybe we can even leave the show-when-focus case out.

I think Eben is fine with dropping that.

Layout nitpicks: - i added the SugarModalAlert gtype to the sugar theme which gives the black background color and the white font but does not produce the grey border, any idea someone?

Nope :/ Maybe you should add the theme patch?

- the button in the alert takes all the available size even when I try to tell him not to self._vbox.pack_start(self._show_journal, False)

Yeah, that's expected, from http://www.pygtk.org/docs/pygtk/class-gtkbox.html :

"A child is always allocated the full height of a gtk.HBox and the full width of a gtk.VBox."

This works:

        alignment = gtk.Alignment(xalign=0.5, yalign=0.5)
        self._vbox.pack_start(alignment, expand=False)
        alignment.show()

        self._show_journal = gtk.Button()
        self._show_journal.set_label(_('Show Journal'))
        self._show_journal.set_border_width(0)
        alignment.add(self._show_journal)
        self._show_journal.show()
        self._show_journal.connect('clicked', self.__show_journal_cb)

Otherwise looks good to me.

  Changed 6 years ago by tomeu

Also, these two lines brings it closer to the mockup:

        self._message.props.justify = gtk.JUSTIFY_CENTER
        self._message.set_line_wrap(True)

I'd say to push it ASAP and open tickets for the visual bugs.

  Changed 6 years ago by erikos

The button alignment works, thanks.

For the background I added SugarModalAlert to the gtkrc.em like we do for SugarAlert for example. I will ping benzea about it - otherwise I can use an Eventbox and modify_bg calls to get the desired layout - not very nice though.

widget_class "*<SugarAlert>"               style "black-bg"
widget_class "*<SugarAlert>*"              style "black-bg-child"

widget_class "*<SugarSectionView>"       style "white-bg"
widget_class "*<SugarSectionView>*"      style "white-bg-child"

widget_class "*<SugarModalAlert>"               style "black-bg"
widget_class "*<SugarModalAlert>*"              style "black-bg-child"

The wrapping of the message only nearly works - it is a bit shifted. Already in another occasion I found out that the wrapping for labels is not that glory. Especially when you use wrap and justify together.

Changed 6 years ago by erikos

new version with better layout

  Changed 6 years ago by tomeu

  • keywords r+ added; r? removed

Bonus points if you can get by without the close-alert signal.

  Changed 6 years ago by erikos

  • next_action changed from review to package

Took the extra bonus and committed to journal master and sucrose-0.82. Thanks for the detailed review!

  Changed 6 years ago by marco

  • next_action changed from package to test in build

  Changed 6 years ago by erikos

  • next_action changed from test in build to add to build

  Changed 6 years ago by cscott

Again, can I please get a list of specific packages/versions, so I can write a proper changelog?

Also, it's not clear whether this is "add to (joyride) build" or "add to (stable 8.2) build"; if the latter, then packages should go through the "approved for release" state and they should then be tagged on the dist-olpc3-testing branch. Thanks.

  Changed 6 years ago by cscott

  • next_action changed from add to build to add to release

  Changed 6 years ago by marco

sugar-journal-98-1.fc9

Approved by me. (We will be following the above workflow for new "add to release" requests).

  Changed 6 years ago by cscott

  • next_action changed from add to release to test in release

Committed to stable repo; should be in 758 and later; please test.

  Changed 6 years ago by kimquirk

  • next_action changed from test in release to diagnose

Build 8.2-760: I filled the NAND by downloading tons of activities and content (from our wiki site). There were no warnings or suggestions that the journal was full. I did the NAND fill-up all in one setting, without reboots between downloads. When I do the 'df' command, I get 99% memory full.

Not sure exactly when I should get these messages, but perhaps I went by the critical percentage so quickly that it didn't trigger any messages...

Thoughts on a different test case that would trigger the warning??

  Changed 6 years ago by erikos

Hmm I tested here on 706. You get the message when you are less than 50 MB. I had 80MB less here after copying a big file to the xo and started a big download then. The alert came up after reaching the threshold. You can not miss it because it is a modal one. Note sure yet what went wrong in your case :/

  Changed 6 years ago by erikos

sorry 760 i meant of course.

  Changed 6 years ago by erikos

|TestCase|

Fill up the nand by, for example, copying a big file over to the XO. When less than 50 MB is free on the NAND (verify with the palette from the journal icon in the frame) you will see a modal alert that states that your journal is full. When you hit the 'Show journal' button the journal will be resumed. The alert is triggered when you have less then 50MB of space and you try to write to the DS (open an activity, make changes to that entry...) and on boot.

  Changed 6 years ago by marco

        try:
            self._check_for_bundle(jobject)
        finally:
            jobject.destroy()
        self._check_available_space()

If the last bundle installed is a big one (> 50 mb when uncompressed on nand), I guess _check_for_bundle will emit an exception and _check_available_space() will not be run.

At that point, trying to close an activity should complain (but I don't think it will give you specific info about space being low). Doing a reboot should bring up the no space available dialog.

Just a theory, I'd need logs to confirm it.

  Changed 6 years ago by erikos

Installing a big bundle worked here as well. The alert showed up fine.

  Changed 6 years ago by marco

Was the bundle small enough to not go under 50 mb when downloaded, but big enough to fill all disk when uncompressed? If so, I'd like to see the logs.

  Changed 6 years ago by marco

  • next_action changed from diagnose to test in release

Can someone please try out Simon testcase? The possible issue I detected in the code would not be a release blocker imo (pretty hard to hit and a reboot fixes it).

  Changed 6 years ago by marco

Also make sure to provide logs if you run into issues with it.

  Changed 6 years ago by cscott

Well, jffs2's reporting of freespace isn't accurate: it never really knows how much space is reclaimable if it 'tried really hard' at gc. But still, if jffs2 is reporting 99% full, it should be triggering the message. A mystery...

  Changed 6 years ago by mchua

  • owner changed from erikos to kimquirk

assigning NAND-full bug to kimquirk for testing as per kim's request.

  Changed 6 years ago by mchua

  • status changed from new to closed
  • resolution set to fixed

Kimquirk says NAND tests pass.

  Changed 19 months ago by humitos

  • cc godiard, humitos added
Note: See TracTickets for help on using tickets.