[Question] How to handle timeout in transaction?
Nyankoo opened this issue · 16 comments
[REQUIRED] Please fill in the following fields:
- Unity editor version: 2020.3.7f1
- Firebase Unity SDK version: 7.2.0
- Source you installed the SDK: Unity Package Manager
- Problematic Firebase Component: Firestore
- Other Firebase Components in use:
- Additional SDKs you are using:
- Platform you are using the Unity editor on:Windows
- Platform you are targeting: iOS, Android
- Scripting Runtime: IL2CPP)
[REQUIRED] Please describe the question here:
How do we handle timeout exceptions in a transaction? Please see this example:
firestoreDB.RunTransactionAsync(transaction =>
{
try
{
return transaction.GetSnapshotAsync(groupRef).ContinueWith(async (snapshotTask) =>
{
await groupRef.SetAsync(new Dictionary<string, object>
{
{ "rank", leaderboardRank }
},
SetOptions.MergeAll).AsUniTask().Timeout(TimeSpan.FromSeconds(6));
DocumentReference participantsRef = firestoreDB.Collection("wlb").Document(groupRef.Id).Collection("participants").Document(AccountID);
await participantsRef.SetAsync(new Dictionary<string, object>
{
{ "score", 10 }
},
SetOptions.MergeAll).AsUniTask().Timeout(TimeSpan.FromSeconds(6));
});
}
catch
{
//What do we do/return here?
}
}
It is more typical to treat transaction as a single unit, and handle timeouts on the transaction itself, then handling timeouts inside a transaction, as your example shows.
RunTransactionAsync
returns a Task
, on which you could specify your timeout handling logic. Internally, transactions are re-tried automatically when possible, so you can probably abort there, because something is wrong, and there is not much the SDK can do (like the App is offline).
@wu-hui Thank you for the reply!
The main issue is that awaiting SetAsync will never finish when the device is offline, so we need to handle the timeout directly at the SetAsync.
Another issue is when the first SetAsync is successful, but the second one fails with a timeout, no roll-back is happening. This results in 50% of the transaction being saved in Firestore while the other 50% isn't saved.
The transaction has an exponential logic, which leads to pretty long timeout, the SetAsync
itself might not finish, but it should be aborted by the transaction timeout, then run again. When all retry fails, the transaction task fails. You can try add some counter or log to observe the retry logic.
And the second issue seems to suggest the transaction failed to satisfy its atomic property, which is pretty bad. Do you observe the partial update after your transaction fails, or is it from inside the transaction?
@wu-hui I'm testing the second issue by throwing a TimeoutException after the first SetAsync. This is the normal behavior when using UniTask's Timeout function.
Thanks..I'll give it a try myself. If this is true, then this is something we need to fix.
Ah, my bad. I just noticed in your code, you are using groupRef.SetAsync
to set documents. To ensure transactional operations, you need to use Transaction.Set(groupRef)
instead. All reads and writes need to register themselves with the running transaction to ensure atomicity.
Calling groupRef.SetAsync
breaks out of the transaction, and does a mutation on its own, which is why they will not be rolled back when the transaction itself is rolled back.
See: https://firebase.google.com/docs/reference/unity/class/firebase/firestore/transaction
Closing this issue because I am confident of the cause, feel free to comment on it if switching to using transaction object for writes does not work.
@wu-hui Doing the following still results in the first Set being saved in the database without any rollback:
return transaction.GetSnapshotAsync(groupRef).ContinueWith(async (snapshotTask) =>
{
transaction.Set(groupRef, new Dictionary<string, object>
{
{ "r", leaderboardRank }
},
SetOptions.MergeAll);
throw new TimeoutException();
transaction.Set(participantsRef, new Dictionary<string, object>
{
{ "s", score }
},
SetOptions.MergeAll);
});
Did I miss something?
Hi @Nyankoo
This is strange, I just tried myself and it works fine ( the mutation is not saved). I would need 2 things to be able to move forward:
- The block you post should be run in a
RunTransaction
call, which returns aTask
. Can you check the returned task runs to success? - Can you confirm if you can reproduce this from all platforms? Does it only happen with IL2CPP platforms or can you reproduce from the Editor directly?
- The task runs successful (IsFaulted and IsCanceled both false)
- This happens in the editor on a Windows 10 machine and on a real Android device
Update: I have a fix but it unfortunately won't make it into the next release of the Firebase Unity SDK. It will, however, make it into the release after that.
Also, there is a relatively easy workaround in the meantime. The bug is in the overload of RunTransactionAsync()
that does not have a generic type parameter. The bug is that it wraps the given callback and calls RunTransactionAsync<T>()
but fails to propagate the error from the given callback.
So the workaround is to adapt your code to call RunTransactionAsync<T>()
and just return some dummy value. This will call the RunTransactionAsync<T>()
method, which does not incorrectly wrap the given callback.
For example, your code above could be changed to the following:
return transaction.GetSnapshotAsync(groupRef).ContinueWith<object>(async (snapshotTask) =>
{
transaction.Set(groupRef, new Dictionary<string, object>
{
{ "r", leaderboardRank }
},
SetOptions.MergeAll);
throw new TimeoutException();
transaction.Set(participantsRef, new Dictionary<string, object>
{
{ "s", score }
},
SetOptions.MergeAll);
return null;
});
And just make sure to explicitly call RunTransactionAsync<object>()
instead of RunTransactionAsync()
.
I will update this issue once my fix is submitted and when it gets released.
Update: The fix has been submitted and will be included in the next release of the Firebase Unity SDK. I can't promise a concrete release date, but I'd expect some time in the next 2-3 weeks. In the meantime, I recommend to continue using the workaround I provided in the previous comment. I'll close this ticket for now since there are no further actions from our end. Thank you for reporting this and working together on the investigation.
Note to Googlers: The fix is cl/375130076 (release notes: cl/375712637). I also created b/189214152 to ensure that we improve the test coverage of RunTransactionAsync()
to avoid problems like this in the future.
Update: The fix was released with the Unity SDK 8.0.0 on June 17, 2021.
https://firebase.google.com/support/release-notes/unity#version_800_-_june_17_2021