Skip to content

Add batch transactions #89

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 11 commits into from
Mar 14, 2021
Merged

Add batch transactions #89

merged 11 commits into from
Mar 14, 2021

Conversation

cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented Mar 8, 2021

Support for batch transactions parse-community/parse-server#5849

  • Add transaction for saveAll and deleteAll
  • Add batchLimit argument to all publisher (SwiftUI) saveAll and deleteAll (this was an oversight when adding publishers)
  • Update documentation
  • Add test cases
  • Update changelog
  • Switch default playgrounds to macOS. Add notes to playground files about when to switch
  • Improve playground Pointer example by querying with include and includeAll
  • Only send idempotent on POST and PUT
  • Prepare for new release

@cbaker6 cbaker6 requested a review from TomWFox March 8, 2021 17:19
@cbaker6
Copy link
Contributor Author

cbaker6 commented Mar 8, 2021

@TomWFox can you look through the documentation when you get a chance?

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #89 (b03da01) into main (0ac301b) will increase coverage by 0.28%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #89      +/-   ##
==========================================
+ Coverage   78.75%   79.04%   +0.28%     
==========================================
  Files          63       63              
  Lines        5042     5115      +73     
==========================================
+ Hits         3971     4043      +72     
- Misses       1071     1072       +1     
Impacted Files Coverage Δ
Sources/ParseSwift/API/API.swift 95.95% <ø> (ø)
Sources/ParseSwift/API/BatchUtils.swift 100.00% <ø> (ø)
Sources/ParseSwift/Types/Query.swift 90.71% <ø> (ø)
Sources/ParseSwift/API/API+Commands.swift 82.72% <100.00%> (+0.19%) ⬆️
...ParseSwift/Objects/ParseInstallation+combine.swift 100.00% <100.00%> (ø)
Sources/ParseSwift/Objects/ParseInstallation.swift 82.78% <100.00%> (+0.59%) ⬆️
...urces/ParseSwift/Objects/ParseObject+combine.swift 100.00% <100.00%> (ø)
Sources/ParseSwift/Objects/ParseObject.swift 77.65% <100.00%> (+1.42%) ⬆️
Sources/ParseSwift/Objects/ParseUser+combine.swift 92.98% <100.00%> (+0.67%) ⬆️
Sources/ParseSwift/Objects/ParseUser.swift 76.57% <100.00%> (+0.46%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ac301b...883956a. Read the comment docs.

@cbaker6
Copy link
Contributor Author

cbaker6 commented Mar 8, 2021

@davimacedo @dplewis the way to leverage server level transactions from the client side seemed to be intuitive when I took a look. Where it seemed to get tricky (from your discussions on the server and JS SDK) is when handling unsaved child objects and possibly running into batch limits? Is there anything I missed or should be on the lookout for?

One possible issue I can see coming up is when unsaved child objects are involved, the saving of those children are essentially mini transactions, meaning that if batch1 of children save correctly, but batch2 (say the parent objects) fails, transaction 1 was still committed to the DB. This may confuse the developer because not be thinking in terms of children/parent. This essentially acts "almost" like a regular batch without transactions, but gives you slightly more benefits as the second batch is reverted. Is this correct? If so, I don't see it as much of a problem and can't really think of a workaround on the server level (seems you need to cache some transaction list or something similar). It might be easier to do this on the client side though. Basically, if the client hits an error in batch2, they call deleteAll for batch1. Though, they might need the masterKey if batch1 contained files. This would probably only work for POST and not PUT as it seems easier to say if all saves didn't go through, deleteAll previous. PUT could be a problem on the client side because you may not know what the original object looked like before the transaction began. It also won't work for deleteAll for similar reasons. So we may have to throw out the whole client solution I mentioned above.

Above does make wonder though if you can use the UUID of the idempotent feature that @mtrezza added to revertAll "mini transactions" (what I used above)? I haven't looked into how long the idempotent UUID exists on the DB side, but may help with a solution.

Question about batch limits, I've seen in the iOS and Android SDKs a batch limit of 50, but see the JS mention 100. The JS SDK seems to have a way to change this. Parse-Swift has a way to change it as well. Is there a real sever limit or was that left over from Parse.com? If there is a limit, I assumed this can be configured by the developer, and hinted towards this in the docs.

One of things I did was change the batch limit to whatever the transaction size is:

let batchLimit: Int!
if transaction {
batchLimit = commands.count
} else {
batchLimit = limit != nil ? limit! : ParseConstants.batchLimit
}

I also added this warning in the docs:

- warning: If `transaction = true`, then `batchLimit` will be automatically be set to the amount of the
objects in the transaction. The developer should ensure their respective Parse Servers can handle the limit or else
the transactions can fail.

Let me know what you think and I'm interested in hearing any limitations/problems you see.

Linking parse-community/Parse-SDK-JS#1090 as these PRs overlap in "how to design"

@cbaker6
Copy link
Contributor Author

cbaker6 commented Mar 9, 2021

Noting a comment from @davimacedo in the linked PR:

Hi guys. I am working on this (to include the validations and the tests) but I found out a bug in Parse Server when deleting objects in a transaction. I am working now on a first PR to Parse Server before fixing this one here.

If this bug still exists, I can remove transactions for deleteAll for now if there's not an easy fix

@cbaker6 cbaker6 merged commit 0b0d775 into main Mar 14, 2021
@cbaker6 cbaker6 deleted the transactions branch March 14, 2021 14:04
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant