[Code-review] [PATCH] Maintain a metadata copy outside the index.

[Tomeu Vizoso]

On Thu, Jun 19, 2008 at 8:00 AM, Michael Stone <michael at laptop.org> wrote:
> On Wed, Jun 18, 2008 at 09:12:31PM +0200, Tomeu Vizoso wrote:
>> +try:
>> +    import cjson
>> +    has_cjson = True
>> +except ImportError:
>> +    import simplejson
>> +    has_cjson = False
>> +
>
> May I assume that at least one of the packages providing these python
> libraries is listed as a dependency of the sugar-datastore RPM?

No, will have to be added to the spec, I guess.

> Also, do we need to test this code against both libraries?

Well, ideally we would choose the one that fulfills best our needs and
just use that. Using cjson will save us some milliseconds that may be
nice to save but won't change much anything. Any comments?

>> +    def _export_metadata(self, uids_to_export):
>> +        uid = uids_to_export.pop()
>> +        props = self.indexmanager.get(uid).properties
>> +        self._store_metadata(uid, props)
>> +        return len(uids_to_export) > 0
>
> I take it that we're not worried about the effect of concurrent writers
> on our stored metadata?

The one stored in xapian or in json files? Anyway, both storages are
to be written only by the DS' main thread.

> What happens if our _store_metadata() function dies with something like
> ESPACE?

The laptop won't boot anymore? ;) What do you suggest to do here?

>> +    def _encode_json(self, metadata, file_path):
>> +        if has_cjson:
>> +            f = open(file_path, 'w')
>> +            f.write(cjson.encode(metadata))
>> +            f.close()
>> +        else:
>> +            simplejson.dump(metadata, open(file_path, 'w'))
>
> Hard power failure during this write could generate a corrupted
> .metadata file. We should write the metadata to a tmpfile in the same
> directory, then rename() the tmpfile into place. That way the update
> will be (closer to) atomic.

Good idea, thanks.

>> +    def _delete_metadata(self, uid, props):
>> +        path = os.path.join(self.base, uid + '.metadata')
>> +        if os.path.exists(path):
>> +            os.unlink(path)
>
> The props argument seems to be unused.

Good catch.

> General question: what code reads the .metadata files?

Martin's backup script.

New patch attached.

Thanks,

Tomeu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Maintain-a-metadata-copy-outside-the-index.patch
Type: text/x-diff
Size: 7811 bytes
Desc: not available
Url : http://lists.laptop.org/pipermail/code-review/attachments/20080619/4da6d4a2/attachment-0001.patch