Change write groups to use transactions (#912)

* Start work on leveldb transactions

* Style

* Working API. Not plumbed in yet

* Move files into correct place

* Wrangling file locations and associations

* Tests pass

* Add some comments

* style

* Fix copyright

* Rewrite iterator internals to handle deletion-while-iterating. Also add tests for same

* Switch to strings instead of slices

* Style

* More style fixes

* Start switching writegroup over

* Swap out write group tracking for transaction usage

* Style

* Response to feedback before updating docs

* Style

* Add comment

* Initialize version_

* Satisfy the linter

* Start switching writegroup over

* Swap out write group tracking for transaction usage

* Style

* Checkpoint before implementing BatchDescription

* Style

* Everything passing

* Drop unused function

* Style

* Renaming

* Style

* Add log line

* Style

* Add debug log line for commits, drop unused BatchDescription

12 files changed, 164 insertions, 168 deletions

diff --git a/Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm b/Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm
index 3559d5d..822e3ca 100644
--- a/Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm
+++ b/Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm
@@ -18,10 +18,10 @@
 #include <leveldb/db.h>
 
 #import "Firestore/Protos/objc/firestore/local/Target.pbobjc.h"
+#import "Firestore/Source/Local/FSTLevelDB.h"
 #import "Firestore/Source/Local/FSTLevelDBKey.h"
 #import "Firestore/Source/Local/FSTLevelDBMigrations.h"
 #import "Firestore/Source/Local/FSTLevelDBQueryCache.h"
-#import "Firestore/Source/Local/FSTWriteGroup.h"
 
 #include "Firestore/core/src/firebase/firestore/util/ordered_code.h"
 
@@ -29,6 +29,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+using firebase::firestore::local::LevelDbTransaction;
 using firebase::firestore::util::OrderedCode;
 using leveldb::DB;
 using leveldb::Options;
@@ -60,43 +61,52 @@ using leveldb::Status;
 - (void)testAddsTargetGlobal {
   FSTPBTargetGlobal *metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db];
   XCTAssertNil(metadata, @"Not expecting metadata yet, we should have an empty db");
-  [FSTLevelDBMigrations runMigrationsOnDB:_db];
+  LevelDbTransaction transaction(_db.get());
+  [FSTLevelDBMigrations runMigrationsWithTransaction:&transaction];
+  transaction.Commit();
   metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db];
   XCTAssertNotNil(metadata, @"Migrations should have added the metadata");
 }
 
 - (void)testSetsVersionNumber {
-  FSTLevelDBSchemaVersion initial = [FSTLevelDBMigrations schemaVersionForDB:_db];
+  LevelDbTransaction transaction(_db.get());
+  FSTLevelDBSchemaVersion initial =
+      [FSTLevelDBMigrations schemaVersionWithTransaction:&transaction];
   XCTAssertEqual(0, initial, "No version should be equivalent to 0");
 
   // Pick an arbitrary high migration number and migrate to it.
-  [FSTLevelDBMigrations runMigrationsOnDB:_db];
-  FSTLevelDBSchemaVersion actual = [FSTLevelDBMigrations schemaVersionForDB:_db];
+  [FSTLevelDBMigrations runMigrationsWithTransaction:&transaction];
+  FSTLevelDBSchemaVersion actual = [FSTLevelDBMigrations schemaVersionWithTransaction:&transaction];
   XCTAssertGreaterThan(actual, 0, @"Expected to migrate to a schema version > 0");
 }
 
 - (void)testCountsQueries {
   NSUInteger expected = 50;
-  FSTWriteGroup *group = [FSTWriteGroup groupWithAction:@"Setup"];
-  for (int i = 0; i < expected; i++) {
-    std::string key = [FSTLevelDBTargetKey keyWithTargetID:i];
-    [group setData:"dummy" forKey:key];
+  {
+    // Setup some targets to be counted in the migration.
+    LevelDbTransaction transaction(_db.get());
+    for (int i = 0; i < expected; i++) {
+      std::string key = [FSTLevelDBTargetKey keyWithTargetID:i];
+      transaction.Put(key, "dummy");
+    }
+    // Add a dummy entry after the targets to make sure the iteration is correctly bounded.
+    // Use a table that would sort logically right after that table 'target'.
+    std::string dummyKey;
+    // Magic number that indicates a table name follows. Needed to mimic the prefix to the target
+    // table.
+    OrderedCode::WriteSignedNumIncreasing(&dummyKey, 5);
+    OrderedCode::WriteString(&dummyKey, "targetA");
+    transaction.Put(dummyKey, "dummy");
+    transaction.Commit();
+  }
+
+  {
+    LevelDbTransaction transaction(_db.get());
+    [FSTLevelDBMigrations runMigrationsWithTransaction:&transaction];
+    transaction.Commit();
+    FSTPBTargetGlobal *metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db];
+    XCTAssertEqual(expected, metadata.targetCount, @"Failed to count all of the targets we added");
   }
-  // Add a dummy entry after the targets to make sure the iteration is correctly bounded.
-  // Use a table that would sort logically right after that table 'target'.
-  std::string dummyKey;
-  // Magic number that indicates a table name follows. Needed to mimic the prefix to the target
-  // table.
-  OrderedCode::WriteSignedNumIncreasing(&dummyKey, 5);
-  OrderedCode::WriteString(&dummyKey, "targetA");
-  [group setData:"dummy" forKey:dummyKey];
-
-  Status status = [group writeToDB:_db];
-  XCTAssertTrue(status.ok(), @"Failed to write targets");
-
-  [FSTLevelDBMigrations runMigrationsOnDB:_db];
-  FSTPBTargetGlobal *metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db];
-  XCTAssertEqual(expected, metadata.targetCount, @"Failed to count all of the targets we added");
 }
 
 @end
diff --git a/Firestore/Example/Tests/Local/FSTWriteGroupTests.mm b/Firestore/Example/Tests/Local/FSTWriteGroupTests.mm
index edd05a3..871bd99 100644
--- a/Firestore/Example/Tests/Local/FSTWriteGroupTests.mm
+++ b/Firestore/Example/Tests/Local/FSTWriteGroupTests.mm
@@ -80,26 +80,6 @@ NS_ASSUME_NONNULL_BEGIN
   XCTAssertTrue(status.IsNotFound());
 }
 
-- (void)testDescription {
-  std::string key = [FSTLevelDBMutationKey keyWithUserID:"user1" batchID:42];
-  FSTPBWriteBatch *message = [FSTPBWriteBatch message];
-  message.batchId = 42;
-
-  FSTWriteGroup *group = [FSTWriteGroup groupWithAction:@"Action"];
-  XCTAssertEqualObjects([group description], @"<FSTWriteGroup for Action: 0 changes (0 bytes):>");
-
-  [group setMessage:message forKey:key];
-  XCTAssertEqualObjects([group description],
-                        @"<FSTWriteGroup for Action: 1 changes (2 bytes):\n"
-                         "  - Put [mutation: userID=user1 batchID=42] (2 bytes)>");
-
-  [group removeMessageForKey:key];
-  XCTAssertEqualObjects([group description],
-                        @"<FSTWriteGroup for Action: 2 changes (2 bytes):\n"
-                         "  - Put [mutation: userID=user1 batchID=42] (2 bytes)\n"
-                         "  - Delete [mutation: userID=user1 batchID=42]>");
-}
-
 - (void)testCommittingWrongGroupThrows {
   // If you don't create the group through persistence, it should throw.
   FSTWriteGroup *group = [FSTWriteGroup groupWithAction:@"group"];
diff --git a/Firestore/Source/Local/FSTLevelDB.mm b/Firestore/Source/Local/FSTLevelDB.mm
index ac29304..a7ee99d 100644
--- a/Firestore/Source/Local/FSTLevelDB.mm
+++ b/Firestore/Source/Local/FSTLevelDB.mm
@@ -31,6 +31,7 @@
 
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
 #include "Firestore/core/src/firebase/firestore/core/database_info.h"
+#include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 
@@ -43,6 +44,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 static NSString *const kReservedPathComponent = @"firestore";
 
+using firebase::firestore::local::LevelDbTransaction;
 using leveldb::DB;
 using leveldb::Options;
 using leveldb::ReadOptions;
@@ -58,7 +60,9 @@ using leveldb::WriteOptions;
 
 @end
 
-@implementation FSTLevelDB
+@implementation FSTLevelDB {
+  std::unique_ptr<LevelDbTransaction> _transaction;
+}
 
 /**
  * For now this is paranoid, but perhaps disable that in production builds.
@@ -137,7 +141,9 @@ using leveldb::WriteOptions;
     return NO;
   }
   _ptr.reset(database);
-  [FSTLevelDBMigrations runMigrationsOnDB:_ptr];
+  LevelDbTransaction transaction(_ptr.get());
+  [FSTLevelDBMigrations runMigrationsWithTransaction:&transaction];
+  transaction.Commit();
   return YES;
 }
 
@@ -212,20 +218,16 @@ using leveldb::WriteOptions;
 }
 
 - (FSTWriteGroup *)startGroupWithAction:(NSString *)action {
-  return [self.writeGroupTracker startGroupWithAction:action];
+  FSTAssert(_transaction == nullptr, @"Starting a transaction while one is already outstanding");
+  _transaction = std::make_unique<LevelDbTransaction>(_ptr.get());
+  return [self.writeGroupTracker startGroupWithAction:action transaction:_transaction.get()];
 }
 
 - (void)commitGroup:(FSTWriteGroup *)group {
+  FSTAssert(_transaction != nullptr, @"Committing a transaction before one is started");
   [self.writeGroupTracker endGroup:group];
-
-  NSString *description = [group description];
-  FSTLog(@"Committing %@", description);
-
-  Status status = [group writeToDB:_ptr];
-  if (!status.ok()) {
-    FSTFail(@"%@ failed with status: %s, description: %@", group.action, status.ToString().c_str(),
-            description);
-  }
+  _transaction->Commit();
+  _transaction.reset();
 }
 
 - (void)shutdown {
diff --git a/Firestore/Source/Local/FSTLevelDBMigrations.h b/Firestore/Source/Local/FSTLevelDBMigrations.h
index 24fb5c8..1724edf 100644
--- a/Firestore/Source/Local/FSTLevelDBMigrations.h
+++ b/Firestore/Source/Local/FSTLevelDBMigrations.h
@@ -18,6 +18,7 @@
 
 #include <memory>
 
+#include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h"
 #include "leveldb/db.h"
 
 NS_ASSUME_NONNULL_BEGIN
@@ -29,12 +30,13 @@ typedef int32_t FSTLevelDBSchemaVersion;
 /**
  * Returns the current version of the schema for the given database
  */
-+ (FSTLevelDBSchemaVersion)schemaVersionForDB:(std::shared_ptr<leveldb::DB>)db;
++ (FSTLevelDBSchemaVersion)schemaVersionWithTransaction:
+    (firebase::firestore::local::LevelDbTransaction *)transaction;
 
 /**
  * Runs any migrations needed to bring the given database up to the current schema version
  */
-+ (void)runMigrationsOnDB:(std::shared_ptr<leveldb::DB>)db;
++ (void)runMigrationsWithTransaction:(firebase::firestore::local::LevelDbTransaction *)transaction;
 
 @end
 
diff --git a/Firestore/Source/Local/FSTLevelDBMigrations.mm b/Firestore/Source/Local/FSTLevelDBMigrations.mm
index 7595c53..7f521d1 100644
--- a/Firestore/Source/Local/FSTLevelDBMigrations.mm
+++ b/Firestore/Source/Local/FSTLevelDBMigrations.mm
@@ -16,13 +16,12 @@
 
 #include "Firestore/Source/Local/FSTLevelDBMigrations.h"
 
+#include <absl/strings/match.h>
 #include "leveldb/write_batch.h"
 
 #import "Firestore/Protos/objc/firestore/local/Target.pbobjc.h"
-#import "Firestore/Source/Local/FSTLevelDB.h"
 #import "Firestore/Source/Local/FSTLevelDBKey.h"
 #import "Firestore/Source/Local/FSTLevelDBQueryCache.h"
-#import "Firestore/Source/Local/FSTWriteGroup.h"
 #import "Firestore/Source/Util/FSTAssert.h"
 
 NS_ASSUME_NONNULL_BEGIN
@@ -30,6 +29,7 @@ NS_ASSUME_NONNULL_BEGIN
 // Current version of the schema defined in this file.
 static FSTLevelDBSchemaVersion kSchemaVersion = 2;
 
+using firebase::firestore::local::LevelDbTransaction;
 using leveldb::DB;
 using leveldb::Iterator;
 using leveldb::Status;
@@ -38,24 +38,24 @@ using leveldb::WriteOptions;
 
 /**
  * Ensures that the global singleton target metadata row exists in LevelDB.
- * @param db The db in which to require the row.
  */
-static void EnsureTargetGlobal(std::shared_ptr<DB> db, FSTWriteGroup *group) {
-  FSTPBTargetGlobal *targetGlobal = [FSTLevelDBQueryCache readTargetMetadataFromDB:db];
+static void EnsureTargetGlobal(LevelDbTransaction *transaction) {
+  FSTPBTargetGlobal *targetGlobal =
+      [FSTLevelDBQueryCache readTargetMetadataWithTransaction:transaction];
   if (!targetGlobal) {
-    [group setMessage:[FSTPBTargetGlobal message] forKey:[FSTLevelDBTargetGlobalKey key]];
+    transaction->Put([FSTLevelDBTargetGlobalKey key], [FSTPBTargetGlobal message]);
   }
 }
 
 /**
  * Save the given version number as the current version of the schema of the database.
  * @param version The version to save
- * @param group The transaction in which to save the new version number
+ * @param transaction The transaction in which to save the new version number
  */
-static void SaveVersion(FSTLevelDBSchemaVersion version, FSTWriteGroup *group) {
+static void SaveVersion(FSTLevelDBSchemaVersion version, LevelDbTransaction *transaction) {
   std::string key = [FSTLevelDBVersionKey key];
   std::string version_string = std::to_string(version);
-  [group setData:version_string forKey:key];
+  transaction->Put(key, version_string);
 }
 
 /**
@@ -65,30 +65,32 @@ static void SaveVersion(FSTLevelDBSchemaVersion version, FSTWriteGroup *group) {
  *
  * It assumes the metadata has already been written and is able to be read in this transaction.
  */
-static void AddTargetCount(std::shared_ptr<DB> db, FSTWriteGroup *group) {
-  std::unique_ptr<Iterator> it(db->NewIterator([FSTLevelDB standardReadOptions]));
-  Slice start_key = [FSTLevelDBTargetKey keyPrefix];
+static void AddTargetCount(LevelDbTransaction *transaction) {
+  std::unique_ptr<LevelDbTransaction::Iterator> it(transaction->NewIterator());
+  std::string start_key = [FSTLevelDBTargetKey keyPrefix];
   it->Seek(start_key);
 
   int32_t count = 0;
-  while (it->Valid() && it->key().starts_with(start_key)) {
+  while (it->Valid() && absl::StartsWith(it->key(), start_key)) {
     count++;
     it->Next();
   }
 
-  FSTPBTargetGlobal *targetGlobal = [FSTLevelDBQueryCache readTargetMetadataFromDB:db];
+  FSTPBTargetGlobal *targetGlobal =
+      [FSTLevelDBQueryCache readTargetMetadataWithTransaction:transaction];
   FSTCAssert(targetGlobal != nil,
              @"We should have a metadata row as it was added in an earlier migration");
   targetGlobal.targetCount = count;
-  [group setMessage:targetGlobal forKey:[FSTLevelDBTargetGlobalKey key]];
+  transaction->Put([FSTLevelDBTargetGlobalKey key], targetGlobal);
 }
 
 @implementation FSTLevelDBMigrations
 
-+ (FSTLevelDBSchemaVersion)schemaVersionForDB:(std::shared_ptr<DB>)db {
++ (FSTLevelDBSchemaVersion)schemaVersionWithTransaction:
+    (firebase::firestore::local::LevelDbTransaction *)transaction {
   std::string key = [FSTLevelDBVersionKey key];
   std::string version_string;
-  Status status = db->Get([FSTLevelDB standardReadOptions], key, &version_string);
+  Status status = transaction->Get(key, &version_string);
   if (status.IsNotFound()) {
     return 0;
   } else {
@@ -96,34 +98,24 @@ static void AddTargetCount(std::shared_ptr<DB> db, FSTWriteGroup *group) {
   }
 }
 
-+ (void)runMigrationsOnDB:(std::shared_ptr<DB>)db {
-  FSTWriteGroup *group = [FSTWriteGroup groupWithAction:@"Migrations"];
-  FSTLevelDBSchemaVersion currentVersion = [self schemaVersionForDB:db];
++ (void)runMigrationsWithTransaction:(firebase::firestore::local::LevelDbTransaction *)transaction {
+  FSTLevelDBSchemaVersion currentVersion = [self schemaVersionWithTransaction:transaction];
   // Each case in this switch statement intentionally falls through. This lets us
   // start at the current schema version and apply any migrations that have not yet
   // been applied, to bring us up to current, as defined by the kSchemaVersion constant.
   switch (currentVersion) {
     case 0:
-      EnsureTargetGlobal(db, group);
+      EnsureTargetGlobal(transaction);
       // Fallthrough
     case 1:
-      // We need to make sure we have metadata, since we're going to read and modify it
-      // in this migration. Commit the current transaction and start a new one. Since we're
-      // committing, we need to save a version. It's safe to save this one, if we crash
-      // after saving we'll resume from this step when we try to migrate.
-      SaveVersion(1, group);
-      [group writeToDB:db];
-      group = [FSTWriteGroup groupWithAction:@"Migrations"];
-      AddTargetCount(db, group);
+      // We're now guaranteed that the target global exists. We can safely add a count to it.
+      AddTargetCount(transaction);
       // Fallthrough
     default:
       if (currentVersion < kSchemaVersion) {
-        SaveVersion(kSchemaVersion, group);
+        SaveVersion(kSchemaVersion, transaction);
       }
   }
-  if (!group.isEmpty) {
-    [group writeToDB:db];
-  }
 }
 
 @end
diff --git a/Firestore/Source/Local/FSTLevelDBQueryCache.h b/Firestore/Source/Local/FSTLevelDBQueryCache.h
index 1f6fbd4..d955b02 100644
--- a/Firestore/Source/Local/FSTLevelDBQueryCache.h
+++ b/Firestore/Source/Local/FSTLevelDBQueryCache.h
@@ -19,6 +19,7 @@
 #include <memory>
 
 #import "Firestore/Source/Local/FSTQueryCache.h"
+#include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h"
 #include "leveldb/db.h"
 
 @class FSTLocalSerializer;
@@ -32,9 +33,16 @@ NS_ASSUME_NONNULL_BEGIN
 
 /**
  * Retrieves the global singleton metadata row from the given database, if it exists.
+ * TODO(gsoltis): remove this method once fully ported to transactions.
  */
 + (nullable FSTPBTargetGlobal *)readTargetMetadataFromDB:(std::shared_ptr<leveldb::DB>)db;
 
+/**
+ * Retrieves the global singleton metadata row using the given transaction, if it exists.
+ */
++ (nullable FSTPBTargetGlobal *)readTargetMetadataWithTransaction:
+    (firebase::firestore::local::LevelDbTransaction *)transaction;
+
 - (instancetype)init NS_UNAVAILABLE;
 
 /** The garbage collector to notify about potential garbage keys. */
diff --git a/Firestore/Source/Local/FSTLevelDBQueryCache.mm b/Firestore/Source/Local/FSTLevelDBQueryCache.mm
index fe1bf19..95b032e 100644
--- a/Firestore/Source/Local/FSTLevelDBQueryCache.mm
+++ b/Firestore/Source/Local/FSTLevelDBQueryCache.mm
@@ -31,6 +31,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+using firebase::firestore::local::LevelDbTransaction;
 using Firestore::StringView;
 using leveldb::DB;
 using leveldb::Iterator;
@@ -59,6 +60,30 @@ using leveldb::WriteOptions;
   FSTSnapshotVersion *_lastRemoteSnapshotVersion;
 }
 
++ (nullable FSTPBTargetGlobal *)readTargetMetadataWithTransaction:
+    (firebase::firestore::local::LevelDbTransaction *)transaction {
+  std::string key = [FSTLevelDBTargetGlobalKey key];
+  std::string value;
+  Status status = transaction->Get(key, &value);
+  if (status.IsNotFound()) {
+    return nil;
+  } else if (!status.ok()) {
+    FSTFail(@"metadataForKey: failed loading key %s with status: %s", key.c_str(),
+            status.ToString().c_str());
+  }
+
+  NSData *data =
+      [[NSData alloc] initWithBytesNoCopy:(void *)value.data() length:value.size() freeWhenDone:NO];
+
+  NSError *error;
+  FSTPBTargetGlobal *proto = [FSTPBTargetGlobal parseFromData:data error:&error];
+  if (!proto) {
+    FSTFail(@"FSTPBTargetGlobal failed to parse: %@", error);
+  }
+
+  return proto;
+}
+
 + (nullable FSTPBTargetGlobal *)readTargetMetadataFromDB:(std::shared_ptr<DB>)db {
   std::string key = [FSTLevelDBTargetGlobalKey key];
   std::string value;
diff --git a/Firestore/Source/Local/FSTWriteGroup.h b/Firestore/Source/Local/FSTWriteGroup.h
index c21ff72..a554597 100644
--- a/Firestore/Source/Local/FSTWriteGroup.h
+++ b/Firestore/Source/Local/FSTWriteGroup.h
@@ -19,6 +19,7 @@
 #include <memory>
 
 #include "Firestore/Source/Local/StringView.h"
+#include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h"
 #include "leveldb/db.h"
 
 NS_ASSUME_NONNULL_BEGIN
@@ -46,13 +47,15 @@ NS_ASSUME_NONNULL_BEGIN
  */
 + (instancetype)groupWithAction:(NSString *)action;
 
++ (instancetype)groupWithAction:(NSString *)action
+                    transaction:(firebase::firestore::local::LevelDbTransaction *)transaction;
+
 - (instancetype)init __attribute__((unavailable("Use a static constructor instead")));
 
 /** The action description assigned to this write group. */
 @property(nonatomic, copy, readonly) NSString *action;
 
-/** Returns YES if the write group has no messages in it. */
-- (BOOL)isEmpty;
+@property(nonatomic, readonly) firebase::firestore::local::LevelDbTransaction *transaction;
 
 /**
  * Marks the given key for deletion.
@@ -78,8 +81,8 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)setData:(Firestore::StringView)data forKey:(Firestore::StringView)key;
 
-/** Writes the contents to the given LevelDB. */
-- (leveldb::Status)writeToDB:(std::shared_ptr<leveldb::DB>)db;
+/** Returns YES if the write group has no messages in it. */
+- (BOOL)isEmpty;
 
 @end
 
diff --git a/Firestore/Source/Local/FSTWriteGroup.mm b/Firestore/Source/Local/FSTWriteGroup.mm
index 80d09ca..9aee31a 100644
--- a/Firestore/Source/Local/FSTWriteGroup.mm
+++ b/Firestore/Source/Local/FSTWriteGroup.mm
@@ -16,13 +16,12 @@
 
 #import "Firestore/Source/Local/FSTWriteGroup.h"
 
-#import <Protobuf/GPBProtocolBuffers.h>
-#include <leveldb/db.h>
 #include <leveldb/write_batch.h>
 
 #import "Firestore/Source/Local/FSTLevelDBKey.h"
 #import "Firestore/Source/Util/FSTAssert.h"
 
+using firebase::firestore::local::LevelDbTransaction;
 using Firestore::StringView;
 using leveldb::DB;
 using leveldb::Slice;
@@ -32,109 +31,62 @@ using leveldb::WriteOptions;
 
 NS_ASSUME_NONNULL_BEGIN
 
-namespace Firestore {
-
-/**
- * A WriteBatch::Handler implementation that extracts batch details from a leveldb::WriteBatch.
- * This is used for describing a write batch primarily in log messages after a failure.
- */
-class BatchDescription : public WriteBatch::Handler {
- public:
-  BatchDescription() : ops_(0), size_(0), message_([NSMutableString string]) {
-  }
-  virtual ~BatchDescription();
-  virtual void Put(const Slice &key, const Slice &value);
-  virtual void Delete(const Slice &key);
-
-  // Converts the batch to a printable string description of it
-  NSString *ToString() const {
-    return [NSString
-        stringWithFormat:@"%d changes (%lu bytes):%@", ops_, (unsigned long)size_, message_];
-  }
-
-  // Disallow copies and moves
-  BatchDescription(const BatchDescription &) = delete;
-  BatchDescription &operator=(const BatchDescription &) = delete;
-  BatchDescription(BatchDescription &&) = delete;
-  BatchDescription &operator=(BatchDescription &&) = delete;
-
- private:
-  int ops_;
-  size_t size_;
-  NSMutableString *message_;
-};
-
-BatchDescription::~BatchDescription() {
-}
-
-void BatchDescription::Put(const Slice &key, const Slice &value) {
-  ops_ += 1;
-  size_ += value.size();
-
-  [message_ appendFormat:@"\n  - Put %@ (%lu bytes)", [FSTLevelDBKey descriptionForKey:key],
-                         (unsigned long)value.size()];
-}
-
-void BatchDescription::Delete(const Slice &key) {
-  ops_ += 1;
-
-  [message_ appendFormat:@"\n  - Delete %@", [FSTLevelDBKey descriptionForKey:key]];
-}
-
-}  // namespace Firestore
-
 @interface FSTWriteGroup ()
 - (instancetype)initWithAction:(NSString *)action NS_DESIGNATED_INITIALIZER;
+- (instancetype)initWithAction:(NSString *)action transaction:(LevelDbTransaction *)transaction;
 @end
 
 @implementation FSTWriteGroup {
   int _changes;
-  WriteBatch _contents;
 }
 
 + (instancetype)groupWithAction:(NSString *)action {
   return [[FSTWriteGroup alloc] initWithAction:action];
 }
 
++ (instancetype)groupWithAction:(NSString *)action
+                    transaction:(firebase::firestore::local::LevelDbTransaction *)transaction {
+  return [[FSTWriteGroup alloc] initWithAction:action transaction:transaction];
+}
+
 - (instancetype)initWithAction:(NSString *)action {
   if (self = [super init]) {
     _action = action;
+    _transaction = nullptr;
   }
   return self;
 }
 
-- (NSString *)description {
-  Firestore::BatchDescription description;
-  Status status = _contents.Iterate(&description);
-  if (!status.ok()) {
-    FSTFail(@"Iterate over write batch should not fail");
+- (instancetype)initWithAction:(NSString *)action transaction:(LevelDbTransaction *)transaction {
+  if (self = [self initWithAction:action]) {
+    _transaction = transaction;
   }
-  return [NSString
-      stringWithFormat:@"<FSTWriteGroup for %@: %@>", self.action, description.ToString()];
+  return self;
 }
 
 - (void)removeMessageForKey:(StringView)key {
-  _contents.Delete(key);
+  FSTAssert(_transaction != nullptr, @"Using group without a transaction");
+  Slice keySlice = key;
+  _transaction->Delete(keySlice.ToString());
   _changes += 1;
 }
 
 - (void)setMessage:(GPBMessage *)message forKey:(StringView)key {
-  NSData *data = [message data];
-  Slice value((const char *)data.bytes, data.length);
-
-  _contents.Put(key, value);
+  FSTAssert(_transaction != nullptr, @"Using group without a transaction");
+  Slice keySlice = key;
+  _transaction->Put(keySlice.ToString(), message);
   _changes += 1;
 }
 
 - (void)setData:(StringView)data forKey:(StringView)key {
-  _contents.Put(key, data);
+  FSTAssert(_transaction != nullptr, @"Using group without a transaction");
+  Slice keySlice = key;
+  Slice valueSlice = data;
+  std::string value = valueSlice.ToString();
+  _transaction->Put(keySlice.ToString(), value);
   _changes += 1;
 }
 
-- (leveldb::Status)writeToDB:(std::shared_ptr<leveldb::DB>)db {
-  return db->Write(leveldb::WriteOptions(), &_contents);
-}
-
 - (BOOL)isEmpty {
   return _changes == 0;
 }
diff --git a/Firestore/Source/Local/FSTWriteGroupTracker.h b/Firestore/Source/Local/FSTWriteGroupTracker.h
index bd26a46..c4651ab 100644
--- a/Firestore/Source/Local/FSTWriteGroupTracker.h
+++ b/Firestore/Source/Local/FSTWriteGroupTracker.h
@@ -15,6 +15,7 @@
  */
 
 #import <Foundation/Foundation.h>
+#import "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h"
 
 @class FSTWriteGroup;
 
@@ -37,6 +38,10 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (FSTWriteGroup *)startGroupWithAction:(NSString *)action;
 
+- (FSTWriteGroup *)startGroupWithAction:(NSString *)action
+                            transaction:
+                                (firebase::firestore::local::LevelDbTransaction *)transaction;
+
 /** Ends a group previously started with `startGroupWithAction`. */
 - (void)endGroup:(FSTWriteGroup *)group;
 
diff --git a/Firestore/Source/Local/FSTWriteGroupTracker.mm b/Firestore/Source/Local/FSTWriteGroupTracker.mm
index 7e3bf60..2cb10bd 100644
--- a/Firestore/Source/Local/FSTWriteGroupTracker.mm
+++ b/Firestore/Source/Local/FSTWriteGroupTracker.mm
@@ -40,6 +40,17 @@ NS_ASSUME_NONNULL_BEGIN
   return self.activeGroup;
 }
 
+- (FSTWriteGroup *)startGroupWithAction:(NSString *)action
+                            transaction:
+                                (firebase::firestore::local::LevelDbTransaction *)transaction {
+  // NOTE: We can relax this to allow nesting if/when we find we need it.
+  FSTAssert(!self.activeGroup,
+            @"Attempt to create write group (%@) while existing write group (%@) still active.",
+            action, self.activeGroup.action);
+  self.activeGroup = [FSTWriteGroup groupWithAction:action transaction:transaction];
+  return self.activeGroup;
+}
+
 - (void)endGroup:(FSTWriteGroup *)group {
   FSTAssert(self.activeGroup == group,
             @"Attempted to end write group (%@) which is different from active group (%@)",
diff --git a/Firestore/core/src/firebase/firestore/local/leveldb_transaction.cc b/Firestore/core/src/firebase/firestore/local/leveldb_transaction.cc
index 4e4a313..d6c9799 100644
--- a/Firestore/core/src/firebase/firestore/local/leveldb_transaction.cc
+++ b/Firestore/core/src/firebase/firestore/local/leveldb_transaction.cc
@@ -20,6 +20,7 @@
 
 #include "Firestore/core/src/firebase/firestore/local/leveldb_key.h"
 #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h"
+#include "Firestore/core/src/firebase/firestore/util/log.h"
 
 using leveldb::DB;
 using leveldb::ReadOptions;
@@ -202,9 +203,14 @@ void LevelDbTransaction::Commit() {
     batch.Put(it->first, it->second);
   }
 
+  if (util::LogGetLevel() <= util::kLogLevelDebug) {
+    util::LogDebug("Committing transaction: %s", ToString().c_str());
+  }
+
   Status status = db_->Write(write_options_, &batch);
-  FIREBASE_ASSERT_MESSAGE(status.ok(), "Failed to commit transaction: %s",
-                          status.ToString().c_str());
+  FIREBASE_ASSERT_MESSAGE(status.ok(),
+                          "Failed to commit transaction:\n%s\n Failed: %s",
+                          ToString().c_str(), status.ToString().c_str());
 }
 
 std::string LevelDbTransaction::ToString() {