floor icon indicating copy to clipboard operation
floor copied to clipboard

handle changeListener inputs in transactions correctly

Open mqus opened this issue 4 years ago • 2 comments

This is something that would help us in handling the changes made by transactions and lessen the amount of updates that take place.

Currently, database changes made in @transaction functions immediately notify the changeListener and start to update the table in streams. This has the following problems:

  • The tables are updated, even if the transaction is not finished yet (and therefore no changes have been persisted yet)
  • If a transaction e.g. deletes some entries in a table and immediately inserts them again, two notifications are put inside the changeListener, adding unnecessary refresh cycles.
  • When a stream (e.g. on joins) has to listen on multiple entity names at once, even more unnecessary refreshes happen.
  • If the transaction fails, the streams will get updated even if nothing happened.

Because of the reasons above, I propose to change the type of the changeListener from Stream<String> to Stream<Set<String>> and put a proxy changeListener into the transaction generation (Line 32, see below), which buffers all changes and then submits them together once the transaction was completed successfully. This adds some overhead into transactions (for the proxy) and some smaller overhead for streams, which now have calculate an intersection, instead of a simple contains calculation for filtering relevant updates.

I want to hear your input first, @vitusortner and I think that I probably won't start to work on this soon. (I'm currently having more fun with the sqlparser :wink: )

https://github.com/vitusortner/floor/blob/18117ed397b5787905576aedad90e0f347931792/floor_generator/lib/writer/transaction_method_writer.dart#L27-L36

A small note: I was surprised to see that adding this to transactions is far easier than I imagined.

mqus avatar Jun 14 '20 19:06 mqus

Ah right, one more thing:

  • Is there a reason why the transaction currently only returns void? Afaict it should be able to return anything (wrapped inside a Future, obviously).

mqus avatar Jun 14 '20 19:06 mqus

Ah right, one more thing:

* Is there a reason why the transaction currently only returns void? Afaict it should be able to return anything (wrapped inside a Future, obviously).

was adressed in #381

mqus avatar Apr 18 '21 17:04 mqus