From 8d3264f7f207034131250ddb93f013a64ab38d05 Mon Sep 17 00:00:00 2001
From: Philip Jenvey <pjenvey@underboss.org>
Date: Fri, 13 Dec 2019 15:43:09 -0800
Subject: [PATCH] fix: lower max_total_records per batch_commit_update costs

and remove the potential, redundant touch_collection on batch commit

recalculating the mutations (see adca8d67):

- 4 for touch_collection: always an UPDATE on batch commit (due to
  pretouch_collection). we thought this cost 1 but it's probably 4 (see
  #333)
- 19992 (1666 max_total_records * 12, for the UPDATE case, see #333) for
  the mix of post_bsos and batch_commit
- 2 for batch delete (1 for the delete and 1 BsoExpiry update)

total: 19998

Closes #333
---
 spanner_config.ini       | 2 +-
 src/db/spanner/models.rs | 8 ++++++++
 src/web/handlers.rs      | 4 ----
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/spanner_config.ini b/spanner_config.ini
index b78b39cc32..0e3b32ac70 100644
--- a/spanner_config.ini
+++ b/spanner_config.ini
@@ -1,2 +1,2 @@
 # Temp limit on the number of incoming records (See issue #298/#333)
-limits.max_total_records=1999
+limits.max_total_records=1666
diff --git a/src/db/spanner/models.rs b/src/db/spanner/models.rs
index 308a8b2dc4..00360d351c 100644
--- a/src/db/spanner/models.rs
+++ b/src/db/spanner/models.rs
@@ -81,6 +81,8 @@ pub(super) struct SpannerDbSession {
     pub(super) mutations: Option<Vec<Mutation>>,
     in_write_transaction: bool,
     execute_sql_count: u64,
+    /// Whether touch_collection has already been called
+    touched_collection: bool,
 }
 
 #[derive(Clone, Debug)]
@@ -816,6 +818,11 @@ impl SpannerDb {
         // buffered on the client side and only issued to Spanner in the final
         // transaction Commit.
         let timestamp = self.timestamp()?;
+        if self.session.borrow().touched_collection {
+            // No need to touch it again
+            return Ok(timestamp);
+        }
+
         let sqlparams = params! {
             "fxa_uid" => user_id.fxa_uid.clone(),
             "fxa_kid" => user_id.fxa_kid.clone(),
@@ -858,6 +865,7 @@ impl SpannerDb {
             .param_types(sql_types)
             .execute(&self.conn)?;
         }
+        self.session.borrow_mut().touched_collection = true;
         Ok(timestamp)
     }
 
diff --git a/src/web/handlers.rs b/src/web/handlers.rs
index aa0c6ad2f5..8ac47f7e31 100644
--- a/src/web/handlers.rs
+++ b/src/web/handlers.rs
@@ -259,10 +259,6 @@ pub fn post_collection_batch(
                 // Spanner we would pay twice the mutations for those pending
                 // items (once writing them to to batch_bsos, then again
                 // writing them to bsos)
-
-                // NOTE: Unfortunately this means we make two calls to
-                // touch_collection (in post_bsos and then commit_batch). The
-                // second touch is redundant, writing the same timestamp
                 Either::A(
                     coll.db
                         .post_bsos(params::PostBsos {