From 5b882b55bec88bbb12d2ec279772e60a5fb95d12 Mon Sep 17 00:00:00 2001
From: Neil Williams <neil@reddit.com>
Date: Mon, 22 Oct 2012 16:33:51 -0700
Subject: [PATCH] thing: Use transactions for committing things.

This should protect us against partial commits leading to broken things
that have fewer data attributes than they're supposed to.  It should
also improve Postgres performance as we'll be doing fewer total
transactions.
---
 r2/r2/lib/db/thing.py | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/r2/r2/lib/db/thing.py b/r2/r2/lib/db/thing.py
index 817a2aa11..5345bf530 100644
--- a/r2/r2/lib/db/thing.py
+++ b/r2/r2/lib/db/thing.py
@@ -232,18 +232,30 @@ class DataThing(object):
         return self._dirty
 
     def _commit(self, keys=None):
-        if not self._created:
-            self._create()
-            just_created = True
-        else:
-            just_created = False
+        lock = None
 
-        with g.make_lock("thing_commit", 'commit_' + self._fullname):
-            if not self._sync_latest():
+        try:
+            if not self._created:
+                begin()
+                self._create()
+                just_created = True
+            else:
+                just_created = False
+
+            lock = g.make_lock("thing_commit", 'commit_' + self._fullname)
+            lock.acquire()
+
+            if not just_created and not self._sync_latest():
                 #sync'd and we have nothing to do now, but we still cache anyway
                 self._cache_myself()
                 return
 
+            # begin is a no-op if already done, but in the not-just-created
+            # case we need to do this here because the else block is not
+            # executed when the try block is exited prematurely in any way
+            # (including the return in the above branch)
+            begin()
+
             if keys:
                 keys = tup(keys)
                 to_set = dict((k, self._dirties[k][1])
@@ -276,6 +288,14 @@ class DataThing(object):
                 self._dirties.clear()
 
             self._cache_myself()
+        except:
+            rollback()
+            raise
+        else:
+            commit()
+        finally:
+            if lock:
+                lock.release()
 
     @classmethod
     def _load_multi(cls, need):