Redneys
Redneys

Reputation: 305

Firestore Concurrent Transactions Freezing

I am having an issue where Firestore transactions freeze when run concurrently. As background, I have a Firestore collection, where each document has a dollar amount and a time. I am creating a function that releases a desired dollar amount from this collection, starting with the oldest documents.

For example, a function call to release $150 will iterate through the collection, removing dollar amounts from the collection until a total of $150 is removed. I do this using a recursive function which 1) finds the oldest dollar amount, 2) removes the inputted amount (i.e. $150) from that number, or deletes the number if the inputted amount is greater than the number, and 3) recurs if there is still a remaining amount to be removed. I use a Firestore transaction for step (2), as there is the possibility this collection will be changed by multiple users at the same time (and note that if I combine (1) and (2) to include the query in the transaction, the code behavior is unchanged).

The code below updates the collection correctly. However, it takes an extremely long time if it is called while an earlier instance is already running: if I call it once, then call it again before the first call is completed, it freezes and has potential to take 20-30 minutes instead of the usual 1-5 seconds. While it is clear that contention between concurrent transactions is causing the freezing (when I remove the write portion of the transaction, there is no freezing), the unknown question is what specifically about the contention is causing the freezing and how to fix it.

Addition: it seems this freezing may be related to https://github.com/firebase/firebase-tools/issues/2452. Consistent with that post, I am facing a 30 second freeze per transaction, which becomes many minutes given a single release has multiple transactions.

 function releaseAmountFromStack(amount) {
  return new Promise((resolve, reject) => {
    let db = admin.firestore();
    let stackRef = db.collection("stack");

    stackRef.orderBy("expirationTime", "asc").limit(1)
      .get().then((querySnapshot) => {
        if(querySnapshot.empty) {
          return reject("None left in stack");
        }

        let itemToRelease = querySnapshot.docs[0];

        releaseItem(itemToRelease.ref, amount)
        .then((actualReleaseAmount) => {
          // If there is still more to release, trigger the next recursion
          // If the full amount has been released, return it
          if (amount > actualReleaseAmount) {
            releaseAmountFromStack(amount-actualReleaseAmount)
            .then((nextActualReleaseAmount) => {
              return resolve(actualReleaseAmount + nextActualReleaseAmount);
            })
            .catch(() => {
              return resolve(actualReleaseAmount);
            });
          } else {
            return resolve(actualReleaseAmount);
          }
        });
    });
  });
}

function releaseItem(itemRef, amountToRelease) {
  let db = admin.firestore();
  return db.runTransaction((transaction) => {
    return transaction.get(itemRef).then((itemDoc) => {
      let itemAmount = itemDoc.data().amount;
      let actualReleaseAmount = Math.min(amountToRelease, itemAmount);

      // If item is exhausted, delete it. Else, update amount
      if (actualReleaseAmount >= itemAmount) {
        transaction.delete(itemDoc.ref);
      } else {
        transaction.set(itemDoc.ref, {
          amount: admin.firestore.FieldValue.increment(-1*Number(actualReleaseAmount)),
        }, {merge: true});
      }
      return actualReleaseAmount;
      });
  });
}

Here are some useful facts from the debug process so far. Thank you so much.

Upvotes: 1

Views: 419

Answers (2)

Redneys
Redneys

Reputation: 305

I figured out what caused the elusive freezing. I noticed that the freezing of each transaction was consistently 30 seconds, which prompted me to search for discussion on 30 second transaction freezes. I found (https://github.com/firebase/firebase-tools/issues/2452) which indicates this is an issue with how the Firebase emulator handles concurrent transactions. Indeed, when I deploy the code instead of using the emulator, the freezing is no longer there!

TLDR: the Firebase emulator has an unnatural delay with concurrent transactions.

Upvotes: 1

samthecodingman
samthecodingman

Reputation: 26171

First, let's fix your releaseAmountFromStack function so that you aren't using a Promise constructor for a Promise-returning API (known as the Explicit Promise Construction Antipattern). If either your stackRef query or releaseItem function throw an error, your code would encounter an UnhandledPromiseRejection because neither Promise chain has a catch handler.

function releaseAmountFromStack(amount) {
  const db = admin.firestore();
  const stackRef = db.collection("stack");

  return stackRef
    .orderBy("expirationTime", "asc")
    .limit(1)
    .get()
    .then((querySnapshot) => {
      if(querySnapshot.empty) {
        return Promise.reject("Out of stock");
      }

      const itemToRelease = querySnapshot.docs[0];

      return releaseItem(itemToRelease.ref, amount);
    })
    .then((releasedAmount) => {
       const amountLeft = amount - releasedAmount;

       if (amountLeft <= 0)
         return releasedAmount;

       return releaseAmountFromStack(amountLeft)
         .then((nextReleasedAmount) => nextReleasedAmount + releasedAmount)
         .catch((err) => {
           if (err === "Out of stock") {
             return releasedAmount;
           } else {
             // rethrow unexpected errors
             throw err;
           }
         });
     });
}

Because that function involves nested Promise chains, switching to an async/await syntax can flatten it out to:

async function releaseAmountFromStack(amount) {
  const db = admin.firestore();
  const stackRef = db.collection("stack");

  const querySnapshot = await stackRef
    .orderBy("expirationTime", "asc")
    .limit(1)
    .get();
  
  if (querySnapshot.empty)
    return 0; // out of stock

  const itemToRelease = querySnapshot.docs[0];

  const releasedAmount = await releaseItem(itemToRelease.ref, amount);

  const amountLeft = amount - releasedAmount;
  
  if (amountLeft <= 0) {
    // nothing left to release, return released amount
    return releasedAmount; 
  }

  // If here, there is more to release, trigger the next recursion
  const nextReleasedAmount = await releaseAmountFromStack(amountLeft);

  return nextReleasedAmount + releasedAmount;
}

Note: In the above flattened version, we don't need to handle catch because we can just return 0 instead. This means any irrelevant errors will be thrown normally.

Next we can move on to releaseItem, which is the actual cause of your problem. Here, you aren't handling the case where another instance is deleting the item you are reading, you just assume that it exists and then end up dealing with NaN and/or negative balances. As an example of what ends up happening, you can end up with the following sequence of events (it's more complex than this because you are using FieldValue.increment() - add in a Server Client doing transactions too):

 Client A                      Server                      Client B
Release 50                                                Release 80
    ┌┐         Get Doc #1        ┌┐                           ┌┐
    ││     ────────────────►     ││                           ││
    ││                           ││                           ││
    ││                           ││                           ││
    ││      Doc #1 Snapshot      ││         Get Doc #1        ││
    ││       { amount: 10 }      ││     ◄────────────────     ││
    ││     ◄────────────────     ││                           ││
    ││                           ││                           ││
    ││                           ││      Doc #1 Snapshot      ││
    ││                           ││       { amount: 10 }      ││
    ││   Result: Delete Doc #1   ││     ────────────────►     ││
    ││     ────────────────►     ││                           ││
    ││          ACCEPTED         ││                           ││
    ││     ◄────────────────     ││                           ││
    ││                           ││   Result: Delete Doc #1   ││
    ││         Get Doc #2        ││     ◄────────────────     ││
    ││     ────────────────►     ││                           ││
    ││                           ││         REJECTED          ││
    ││                           ││    New Doc #1 Snapshot    ││
    ││                           ││          <null>           ││
    ││      Doc #2 Snapshot      ││     ────────────────►     ││
    ││      { amount: 15 }       ││                           ││
    ││     ◄────────────────     ││                           ││
    ││                           ││                           ││
    ││                           ││ Result: Set Doc #1 to -10 ││
    ││                           ││     ◄────────────────     ││
    ││   Result: Delete Doc #2   ││             ▲             ││
    ││     ────────────────►     ││             │             ││
    ││          ACCEPTED         ││           ERROR           ││
    ││     ◄────────────────     ││                           ││
    └┘                           └┘                           └┘

Because you are using a transaction, you already know the current value for amount so you can just do the calculation on the client and write the new amount instead of using FieldValue.increment(). That operator is better suited for doing simple updates of a value where you don't need to know the current value.

function releaseItem(itemRef, amountToRelease) {
  const db = admin.firestore();
  return db.runTransaction((transaction) => {
    return transaction.get(itemRef).then((itemDoc) => {
      if (!itemDoc.exists) {
        // target has been deleted, do nothing & return
        // amount that was released (0)
        return 0;
      }
  
      const itemAmount = itemDoc.get("amount");
      const actualReleaseAmount = Math.min(amountToRelease, itemAmount);

      if (actualReleaseAmount >= itemAmount) {
        // exhausted supply. delete item
        transaction.delete(itemDoc.ref);
      } else {
        // have leftover supply. update amount
        transaction.set(itemDoc.ref, {
          amount: itemAmount - actualReleaseAmount, 
        }, {merge: true});
      }

      // return amount that was released
      return actualReleaseAmount;
    });
  });
}

This won't solve your issue completely as you still have a data contention problem if two or more clients try to take items off the same stack at the same time. As an example of this, see the following event flow (yet again timing will be a major factor in how things play out):

 Client A                      Server                      Client B
Release 50                                                Release 80
    ┌┐         Get Doc #1        ┌┐                           ┌┐
    ││     ────────────────►     ││                           ││
    ││                           ││                           ││
    ││                           ││                           ││
    ││      Doc #1 Snapshot      ││         Get Doc #1        ││
    ││       { amount: 10 }      ││     ◄────────────────     ││
    ││     ◄────────────────     ││                           ││
    ││                           ││                           ││
    ││                           ││      Doc #1 Snapshot      ││
    ││                           ││       { amount: 10 }      ││
    ││   Result: Delete Doc #1   ││     ────────────────►     ││
    ││     ────────────────►     ││                           ││
    ││          ACCEPTED         ││                           ││
    ││     ◄────────────────     ││                           ││
    ││                           ││   Result: Delete Doc #1   ││
    ││         Get Doc #2        ││     ◄────────────────     ││
    ││     ────────────────►     ││                           ││
    ││                           ││         REJECTED          ││
    ││                           ││    New Doc #1 Snapshot    ││
    ││                           ││          <null>           ││
    ││      Doc #2 Snapshot      ││     ────────────────►     ││
    ││      { amount: 15 }       ││                           ││
    ││     ◄────────────────     ││     Result: Cancelled     ││
    ││                           ││     ◄────────────────     ││
    ││                           ││                           ││
    ││                           ││                           ││
    ││   Result: Delete Doc #2   ││         Get Doc #2        ││
    ││     ────────────────►     ││     ◄────────────────     ││
    ││          ACCEPTED         ││                           ││
    ││     ◄────────────────     ││                           ││
    ││                           ││      Doc #2 Snapshot      ││
    ││         Get Doc #3        ││          <null>           ││
    ││     ────────────────►     ││     ────────────────►     ││
    ││                           ││                           ││
    ││                           ││     Result: Cancelled     ││
    ││                           ││     ◄────────────────     ││
    ││      Doc #3 Snapshot      ││                           ││
    ││      { amount: 10 }       ││                           ││
    ││     ◄────────────────     ││         Get Doc #3        ││
    ││                           ││     ◄────────────────     ││
    ││                           ││                           ││
    ││                           ││                           ││
    ││   Result: Delete Doc #3   ││      Doc #3 Snapshot      ││
    ││     ────────────────►     ││          <null>           ││
    ││          ACCEPTED         ││     ────────────────►     ││
    ││     ◄────────────────     ││                           ││
    ││                           ││     Result: Cancelled     ││
    ││         Get Doc #4        ││     ◄────────────────     ││
    └┘     ────────────────►     └┘                           └┘

One way around this would be to pull down multiple items at a time, consume them as appropriate and write the changes back, all in a transaction. Yet again, not dealing with the problem, just making it less likely that clients will fight over the same documents over and over again.

async function releaseAmountFromStack(amount) {
  const db = admin.firestore();
  const stackQuery = db
    .collection("stack")
    .orderBy("expirationTime", "asc")
    .limit(1);

  const releasedAmount = await _releaseAmountFromStack(stackQuery, amount);

  const amountLeft = amount - releasedAmount;

  if (amountLeft <= 0) {
    // nothing left to release, return released amount
    return releasedAmount; 
  }

  // If here, there is more to release, trigger the next recursion
  const nextReleasedAmount = await releaseAmountFromStack(amountLeft)
    .catch((err) => {
      if (err !== "Empty stack") throw err;
      return 0;
    });

  return nextReleasedAmount + releasedAmount;
}

function _releaseAmountFromStack(query, amountToRelease) => {
  return db.runTransaction((transaction) => {
    return transaction.get(query).then((querySnapshot) => {
      if (!querySnapshot.empty) {
        // nothing in stack, return released amount (0)
        return Promise.reject("Empty stack");
      }

      let remainingAmountToRelease = amountToRelease;

      for (const doc of querySnapshot.docs) {
        const itemAmount = doc.get("amount");
        const amountChange = Math.min(itemAmount, remainingAmountToRelease);

        if (amountChange >= itemAmount) {
          transaction.delete(doc.ref);
        } else {
          remainingAmountToRelease -= amountChange;
          transaction.set(doc.ref, {
            amount: itemAmount - amountChange
          }, { merge: true });
        }

        if (remainingAmountToRelease <= 0) break; // stop iterating early
      }
  
      // return amount that was released
      return /* totalAmountReleased = */ amountToRelease - remainingAmountToRelease;
    });
  });
}

Upvotes: 1

Related Questions