Shutdown should sync activities data

[michael.tiemann]

My daughter was using her XO on a long trip and typing pages of a story into the text editor activity. After a while she decided to rest, and she shutdown the computer as we did not have car adaptor to keep the XO plugged in. When she rebooted some time later, she observed that her changes from the last session were all lost.

I understand that if she had closed the activities first, before issuing a shutdown command, the activities would have prompted her to save her work. The system-wide shutdown should cause the various activities to run their shutdown processes, which would then prompt for saves and avoid lost work. I understand that sending a dbus message would accomplish this.

[morgs]

Though D-Bus is mentioned, this isn't a bug in dbus-python but rather a system issue, probably involving Sugar and ohm.

[jg]

Ohm and sugar should cooperate in shutting down cleanly on low power....

[erikos]

The attached patch adds the dbus method 'SyncData' to the activity service so that we can sync the data to the datastore when we reboot or shutdown. This method is called for all the running activities when you use the palette in the home view to reboot or shutdown.

To verify:

• open an activity like calculate and type some equations
• reboot or shutdown (without changig the active activity since this will sync the data as well)
• resume the activity and check that the data is there

Furthermore the 'SyncActivityData' dbus method is added to the shell service so that ohm can call it to sync the data of all the running activities when going to suspend.

[erikos]

Sync the data when reboot/shutdown and suspend

[cjb]

Looks good to me. Reassign to me once the patch is reviewed and in sugar, and I'll add to OHM.

[erikos]

Shouldn't we get this into update.1, Jim?

[erikos]

Loosing data is bad. Maybe we should at least get the fix in for the shutdown/reboot case. This is sugar only.

[marco]

I think we probably want to reuse some standard way to do this, rather then providing our own interface.

[jg]

I agree with Marco.

There are existing desktop conventions for notifying applications their session is shutting down. We should not invent our own...

[cscott]

See: http://www.news.com/8301-10784_3-9859801-7.html?part=rss&subj=news&tag=2547-1_3-0-20

I think Update.2 is a reasonable target; there seems to be some disagreement on how best to do this. olpc-update will need to be patched as well; see http://dev.laptop.org/git?p=users/cscott/olpc-update;a=blob;f=bitfrost/update/setup.py;h=772f97e82c1eef1f3b2c593a174e5874497aa7c9;hb=HEAD#l258

Trac #4877 and #4878 are related.

[cjb]

Hi Marco,

It sounds like this is implemented, but we aren't yet triggering it before shutting down due to low power. Could you tell me how I can make the state-saving call over dbus from OHM, so that I can add it to a trigger for when we're at critical battery life?

Thanks!

[gregorio]

Is this fixed?

Can you update the action needed field and whatever else makes sense to make sure we track this for 8.2.0?

I upped the priority to blocker but I could be convinced to lower it if this is hard.

Thanks,

Greg S

[marco]

It's on my radar. I'll try to look at it over the weekend.

[marco]

Code needs a bit of love. Save is async and we need to ensure it's finished before we allow session shutdown. Should be easy.

We also need a rainbow fix for this work, Tomeu is opening a separate ticket which will block this one.

[marco]

Patch that should work but doesn't for weird reasons

[marco]

Working patch

[marco]

This needs careful review and testing.

[marco]

|Testcase|

Start Write. Type some text. Go back to the home view, hover the XO and click "Reboot". When Sugar is back, go to the journal and open the last Write document. Make sure the text you typed is there.

[erikos]

Test result: Applied the sugar patch and the rainbow one and the data was saved and accessible after reboot. Tested with write and browse and tested that there were no errors introduced to save in general with the patch.

[erikos]

Code looks good.

[erikos]

We must make sure that #7574 is in when we want this to work.

[cscott]

Just so I'm sure I understand this correctly: this ensures that data is sync'ed when you shutdown from the home view, but doesn't make 'shutdown -h now' from the terminal sync data. Could this be fixed by just adding a simple python script to the initscripts to invoke 'syncData' on shutdown? This might cause us to sync data twice if you shutdown from the home view -- is that a problem?

For context, I'm trying to fix #4878 now that the blocker is gone. Should I be doing this by making 'shutdown' safe, or should olpc-update explicitly call sugar's syncData before it invokes /sbin/shutdown?

[marco]

Replying to cscott:

Just so I'm sure I understand this correctly: this ensures that data is sync'ed when you shutdown from the home view

Correct. This does basically the same gnome-session does in GNOME (we reuse a lot of that code too).

but doesn't make 'shutdown -h now' from the terminal sync data.

Correct.

Could this be fixed by just adding a simple python script to the initscripts to invoke 'syncData' on shutdown?

Not sure. A tricky aspect is that activities can block shutdown, so the script would have to deal with it.

This might cause us to sync data twice if you shutdown from the home view -- is that a problem?

I'd rather not save twice. It can be expensive for big files (and we don't have a dirty flag in activities at the moment).

For context, I'm trying to fix #4878 now that the blocker is gone. Should I be doing this by making 'shutdown' safe, or should olpc-update explicitly call sugar's syncData before it invokes /sbin/shutdown?

sugar/src/session.py should expose a DBus service (maybe with the same interface as GNOME session http://svn.gnome.org/viewvc/gnome-session/trunk/gnome-session/org.gnome.SessionManagement.xml?revision=4777&view=markup). And I tend to think olpc-update should call the Shutdown method on it directly.

[cscott]

Replying to marco:

sugar/src/session.py should expose a DBus service (maybe with the same interface as GNOME session http://svn.gnome.org/viewvc/gnome-session/trunk/gnome-session/org.gnome.SessionManagement.xml?revision=4777&view=markup). And I tend to think olpc-update should call the Shutdown method on it directly.

"should expose" means this isn't implemented yet, right? If not, then #4877 shouldn't be closed, since that's what that bug was all about.

[cjb]

Agreed, OHM needs this exposed too before the original filer's bug will be addressed.

[gnu]

The original poster (my friend Mike Tiemann) said his daughter "shutdown the computer" -- not that it ran out of battery. See top of this page. Thus his particular bug appears to be fixed.

I agree that it's a good idea for the EC, Ohm, and Sugar to cooperate to cleanly shut down a machine that's about to lose power anyway. But that's a different bug -- #4824, I believe.

[marco]

Replying to cscott:

"should expose" means this isn't implemented yet, right?

Correct.

If not, then #4877 shouldn't be closed, since that's what that bug was all about.

Ok, sorry. It was not very clear from the description, which only talked about saving activity state (which is actually implemented). I'll reopen it.

This bug should stay closed though. It's about saving data on explicit shutdown, which is fixed.

[michael.tiemann]

I agree that this is about saving data in the case of explicit shutdown, not saving data in the face of arbitrary lossage.

BTW, thanks for all the effort on this. I showed the trail to my daughter just last night, and she was impressed. I look forward to loading Update.2 when it is ready.

[marco]

(In #4877) cjb, #6014 is about explicit shutdown, see description.

I'm delaying this to 9.1.0, unless one of the bugs it blocks is made a release blocker.