Skip to content

Commit

Permalink
fix: lower max_total_records per batch_commit_update costs
Browse files Browse the repository at this point in the history
and remove the potential, redundant touch_collection on batch commit

recalculating the mutations (see adca8d6):

- 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
  • Loading branch information
pjenvey committed Dec 13, 2019
1 parent a1b628a commit 8d3264f
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
2 changes: 1 addition & 1 deletion spanner_config.ini
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions src/db/spanner/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -858,6 +865,7 @@ impl SpannerDb {
.param_types(sql_types)
.execute(&self.conn)?;
}
self.session.borrow_mut().touched_collection = true;
Ok(timestamp)
}

Expand Down
4 changes: 0 additions & 4 deletions src/web/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 8d3264f

Please sign in to comment.