Transactions in firebase 3 - ref().update() from within onComplete callback causes [Error: set]

[Alex]

This is a tricky one, after a painful day of debugging I eventually stopped debugging my code and wrote the following test case.

Please consider the following code:

for (var i = 0; i < 10; i++) {

    (function(bidderId){

        firebase.database().ref('counter').transaction(function (currentValue) {

            if (!currentValue) {
                return 1;
            } else {

                if (currentValue >= 5) {
                    return;
                } else {
                    return currentValue + 1;
                }
            }
        }, function(error, committed, snapshot){

            if (error){
                console.error(error);
            }

            if (committed){
                console.log(bidderId + ' successful');
            } else {
                console.log(bidderId + ' not successful');
            }

            var updates = {};
            updates['user/' + bidderId+'/success'] = committed;
            firebase.database().ref().update(updates);

        });

    })(i);
}

When I run this from a Node JS server (v4.4.3) with Firebase JS SDK v3.1.0 I see the following out put on the terminal:

node transaction.js 

5 not successful

[Error: set]

1 not successful

[Error: set]

2 not successful

[Error: set]

3 not successful

[Error: set]

4 not successful

0 successful

[Error: set]

7 not successful

[Error: set]

8 not successful

[Error: set]

9 not successful

6 successful

In the firebase console I inspect the counter and it has a value of 2 which corresponds to the two successful commits that were reported.

Why does the [Error: set] occur here?

I am trying to write a transaction that increments a counter up to a certain maximum value, in this case 5 and then writes to another firebase location to indicate that the attempted increment (whether successful or not) has finished. Imagine I have a bidding system that allows up to 5 bids for a product and needs to indicate to the bidder if their bid constituted one of the 5 allowed. 

Observations:

(1) when I remove the line  firebase.database().ref().update(updates); everything works as expected; the console output is then:

$ node transaction.js 

5 not successful

6 not successful

7 not successful

8 not successful

9 not successful

0 successful

1 successful

2 successful

3 successful

4 successful

... and the counter is correctly incremented to 5. However this does not leave me with any way to indicate success or failure to the bidder.

(2) If I change the following (keeping the call to the .update() function in place):

if (currentValue >= 5) {
    return;
}

to 

if (currentValue >= 5) {
    return currentValue;
}

it works, but I get multiple unnecessary commits as follows:

node transaction.js 

0 successful

1 successful

2 successful

3 successful

4 successful

5 successful

6 successful

7 successful

8 successful

9 successful

The counter is only incremented to value 5, however all bids are reported as successful. 

(3)

If I try to enclose the call to update in a short timeout as follows:

setTimeout(function(){
    var updates = {};
    updates['user/' + bidderId+'/success'] = committed;
    firebase.database().ref().update(updates);
},200)

I still have the same problem. However if I increase the timeout to 2000 then everything works as expected - the counter is incremented to 5 and the  /success firebase locations are updated appropriately.

(4)

If I use the .set() function instead of the .update() function as follows:

firebase.database().ref('user/' + bidderId+'/success').set(committed);

... everything also works as expected. 

I actually need to make several atomic updates here, which is why I am using the .update() method in the first place - so my main questions still stands - why does the .update() function not work in the context of a transactions onComplete() callback, even when it is writing to an entirely different firebase location that that which the transaction itself is pointing to. 

P.S. the full stack for the [Error: set] message above is as follows:

Error: set

    at Error (native)

    at ji (/Users/-------/node_modules/firebase/database-node.js:227:346)

    at /Users/-------/node_modules/firebase/database-node.js:226:104

    at jh (Users/-------/node_modules/firebase/database-node.js:180:196)

    at /Users/-------/node_modules/firebase/database-node.js:180:217

    at /Users/-------/node_modules/firebase/database-node.js:180:148

    at v (/Users/-------/node_modules/firebase/database-node.js:10:370)

    at fh.g.P (/Users/-------/node_modules/firebase/database-node.js:180:116)

    at jh (/Users/-------/node_modules/firebase/database-node.js:180:203)

    at ai (/Users/-------/node_modules/firebase/database-node.js:226:87)

I would very much appreciate your help on this. 

Thanks for reading. 

Alex

[Kato Richardson]

Hi Alex,

This is a pretty long post and I'm probably missing something obvious here. If I understand correctly, the goal is to notify the bidder on whether the bid succeeded, yes? Doesn't that already happen here?

if (committed){
                console.log(bidderId + ' successful');
            } else {
                console.log(bidderId + ' not successful');
            }

☼, Kato

--
You received this message because you are subscribed to the Google Groups "Firebase Google Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firebase-tal...@googlegroups.com.
To post to this group, send email to fireba...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/78a9af9b-c9ce-4c40-88cd-455d6d192ec8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Kato Richardson |

Developer Programs Eng |

kato...@google.com |

775-235-8398

[Alex]

Hi Kato,

Thanks for getting back to me - yes it is quite a long post - I feel that the issue seems complicated so wanted to include as much detail for you guys as possible. 

Well - I am getting Error: set when I run my code as detailed above - also there should always be 5 successful bids however as you can see from the logs this is not the case. 

Please let me know if you need any further details on the specifics here.

Alex. 

[Jacob Wenger]

The "Error: set" in your transaction means you are doing a set() on that same node (or one of its children) while running the transaction. As such, the transaction gets cancelled. You either need to use a transaction in both places or make sure you aren't set()ing while doing a transaction.

Jacob

To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/888878d3-bac4-4773-b822-eee3884fc906%40googlegroups.com.

[Alex]

Hi Jacob, 

Thanks for the info - however this does not really help here as the first code block in my original question is the entire code to reproduce this issue. You can see that I am clearly not doing any set on the same path or any child path and I still get this issue. 

If you try running the code you'll see what I mean. The two paths that are updated from this code are "user" and "counter" - these are obviously entirely separate paths so why should I then get this issue?

Many thanks. 

[Aurélien C]

Hi Alex,

That use case interests me!

Did you try instead of:

updates['user/' + bidderId + '/success'] = committed;
firebase.database().ref().update(updates);

To use:

updates[bidderId + '/success'] = committed;
firebase.database().ref().child('user').update(updates);

I was thinking that even if the update actually impacts only deep nested properties, it is called on the root of the tree, which might impact any transaction occuring at that moment?

--

Aurelien

[Alex]

Thank you very much Aurélien - what you mentioned has made a huge difference - I am currently testing this further and will update this thread with the results. 

Firebase team - is this the expected behaviour? I may have missed it but the docs do not mentioned anything specific about using child vs. full string path?

What is the correct practice for optimal performance with regards to whether you specify the location as a string of breadcrumbs or via .child() - when using transactions and in general?

Thanks again. 

[Michael Lehenbauer]

Hey Alex,

Sorry for the confusion here.  As for your last question, in general, it is best to be as granular as possible (use .child() to access the individual item you want to write).  Firebase is optimized for very granular data access.  On that note, I'd actually recommend you change your code to just:

firebase.database().ref().child('user').child(bidderId).child('success').set(committed);

There's no need for update() at all.  update() is only necessary when you want to set multiple locations at once in a single batch.

As for the problem you ran into:

• That unfortunately confusing error means that you did a set or update that conflicts with an ongoing transaction, and so we canceled the transaction.
• Unfortunately, our logic is a bit too general for update() calls.  We only take into account the ref that you called update() on, rather than the individual paths in your update payload.  So when you called update() on the root of the firebase database, it canceled *all* outstanding transactions.  As Aurélien suggested, moving the update() under the /user location only cancels transactions under /user.  If you take my suggestion and do a set() directly on the /user/<bidderId>/success location, then it'll only cancel transactions on that location.

Hope this helps!  Sorry again for the confusion,

-Michael

To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/46ed7804-23e0-43bb-ad80-3a9441ab2e6d%40googlegroups.com.

[Alex]

Hey Michael, 

Thanks a lot for your reply - I have changed my code and all the Error: set issues have gone away which is great news. 

I have a couple of follow on questions:

1) In the example I originally posted I only had one update to make from the onComplete() handler - in reality I need to make multiple updates at multiple locations ( I trivialised this in the example as it seemed that the crux of the issue could be demonstrated with just a single update). So my question is how would I got about updating multiple different locations atomically in the onComplete() handler of a transaction, whilst ensuring that I do not cancel the transaction itself at the same time? My guess would be that you advise calling set multiple times and not having the atomicity?

Say for example in the update I need to update not only user/bidderId/success but also supplier/bid_id/bid. This really ought to be an atomic update. 

2) Having resolve this issue I am occasionally seeing the following message in the logs:

FIREBASE WARNING: transaction at /queue/tasks/-KM-yWFzfraPuRhXzDTn failed: disconnect 

to give a little background on this, I am using firebase-queue - 7 separate processes are running as queue workers and each of these has numWorkers =10. Is this potentially a related issue and whats the best way to find the cause of this? It does not seem to be causing a problem as such, but it certainly raises an eyebrow. 

Thanks again for your help.

[Benjamin Mueller]

I'd like to chime in here to say that I'm facing a similar situation, and am unsure of the right path. The simplest way to explain it is that any connected user can create a "page" in our system, and so I want to do two things: first, write the actual page to one node, and then increment a "numPages" counter in another node. It's unclear to me how to do both of these writes and have it all be truly atomic. I could do something like:

ref.transaction ( (currentData) => {
    //increment page count here
}, (error, committed, snapshot) => {
    if (!error && committed) {
        //write page here
    }
})

...but in this case, is it possible for concurrent page writes to screw up the count, as the second write increments the counter based on old data? Or wait...maybe this is okay? Maybe it doesn't matter when the actual page data is written, so long as the counter is handled within a transaction? So, if you had two concurrent writes, you could have an actual execution order like this:

first counter updated

second counter updated

first page written

second page written

I suppose that would maintain data integrity, right?

[Alex]

Hi Michael, 

just wondering if you had any thoughts on my previous reply regarding an atomic update from the onComplete() callback  of two locations?

Many thanks

Alex

[Michael Lehenbauer]

Hi Alex,

Sorry for the delay.  Yeah, currently there isn't a great way to do this.  You can either:

1) Lose the atomicity and do multiple sets.

2) Carefully schedule your transactions / updates so that they don't overlap.  (e.g. delay your next transaction until any outstanding update() calls are complete.

One final option is to avoid using transactions entirely.  You can likely enforce the isolation you require via security rules instead so that you can try to do the operation with a normal set, and it'll fail (due to security rules) if it is invalid (e.g. setting the counter to any value other than the current value + 1).

Sorry there isn't a better answer.

Benjamin, can you please start a new thread for your question if you're still looking for help since it's not directly tied to this thread?

Thanks,

-Michael

To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/44d72268-a611-43df-add1-70f32ae6689e%40googlegroups.com.

[Jacob Wenger]

​Note that we just release version 3.4.1​ of the Firebase JavaScript SDK which improves the behavior of transactions. Michael previously pointed out:

Unfortunately, our logic is a bit too general for update() calls.  We only take into account the ref that you called update() on, rather than the individual paths in your update payload.  So when you called update() on the root of the firebase database, it canceled *all* outstanding transactions.  As Aurélien suggested, moving the update() under the /user location only cancels transactions under /user.  If you take my suggestion and do a set() directly on the /user/<bidderId>/success location, then it'll only cancel transactions on that location.

This is no longer the case and our update() behavior has been improved, as noted in the release notes.

Cheers,

Jacob

To unsubscribe from this group and stop receiving emails from it, send an email to firebase-talk+unsubscribe@googlegroups.com.

To post to this group, send email to fireba...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/44d72268-a611-43df-add1-70f32ae6689e%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Firebase Google Group" group.

To unsubscribe from this group and stop receiving emails from it, send an email to firebase-talk+unsubscribe@googlegroups.com.

To post to this group, send email to fireba...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/CAJCyStfNhB%2Bd6cUFkEr%2ByrMPr6is6M%3DhKop_xiFzwTX_0bPy1g%40mail.gmail.com.

[Alexander Mady]

That's great. Thanks for both making the update and letting us know. Much appreciated.

On Thu, 29 Sep 2016 at 01:20, Jacob Wenger <ja...@firebase.com> wrote:

​Note that we just release version 3.4.1​ of the Firebase JavaScript SDK which improves the behavior of transactions. Michael previously pointed out:

Unfortunately, our logic is a bit too general for update() calls.  We only take into account the ref that you called update() on, rather than the individual paths in your update payload.  So when you called update() on the root of the firebase database, it canceled *all* outstanding transactions.  As Aurélien suggested, moving the update() under the /user location only cancels transactions under /user.  If you take my suggestion and do a set() directly on the /user/<bidderId>/success location, then it'll only cancel transactions on that location.

This is no longer the case and our update() behavior has been improved, as noted in the release notes.

Cheers,

Jacob

To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/44d72268-a611-43df-add1-70f32ae6689e%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Firebase Google Group" group.

To unsubscribe from this group and stop receiving emails from it, send an email to firebase-tal...@googlegroups.com.
To post to this group, send email to fireba...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/CAJCyStfNhB%2Bd6cUFkEr%2ByrMPr6is6M%3DhKop_xiFzwTX_0bPy1g%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to a topic in the Google Groups "Firebase Google Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/firebase-talk/KV3c1LRnSJQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to firebase-tal...@googlegroups.com.

To post to this group, send email to fireba...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/firebase-talk/CAGcMwsDxfCc_G_ZoqbDsL2eqxuJxfLMrfYRuCiN7rofKsuesXg%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Kind regards,

Alexander Mady