[Sugar-devel] [PATCH sugar 2/4] Journal: Increase timeout for data store operations (see SL#2319)

[Sascha Silbe]

The default DBus timeout (currently 25s [2]) can be too short if the data
store is busy, especially (but not only) on systems with a slow SD card
and / or when writing large files.

Address this by increasing the timeout to 1 minute for read operations and
5 minutes for write operations. The values were chosen to roughly match the
patience I believe an average user on a system with slow storage would have.

We still don't cope with the fact that there is no reasonable timeout that
covers _all_ situations, nor does this patch fix the lack of UI feedback
and responsiveness (SL#2480 [3]). Those require extensive code changes and
potentially significant UI changes, so they are out of scope for this patch.
For similar reasons, migration of large data stores (SL#1546 [4]) isn't
addressed either.

[1] https://bugs.sugarlabs.org/ticket/2319
[2] http://dbus.freedesktop.org/doc/api/html/dbus-connection-internal_8h_source.html#l00045
[3] https://bugs.sugarlabs.org/ticket/2480
[4] https://bugs.sugarlabs.org/ticket/1546

Signed-off-by: Sascha Silbe <silbe at activitycentral.com>
---
 src/jarabe/journal/model.py |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/jarabe/journal/model.py b/src/jarabe/journal/model.py
index 5d9dd6c..b34ad84 100644
--- a/src/jarabe/journal/model.py
+++ b/src/jarabe/journal/model.py
@@ -41,6 +41,8 @@
 DS_DBUS_SERVICE = 'org.laptop.sugar.DataStore'
 DS_DBUS_INTERFACE = 'org.laptop.sugar.DataStore'
 DS_DBUS_PATH = '/org/laptop/sugar/DataStore'
+DBUS_READ_TIMEOUT = 60
+DBUS_WRITE_TIMEOUT = 300

 # Properties the journal cares about.
 PROPERTIES = ['activity', 'activity_id', 'buddies', 'bundle_id',
@@ -222,7 +224,7 @@ def __init__(self, query, page_size):

     def find(self, query):
         entries, total_count = _get_datastore().find(query, PROPERTIES,
-                                                     byte_arrays=True)
+            byte_arrays=True, timeout=DBUS_READ_TIMEOUT)

         for entry in entries:
             entry['mountpoint'] = '/'
@@ -548,7 +550,8 @@ def get(object_id):
         metadata = _get_file_metadata(object_id, stat)
         metadata['mountpoint'] = _get_mount_point(object_id)
     else:
-        metadata = _get_datastore().get_properties(object_id, byte_arrays=True)
+        metadata = _get_datastore().get_properties(object_id, byte_arrays=True,
+            timeout=DBUS_READ_TIMEOUT)
         metadata['mountpoint'] = '/'
     return metadata

@@ -561,7 +564,8 @@ def get_file(object_id):
         return object_id
     else:
         logging.debug('get_file asked for entry with id %r', object_id)
-        file_path = _get_datastore().get_filename(object_id)
+        file_path = _get_datastore().get_filename(object_id,
+                                                  timeout=DBUS_READ_TIMEOUT)
         if file_path:
             return util.TempFilePath(file_path)
         else:
@@ -580,7 +584,8 @@ def get_file_size(object_id, metadata=None):
     if 'filesize' in metadata:
         return int(metadata['filesize'])

-    file_path = _get_datastore().get_filename(object_id)
+    file_path = _get_datastore().get_filename(object_id,
+                                              timeout=DBUS_READ_TIMEOUT)
     if file_path:
         size = os.stat(file_path).st_size
         os.remove(file_path)
@@ -593,14 +598,15 @@ def get_unique_values(key):
     """Returns a list with the different values a property has taken
     """
     empty_dict = dbus.Dictionary({}, signature='ss')
-    return _get_datastore().get_uniquevaluesfor(key, empty_dict)
+    return _get_datastore().get_uniquevaluesfor(key, empty_dict,
+                                                timeout=DBUS_READ_TIMEOUT)


 def delete(object_id):
     """Removes an object from persistent storage
     """
     if not os.path.exists(object_id):
-        _get_datastore().delete(object_id)
+        _get_datastore().delete(object_id, timeout=DBUS_WRITE_TIMEOUT)
     else:
         os.unlink(object_id)
         dir_path = os.path.dirname(object_id)
@@ -650,11 +656,13 @@ def write(metadata, file_path='', update_mtime=True, transfer_ownership=True):
             object_id = _get_datastore().update(metadata['uid'],
                                                  dbus.Dictionary(metadata),
                                                  file_path,
-                                                 transfer_ownership)
+                                                 transfer_ownership,
+                                                 timeout=DBUS_WRITE_TIMEOUT)
         else:
             object_id = _get_datastore().create(dbus.Dictionary(metadata),
                                                  file_path,
-                                                 transfer_ownership)
+                                                 transfer_ownership,
+                                                 timeout=DBUS_WRITE_TIMEOUT)
     else:
         object_id = _write_entry_on_external_device(metadata, file_path)

--
1.7.9