GNOME Bugzilla – Bug 686678
Make local addressbook backend use the SQLite DB exclusively
Last modified: 2012-11-08 08:51:30 UTC
Currently the local file backend of EDS uses Berkeley DB for storing plain vCards, indexed by UID and additionally it uses the SQLite database (EBookBackendSqliteDB) for summaries, which avoids parsing plain vCards on every query and therefore improves performance of queries. However, using two nested databases leads to unnecessary I/O and contributes to flash wear. It would be nice to reduce disk writes by removing the Berkeley DB interactions entirely, instead storing the plain vCard directly in the SQLite backend. As a side effect, this could clean up existing code and reduce the overall complexity of the local addressbook backend. Will provide proposed patches for this shortly.
Created attachment 227502 [details] [review] Minor fixup, fixes query terms including uid matching expressions This is just an additional patch to the ones I'm adding here, basically it's a non issue if you use the get_contact by id apis, but using query terms that include the E_CONTACT_UID breaks without this...
Created attachment 227503 [details] [review] Add e_book_backend_sqlitedb_set/get_revision() In order for the file backend to use SQLiteDB as a complete store, it needs somewhere to store the overall addressbook revision This patch extends EBookBackendSQLiteDB to include a column for the revision
Created attachment 227504 [details] [review] Make local backend use EBookBackendSQLiteDB exclusively This patch is the juice, and depends on the included patch for storing the revision in the sqlite. I've run some of the regression test on master with this applied and no problems in sight.
Note... I've revived the 'openismus-work-master' branch and brought it up to speed. All of these patches can equally be found on that branch.
Thanks for a bug report and a great job with the cleanup. Once an EBackendOfflineCache is written it'll replace the SQLiteDB cache, as it'll be an extended version of it, but that's far away, thus good you dive into it. The patches look good, if I had to pick a little nitpick, then it might be at the e_book_backend_file_upgrade_db(), the old_version not being defined as 'const', but I see it was there before your changes, thus feel free to change it or keep as is, I'm fine with it. I thought there is no issue with the patches, but I found some: a) if the old adressbook is empty, then the open of it claims an error about failed open, and the detailed error says: "Cannot open book: near "TABLE": syntax error". I get the same error after restart of all evolution's processes on the Personal book. b) if I create a new book, then I cannot do with it anything, like adding contacts to it. It doesn't claim any error in UI, neither on evolution's console, only the options are not available.
Hi, I've been spending last week getting ahead and hacking on other patches and now I've come back to this... So, yesterday I tried to reproduce your problems in a test case; I tried to a.) Create a new book b.) Close the new book without adding any contacts to it c.) Open the same book d.) Add a contact e.) Fetch the added contact. But I was not able to reproduce any error. So this morning I proceeded to compile the Evolution Mail program from master. I started up the source registry, addressbook and calendar services with XDG_*_HOME dirs pointing somewhere custom... and then fired up evolution mail with the same XDG dirs o Went through the steps of adding an initial email o Checked the addressbook in the Evo UI o Closed the UI without adding any contacts o Restarted all EDS processes and restarted Evo Up to here I still don't get the "Cannot open book:..." error o Added a new addressbook (by right clicking in the treeview and using the "New Address Book" context menu option). o Added contacts to the default "Personal" address book (by using the "New" button in the main toolbar) o Added contacts to the "Unnamed" address book I had previously created The new contacts appear in the right hand area, I can select them and view the details I previously entered in the dialog. The one error I do get is when actually committing a new contact (i.e. when pressing OK in the "Add New Contact" dialog), I get this warning message: "You are attempting to move a contact from one address book to another but it cannot be removed from the source. Do you want to save a copy instead?" I'm quite sure this is unrelated, in any case I just select "Yes" and the new contact is added without any trouble. Any further tips on how to reproduce this problem ? I'll try compiling EDS without the BDB removal and experiment with the cross-over.
Created attachment 228338 [details] [review] Fixed syntax errors in previous patch This is the same patch with the SQL syntax error upon upgrading out of date DB fixed. Changed: UPDATE TABLE folders SET version = 2 to: UPDATE folders SET version = 2
Created attachment 228339 [details] [review] Updated patch to make local backend use the SQLiteDB exclusively This second version of the patch fixes 2 things: o Avoid calling e_book_backend_sqlitedb_add_contacts() with a null contact list (this was causing an assertion) o Fixed the migration routine to open the BDB with write access This is needed in order to maintain the previous portions of the BDB upgrades, i.e. when an older BDB version is encountered we use the same older upgrade paths to arrive at the current version, and then afterwards migrate those contacts to the SQLite DB.
Milan, I spent the time to debug the scenario and did reproduce the errors you found while using the Evolution Mail client I used 2 scenarios to reproduce & test with: o Compiled Evolution/EDS current master o Ran Evolution (and EDS processes) with relocated XDG_DATA_HOME, XDG_CONFIG_HOME & XDG_CACHE_HOME (all pointing to ~/tmp_evo) o Accessed the "Contacts" page in the UI and did not add any contacts o Stopped Evolution & EDS processes o Copied the contents of ~/tmp_evo/evolution into a backup directory o Started Evolution & EDS processes again o Created some test contacts as well as an extra test addressbook o Stopped Evolution & EDS processes o Copied the contents of ~/tmp_evo/evolution into another backup directory At this point I had good samples of what Evolution creates for both an empty and populated addressbook... so I proceeded to o Compile & install openismus-work-master branch again o Fire up Evolution & EDS processes again and encounter the errors of which you spoke After some debugging I have the migration working cleanly: o Migrating from an empty addressbook works, can add contacts after the migration o Migration from my populated test addressbook also works, the contacts I added to the "Personal" and added "Custom" addressbook appear in the Evolution UI after the migration takes place. I've updated attached patches with the relevant fixes. However there remains this one sticky thing, the warning I spoke of in my first comment today: "You are attempting to move a contact from one address book to another but it cannot be removed from the source. Do you want to save a copy instead?" Still appears... I have really no clue where that error is coming from.
Comment on attachment 228338 [details] [review] Fixed syntax errors in previous patch Only fix the C++ comment (//), change it to C comment style (/* */). If you want, then I would not add two new line breaks after declaration of e_book_backend_sqlitedb_set_revision(), but that's really minor thing.
Comment on attachment 228339 [details] [review] Updated patch to make local backend use the SQLiteDB exclusively Do not forget to turn off d(x) in e-book-backend-file.c before committing. e_book_backend_file_open() has set readonly=TRUE at is beginning, then it's never changed, thus the books are readonly, which explains the warning on contact save. Otherwise looks good, please commit
Patches committed in master, I suppose we can close this one now.