1

Consider the following:

function useCredits(userId, amount){
    var userRef = firebase.database().ref().child('users').child(userId);
    userRef.transaction(function(user) {

        if (!user){
            return user;
        }
        user.credits -= amount;
        return user;


    }, NOOP, false);
}

function notifyUser(userId, message){

    var notificationId = Math.random();
    var userNotificationRef = firebase.database().ref().child('users').child(userId).child('notifications').child(notificationId);
    userNotificationRef.transaction(function(notification) {

        return message;

    }, NOOP, false);
}

These are called from the same node js process.

A user looks like this:

{
  "name": 'Alex',
  "age": 22,
  "credits": 100,
  "notifications": {
    "1": "notification 1",
    "2": "notification 2"
  }
}

When I run my stress tests I notice that sometimes the user object passed to the userRef transaction update function is not the full user it is only the following:

{

  "notifications": {
    "1": "notification 1",
    "2": "notification 2"
  }
}

This obviously causes an Error because user.credits does not exist.

It is suspicious that the user object passed to update function of the userRef transaction is the same as the data returned by the userNotificationRef transaction's update function.

Why is this the case? This problem goes away if I run both transactions on the user parent location, but this is a less optimal solution as I am then effectively locking on and reading the whole user object, which is redundant when adding a write once notification.

user3391835
  • 305
  • 1
  • 3
  • 14

1 Answers1

3

In my experience, you can't rely on the initial value passed into a transaction update function. Even if the data is populated in the datastore, the function might be called with null, a partial value, or a stale old value (in case of a local update in flight). This is not usually a problem as long as you take a defensive approach when writing the function (and you should!), since the bogus update will be refused and the transaction retried.

But beware: if you abort the transaction (by returning undefined) because the data doesn't make sense, then it's not checked against the server and won't get retried. For this reason, I recommend never aborting transactions. I built a monkey patch to apply this fix (and others) transparently; it's browser-only but could be adapted to Node trivially.

Another thing you can do to help a bit is to insert an on('value') call on the same ref just before the transaction and keep it alive until the transaction completes. This will usually cause the transaction to run on the correct data on the first try, doesn't affect bandwidth too much (since the current value would need to be transmitted anyway), and increases local latency a little if you have applyLocally set or defaulting to true. I do this in my NodeFire library, among many other optimizations and tweaks.

On top of all the above, as of this writing there's still a bug in the SDK where very rarely the wrong base value will get "stuck" and the transaction retry continuously (failing with maxretry every so often) until you restart the process.

Good luck! I still use transactions in my server, where failures can be retried easily and I have multiple processes running, but have given up on using them on the client -- they're just too unreliable. In my opinion it's often better to redesign your data structures so that transactions aren't needed.

Piotr
  • 432
  • 5
  • 9
  • I had no idea that transactions had problems like this... +1. How would you suggest redesigning this "increment a value" operation without needing transactions? – qxz Aug 06 '16 at 00:54
  • Thank you very much for the detailed response - understood. It seems transactions are somewhat broken - in my opinion, a client should not have to worry about all of these considerations. How would you suggest that a bank account balance is modelled without using transactions in firebase? if there is not a robust way to handle this using transaction then I feel this a serious limitation. Another issue is being able to transactionally update some value and atomically update other locations at the same time. – user3391835 Aug 06 '16 at 13:36
  • This was all hard-earned experience, with emphasis on *hard*. Transactions are probably fine for simple increments as long as you keep the above caveats in mind. Another approach would be to push "+N" entries to a log and have a server roll them up periodically with a transaction, which should be more robust and eventually consistent. A client can read the last aggregated value and log to compute the current value on the fly. Deep atomic updates are possible, but cannot be transactional -- again, a log-based design is your friend there. – Piotr Aug 06 '16 at 22:29
  • Thanks a lot for your comments. – user3391835 Aug 20 '16 at 11:21
  • Amusingly, this bug in Firebase is still there 6 years later, and in fact got worse in newer SDK releases. I haven't been able to find a minimal repro, though, so Google won't even acknowledge it... If you turn on low-level logging you can see the right value coming in, then the transaction getting run with the wrong value and bouncing with `datastale`. Then the whole cycle repeats since there's no data updates to be received from the server, but the client's value is still wrong. Sigh. – Piotr Jul 14 '22 at 22:38