On Thu, May 22, 2008 at 1:13 AM, Martin Langhoff <[EMAIL PROTECTED]> wrote: > On Thu, May 22, 2008 at 6:14 AM, Tomeu Vizoso <[EMAIL PROTECTED]> wrote: >> the patch attached maintains a copy of the metadata of each object >> outside the xapian index. How it works: > > Fantastic. Except that... erm... arhm... you forgot the patch ;-)
Ouch. >> - at every create and update, a json file is created next to the object's >> file, >> >> - it's also deleted along the object, >> >> - at startup, if the file <datastore_path>/.metadata.exported doesn't >> exist, check how many objects need to get their metadata exported >> (0.8s for 3000 entries) > > That's pretty good. > >> - in an idle callback, process each of those objects one per iteration >> (3ms per entry with simplejson, 2ms with cjson). > > Exporting a few 100 per iteration probably is more efficient ;-) Yes, but it needs to be? We are balancing here the speed at which metadata will get exported _the first time after the update to 8.2_ with usability of Sugar during that limited period of time. Anyway, feel free to try and play with the amount of entries exported at each go, the code allows for this quite easily. >> In my tests this has worked quite well, but I have one concern: can >> something bad happen if we have 20k files in the same dir (for a >> journal with 10k entries)? > > Ok, we can split it into a subdir (which will only have 10K files then). Yes, this is the easiest option. I would like to hear from Dave if there could be any problem with 10k files in the same dir. > If there's a cost to large dirs in jfffs2 then we can use hashed dirs, > and that change will be needed for both the main datastore storage > _and_ the metadata files. Yes, that's the approach I used in my DS rewrite, but I would prefer to leave this change for a future release if it's possible. >> One side effect of this is that when (if) we agree on a new on-disk >> data structure for the DS, it will be easier to convert than if we had >> to extract all the metadata from the index. > > Yes. And as you said earlier, easy recovery if xapian goes to la-la land. Yeah, I'm wondering if metadata should be retrieved from the json file instead of from the index, that may give us a performance improvement, as well as increased robustness. Thanks, Tomeu
From 7c1a25140c5c2132444abe5f7d372e19d7fdda8b Mon Sep 17 00:00:00 2001 From: Tomeu Vizoso <[EMAIL PROTECTED]> Date: Wed, 21 May 2008 20:04:42 +0200 Subject: [PATCH] Maintain a metadata copy outside the index. --- src/olpc/datastore/backingstore.py | 61 +++++++++++++++++++++++++++++++++-- src/olpc/datastore/datastore.py | 22 +------------ 2 files changed, 59 insertions(+), 24 deletions(-) diff --git a/src/olpc/datastore/backingstore.py b/src/olpc/datastore/backingstore.py index fc3c05f..c46a0e9 100644 --- a/src/olpc/datastore/backingstore.py +++ b/src/olpc/datastore/backingstore.py @@ -28,6 +28,7 @@ import sys import dbus import xapian import gobject +import cjson from olpc.datastore.xapianindex import IndexManager from olpc.datastore import bin_copy @@ -215,7 +216,11 @@ class FileBackingStore(BackingStore): instead of a method parameter because this is less invasive for Update 1. """ self.current_user_id = None - + + # source for an idle callback that exports to the file system the + # metadata from the index + self._export_metadata_source = None + # Informational def descriptor(self): """return a dict with atleast the following keys @@ -327,7 +332,29 @@ class FileBackingStore(BackingStore): im.connect(index_name) self.indexmanager = im - + + # Check that all entries have their metadata in the file system. + if not os.path.exists(os.path.join(self.base, '.metadata.exported')): + uids_to_export = [] + uids = self.indexmanager.get_all_ids() + + t = time.time() + for uid in uids: + if not os.path.exists(os.path.join(self.base, uid + '.metadata')): + uids_to_export.append(uid) + + if uids_to_export: + self._export_metadata_source = gobject.idle_add( + self._export_metadata, uids_to_export) + else: + open(os.path.join(self.base, '.metadata.exported'), 'w').close() + + 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 + def bind_to(self, datastore): ## signal from datastore that we are being bound to it self.datastore = datastore @@ -502,6 +529,23 @@ class FileBackingStore(BackingStore): return c.hexdigest() # File Management API + def _store_metadata(self, uid, props): + t = time.time() + path = os.path.join(self.base, uid + '.metadata') + props = props.copy() + for property_name in model.defaultModel.get_external_properties(): + if property_name in props: + del props[property_name] + f = open(path, 'w') + f.write(cjson.encode(props)) + f.close() + logging.debug('exported metadata: %r s.' % (time.time() - t)) + + def _delete_metadata(self, uid, props): + path = os.path.join(self.base, uid + '.metadata') + if os.path.exists(path): + os.unlink(path) + def _create_completion(self, uid, props, completion, exc=None, path=None): if exc: completion(exc) @@ -517,6 +561,7 @@ class FileBackingStore(BackingStore): if completion is None: raise RuntimeError("Completion must be valid for async create") uid = self.indexmanager.index(props) + self._store_metadata(uid, props) props['uid'] = uid if filelike: if isinstance(filelike, basestring): @@ -531,6 +576,7 @@ class FileBackingStore(BackingStore): def create(self, props, filelike, can_move=False): if filelike: uid = self.indexmanager.index(props) + self._store_metadata(uid, props) props['uid'] = uid if isinstance(filelike, basestring): # lets treat it as a filename @@ -540,7 +586,9 @@ class FileBackingStore(BackingStore): self.indexmanager.index(props, path) return uid else: - return self.indexmanager.index(props) + uid = self.indexmanager.index(props) + self._store_metadata(uid, props) + return uid def get(self, uid, env=None, allowMissing=False, includeFile=False): content = self.indexmanager.get(uid) @@ -575,6 +623,7 @@ class FileBackingStore(BackingStore): raise RuntimeError("Completion must be valid for async update") props['uid'] = uid + self._store_metadata(uid, props) if filelike: uid = self.indexmanager.index(props, filelike) props['uid'] = uid @@ -590,6 +639,7 @@ class FileBackingStore(BackingStore): def update(self, uid, props, filelike=None, can_move=False): props['uid'] = uid + self._store_metadata(uid, props) if filelike: if isinstance(filelike, basestring): # lets treat it as a filename @@ -610,6 +660,7 @@ class FileBackingStore(BackingStore): def delete(self, uid, allowMissing=True): self._delete_external_properties(uid) + self._delete_metadata(uid, props) self.indexmanager.delete(uid) path = self._translatePath(uid) @@ -617,7 +668,7 @@ class FileBackingStore(BackingStore): os.unlink(path) else: if not allowMissing: - raise KeyError("object for uid:%s missing" % uid) + raise KeyError("object for uid:%s missing" % uid) def get_uniquevaluesfor(self, propertyname): return self.indexmanager.get_uniquevaluesfor(propertyname) @@ -651,6 +702,8 @@ class FileBackingStore(BackingStore): return self.indexmanager.get_all_ids() def stop(self): + if self._export_metadata_source is not None: + gobject.source_remove(self._export_metadata_source) self.indexmanager.stop() def complete_indexing(self): diff --git a/src/olpc/datastore/datastore.py b/src/olpc/datastore/datastore.py index 67ddca9..a15d5cf 100644 --- a/src/olpc/datastore/datastore.py +++ b/src/olpc/datastore/datastore.py @@ -128,28 +128,10 @@ class DataStore(dbus.service.Object): ### Backup support def pause(self, mountpoints=None): - """pause the datastore, during this time it will not process - requests. this allows the underlying stores to be backup up via - traditional mechanisms - """ - if mountpoints: - mps = [self.mountpoints[mp] for mp in mountpoints] - else: - mps = self.mountpoints.values() - - for mp in mps: - mp.stop() + """ Deprecated. """ def unpause(self, mountpoints=None): - """resume the operation of a set of paused mountpoints""" - if mountpoints: - mps = [self.mountpoints[mp] for mp in mountpoints] - else: - mps = self.mountpoints.values() - - for mp in mps: - mp.initialize_and_load() - + """ Deprecated. """ ### End Backups def connect_backingstore(self, uri, **kwargs): -- 1.5.2.5
_______________________________________________ Devel mailing list Devel@lists.laptop.org http://lists.laptop.org/listinfo/devel