7

Folks,

I'm looking for a design pattern that enables a UI thread to interact with a client-side SQLite database that may have bulk inserts (taking 10s of seconds), quick inserts, and reads, and doesn't block the UI thread.

I would like advice on whether or not I am using the optimal design pattern for this, as I have been recently debugging deadlock and synchronization issues and I am not 100% confident in my final product.

All DB access is now bottlenecked through a singleton class. Here is pseudocode showing how I am approaching writes in my singleton, DataManager:

public class DataManager {

  private SQLiteDatabase mDb;
  private ArrayList<Message> mCachedMessages;

  public ArrayList<Message> readMessages() { 
    return mCachedMessages; 
  }

  public void writeMessage(Message m) {
    new WriteMessageAsyncTask().execute(m);
  }

  protected synchronized void dbWriteMessage(Message m) {
    this.mDb.replace(MESSAGE_TABLE_NAME, null, m.toContentValues());
  }

  protected ArrayList<Message> dbReadMessages() {
    // SQLite query for messages
  }

  private class WriteMessageAsyncTask extends AsyncTask<Message, Void, ArrayList<Messages>> {
    protected Void doInBackground(Message... args) {
       DataManager.this.mDb.execSQL("BEGIN TRANSACTION;");
       DataManager.this.dbWriteMessage(args[0]);
       // More possibly expensive DB writes
       DataManager.this.mDb.execSQL("COMMIT TRANSACTION;");
       ArrayList<Messages> newMessages = DataManager.this.dbReadMessages();
       return newMessages;
    }
    protected void onPostExecute(ArrayList<Message> newMessages) {
       DataManager.this.mCachedMessages = newMessages;
    }
  }
}

Highlights:

  • First: all public write operations (writeMessage) happen via an AsyncTask, never on the main thread
  • Next: all write operations are synchronized and wrapped in BEGIN TRANSACTIONS
  • Next: read operations are non-synchronized, since they need not block during writes
  • Finally: the results of read operations are cached on the main thread in the onPostExecute

Does this represent the Android best practice for writing potentially large volumes of data to a SQLite database while minimizing impact to the UI thread? Are there any obvious synchronization issues with the pseudocode you see above?

Update

There is a significant bug in my code above, and it is as follows:

DataManager.this.mDb.execSQL("BEGIN TRANSACTION;");

That line acquires a lock on the database. However, it is a DEFERRED lock, so until a write happens, other clients can both read and write.

DataManager.this.dbWriteMessage(args[0]);

That line actually modifies the database. At this point, the lock is a RESERVED lock, so no other clients may write.

Note there are more possibly expensive DB writes after the first dbWriteMessage call. Assume that each write operation happens in a protected synchronized method. That means that a lock is acquire on DataManager, the write happens, and the lock is released. If WriteAsyncMessageTask is the only writer, this is fine.

Now let's assume that there is some other task that also does write operations, but does not use a transaction (because it's a quick write). Here's what it might look like:

  private class WriteSingleMessageAsyncTask extends AsyncTask<Message, Void, Message> {
    protected Message doInBackground(Message... args) {
       DataManager.this.dbWriteMessage(args[0]);
       return args[0];
    }
    protected void onPostExecute(Message newMessages) {
       if (DataManager.this.mCachedMessages != null)
         DataManager.this.mCachedMessages.add(newMessages);
    }
  }

In this case, if WriteSingleMessageAsyncTask is executing at the same time as WriteMessageAsyncTask, and WriteMessageAsyncTask has executed at least one write already, it is possible for WriteSingleMessageAsyncTask to call dbWriteMessage, acquire the lock on DataManager, but then be blocked from completing its write due to the RESERVED lock. WriteMessageAsyncTask is acquiring and giving up the lock on DataManager repeatedly, which is a problem.

The takeaway: combining transactions and singleton object-level locking could lead to deadlock. Make sure you have the object-level lock prior to beginning a transaction.

The fix to my original WriteMessageAsyncTask class:

   synchronized(DataManager.this) {
     DataManager.this.mDb.execSQL("BEGIN TRANSACTION;");
     DataManager.this.dbWriteMessage(args[0]);
     // More possibly expensive DB writes
     DataManager.this.mDb.execSQL("COMMIT TRANSACTION;");
   }

Update 2

Check out this video from Google I/O 2012: http://youtu.be/gbQb1PVjfqM?t=19m13s

It suggests a design pattern making use of the built-in exclusive transactions and then using yieldIfContendedSafely

esilver
  • 27,713
  • 23
  • 122
  • 168
  • If I may suggest something on the performance part: One thing I notice is that once you write new messages, you seem to be reading the *entire* messages in the db and setting mCachedMessages. Instead of that, you could simply add the newly added messages to the mCachedMessages list? This will save a lot of db reads. – Aswin Kumar Jul 18 '12 at 06:56
  • @AswinKumar That is true; I meant this to be a simplified example to represent best practices around synchronization. A performance optimization would indeed be to simply add to the cached list that is already there. – esilver Jul 18 '12 at 18:12

3 Answers3

2

I can't really say much about the synchronization/deadlock part, that would be hugely dependent on the rest of your code. Since DataManager class doesn't really interact with the UI, you might want to use a service (IntentService) rather than an AsyncTask. You can show notifications when you are done syncing. You don't really need onPostExecute() if you are not calling UI code.

Nikolay Elenkov
  • 52,576
  • 10
  • 84
  • 84
  • Assume that the UI does interact with DataManager - i.e. the UI thread would call dataManager.readMessages() when the Activity launches, restores, or receives a new message that there is an update available. – esilver Jul 18 '12 at 06:42
  • What's important is whether `DataManager` does anything directly involving the UI thread, i.e. updating text on a `TextView`, etc. If some activity method accesses global cached data via `DataManager` that does not count as interacting with the UI :) – Nikolay Elenkov Jul 18 '12 at 06:48
  • 1
    I'm looking for a design pattern that enables a UI thread to interact with a client-side SQLite database that may have bulk inserts, quick inserts, and reads, and doesn't block the UI thread. While it certainly is possible to have all SQLite database access go through an IntentService, that seems like it might be overkill. – esilver Jul 18 '12 at 17:56
  • Again, I don't know exactly how your system is setup, but you seem to be doing this: read data from db into a cache on startup, and the use the cache from activities. I am suggesting that you might want to move the filling the cache up/sync into a service. All direct reads from activities (if any) can and should `AsyncTask`s on demand. – Nikolay Elenkov Jul 19 '12 at 01:18
1

You may want to consider this info from the SDK (http://developer.android.com/reference/android/os/AsyncTask.html)

When first introduced, AsyncTasks were executed serially on a single background thread. Starting with DONUT, this was changed to a pool of threads allowing multiple tasks to operate in parallel. Starting with HONEYCOMB, tasks are executed on a single thread to avoid common application errors caused by parallel execution.

If you truly want parallel execution, you can invoke executeOnExecutor(java.util.concurrent.Executor, Object[]) with THREAD_POOL_EXECUTOR.

Alexander Kulyakhtin
  • 47,782
  • 38
  • 107
  • 158
0

FYI, every SQL statement ran on SQLite is ran under a transaction even if you don’t specify one.

Check below threads if you are doing Bulk Insert in SQLite:

  1. Android Database Transaction
  2. SQLite Bulk Insert
Community
  • 1
  • 1
Paresh Mayani
  • 127,700
  • 71
  • 241
  • 295
  • 2
    Thanks - the first link is a good intro to using transactions, and the second is good for performance. I'm looking for a design pattern that is strictly correct for handling bulk inserts, short inserts, and reads, and won't block the UI thread. – esilver Jul 18 '12 at 17:55