DataStore loses metadata between reboots

[uwog]

The datastore seems to loose the 'source' metadata property for ds objects (and possibly other metadata).

See the example code, which adds an HTML file to the DS with having the 'source' metadata property set to true.

With the current jhbuild, when you resume such an entry from the journal, Write will correctly open it showing the contents of the file (ie. not with any markup, just raw data).

This will keep working, unless the DS is restarted. Then the 'source' metadata property of the journal object seems to be vanished.

This makes http://dev.laptop.org/search?q=4022 rather useless.

[uwog]

Sample python test program

[marco]

source is not in the model, so the problem might be limited to those properties. It should probably be added to the model but we want to solve the problem with non-model properties too, if that's the case.

[uwog]

Patch that fixes the issue (from marcopg)

[uwog]

Note that that patch will not work for 'non-model' properties

[jg]

OK for Update.1

[tomeu]

I don't think it's wise to restore support for non-model properties for Update1. I'd add the property 'source' to the model and log an error when a property not in the model is submitted.

[marco]

I'm really really unhappy that we didn't get this one right. But if you say it's not safe to change it at this point we will have to deal with it. So +1 on the plan from me.

[tomeu]

Unless Ben can provide a solution that is not too invasive for Update1.

[bert]

Does this mean any custom meta-data properties are not stored? That's ... very unfortunate. What is the definitive list of preserved properties then?

[marco]

Yeah. See line 361 there:

https://dev.laptop.org/git?p=projects/datastore;a=blob;f=src/olpc/datastore/model.py;h=4558a107fd6f37ca73f21e9bca3ff24da47ee16d;hb=HEAD

[tomeu]

another test script that queries entries with the source property

[tomeu]

With the atatched script, I see the 'source' property persisted across reboots. Suspecting of a bug in the activity code, I'm reassigning it to Simon.

[marco]

Tomeu, what is going here?

I thought you said custom metadata properties are not stored. Does your test script contradict that? Does the script restart the DS?

[tomeu]

Replying to marco:

Tomeu, what is going here?

Don't really know. I suspect of some error in the activity code.

I thought you said custom metadata properties are not stored.

This code made me think that: http://dev.laptop.org/git?p=projects/datastore;a=blob;f=src/olpc/datastore/xapianindex.py;h=06041aa5770554f9739c0ccaa726aae92d0ad65e;hb=HEAD#l369

But after reading more, I saw that this other code will add the properties missing in the model to the secore config:

http://dev.laptop.org/git?p=projects/datastore;a=blob;f=src/olpc/datastore/xapianindex.py;h=06041aa5770554f9739c0ccaa726aae92d0ad65e;hb=HEAD#l310

Does your test script contradict that?

Yeah, you can run it and one entry with the source property will be created at every run. If you close sugar, restart and run it again, the script will show you that the entries created earlier still have the source property.

Does the script restart the DS?

No, you need to manually restart the DS when you want to check if the files still have the source property.

[tomeu]

This patch should fix the issue of non-model properties getting lost:

diff --git a/src/olpc/datastore/xapianindex.py b/src/olpc/datastore/xapianindex.py
index 06041aa..dfef53c 100644
--- a/src/olpc/datastore/xapianindex.py
+++ b/src/olpc/datastore/xapianindex.py
@@ -104,6 +104,12 @@ class IndexManager(object):
         datamodel = kwargs.get('model', model.defaultModel)
         datamodel.apply(self)
 
+        # configure the model according to the database
+        for field_name in self.write_index._field_actions:
+            if field_name not in datamodel.fields:
+                datamodel.addField(field_name, 'string')
+            self.fields.add(field_name)
+
         # store a reference
         self.datamodel = datamodel
         
@@ -308,6 +314,7 @@ class IndexManager(object):
         d = {}
         add_anything = False
         for k,v in props.iteritems():
+            k = str(k)
             p, added = self.datamodel.fromstring(k, v,
                                                  allowAddition=True)
             if added is True:

[jg]

OK for update.1.

[tomeu]

Pushed. Marco, can you do an rpm? Thanks!

[marco]

Built as sugar-datastore-0.3. Reassigning to cscott to tag it.

[cscott]

Tagged for update1. Please verify that this is in update1-638 (or later) and close the bug.

[cscott]

Datastore is crashing in update1 after 637. Reverting to previous sugar-datastore-0.2 for update1-641. kim will attach crash logs.

[marco]

#5072 has useful logs

[yani]

Replying to cscott:

Datastore is crashing in update1 after 637. Reverting to previous sugar-datastore-0.2 for update1-641. kim will attach crash logs.

It seems to be the reason that many activities cant load in 640 : http://dev.laptop.org/ticket/5072

crash logs are available there

[tomeu]

Patch attached to #5072.

[marco]

Duplicate of #5072

[tomeu]

I think we should keep this one open until we have a fix in the builds.

#5072 is about activities crashing because of an incompatible upgrade between development versions.

Marking #5120 as a duplicate of this one.

Marking #5253 as depending on this one.

If we fix #5254 for Update.1, we could postpone this one to Update.2.

[jg]

tomeu, what is your recommendation on this?

[tomeu]

#5254 is already in joyride and I see no reason why it would not get into update.1.

So, this one should be deferred until update.2.

[jg]

Tomeu, I'm confused as to what is to be deferred...

[tomeu]

Replying to jg:

Tomeu, I'm confused as to what is to be deferred...

I think we should fix custom properties support for update.2 because it would be a quite invasive change that could render the laptops unusable. So by defer I meant to retarget for update.2.

[marco]

What's the status of this one?

[tomeu]

If we changed the datastore to read the json files from the fs instead of extracting the metadata from the xapian index, this could be solved. Performance and scalability may improve, as well.

[tomeu]

Pascal has asked me about this issue, so here comes a brain dump:

The problem is that the DS stores info about properties in three places: in an in-memory dictionary, in model.py and in a xapian config file.

When we push a yet unknown property to the DS:

• this property gets added to the in-memory dictionary,
• xapian (or rather the xapian bindings) tracks it and updates its config file.

When we later retrieve an entry with that property, xapian will map it to the name we know and the DS will check in its in-memory dict if the property is known and which data type it has, etc.

Now the problem happens when the DS stops and starts again, because the in-memory dictionary gets recreated from model.py, so it doesn't know anything about properties not there. So that property gets dropped with the results we know.

One solution would be to store the custom properties in one more config file, another would be to fallback to some defaults when xapian hands us an unknown property.

[tomeu]

Any solutions to this look quite high risk to me. If anyone wants to provide a patch, we can think again if we should include it in 8.2.0.

[gregorio]

Hi Guys,

I have heard from several deployments recently that students and teachers lose data (aka their work is not in the journal when they go to look for it later). Could this bug the cause of that?

That's a major concern and I want to address it as much as possible in this release.

I know its late in the game to add "high risk" code but I want to make progress on this soon so let me know if we have a design proposal and a scoping on what it would take to get this resolved.

Thanks,

Greg S

[marco]

No, this only apply to custom metadata which I don't we are using in activities yet.

[morgs]

Replying to marco:

No, this only apply to custom metadata which I don't we are using in activities yet.

Read uses this to store the current page number.

I think Gregorio's scenario is #6014, solved by the session manager in 8.2.0. Without that, shutting down when you have activities open doesn't trigger a save.

[marco]

Yeah that's probably the most likely cause. I wouldn't exclude other kind of dataloss though. We know that the datastore is every fragile.

[gnu]

Does Tomeu's new datastore implementation fix this bug? I.e. can an activity store whatever property names (and corresponding values) that it wants, and will the datastore retain them forever?

[tomeu]

Replying to gnu:

Does Tomeu's new datastore implementation fix this bug? I.e. can an activity store whatever property names (and corresponding values) that it wants, and will the datastore retain them forever?

Yup!