fulcrologic / fulcro

A library for development of single-page full-stack web applications in clj/cljs
http://fulcro.fulcrologic.com
MIT License
1.55k stars 140 forks source link

Architecture Improvement: Priority Queue as Central Processing Mechanism #305

Closed awkay closed 5 years ago

awkay commented 5 years ago

One of the big weaknesses of the internal architecture is the fact that transactions, which cause all forward movement of the library, are processed "immediately" on the calling thread and have no facility for dealing with nesting (generally undesirable, but sometimes structurally necessary), timing, and most importantly: data dependencies.

The transact! mechanism, as it exists, has always had an ordering semantic for a single tx expression, but only for the optimistic updates and operations that happen on the same remote (a simple queue). This has been expanded over time to try to address this in a somewhat ad-hoc manner: ptransact! and incubators more useful variant of the same-named function allow for a slightly expanded form of runtime order dependency: that of running something after the remote behavior. This latter expansion essentially provides what vanillajs accomplishes with promises and then.

Some of the weaknesses of all of the above systems are:

I propose extending the internal architecture of Fulcro to support a much richer form of ordering and dependency management for operations. Specifically I plan to place a form of priority queue in place between the running of transact! and the processing of those items.

This would also affect loads, and would allow post mutations to become first-class mutations that are queued into this same priority queue. The current existence of load-action might be affected by this change, though, since the current implementation of loads can bypass the direct use of transact by parasitically "morphing" the remote of the mutation in which they are used. This has always been a hack around the original inherited architecture of Om Next. We will technically still need to support this use-case (which is just rewriting the remote AST), since it is widely adopted and used.

awkay commented 5 years ago

Some Design Aspects to Refine:

claudiuapetrei commented 5 years ago

Hi,

Reading the https://github.com/fulcrologic/fulcro/blob/feature/react-play/docs/RFCs/RFC-transaction-semantics.adoc

Sounds really great :smile:

A few thoughts, don't know if there any good, hard to figure out all the use-cases and implications:

awkay commented 5 years ago

Thanks for the feedback @claudiu-apetrei .

claudiu-apetrei commented 5 years ago

Hi,

awkay commented 5 years ago

So, I don't think you quite understand the group leader stuff...perhaps re-read the RFC? Your example is waaay too syntactic (e.g. extending the language, not the semantics). I really don't want to go that route.

On "abort": You just store that flag in your state, and any of the following mutations can look for it and become a noop...that is the intended place for flow control: in the mutations themselves.

On refresh: what does it matter if it does a refresh when there is nothing affected on-screen? That is more an issue of optimization, and doesn't belong in the semantics. The other :after options on the semantics, I think, can possibly solve any real cases, and UI display is another one where what you put in state is under your control, and what is in state is what renders.

On the optimization side: I do plan on letting you specify the scheulding of transaction queue processing, where you could indicate they are done on RAF, to prevent more than 60fps processing.

claudiu-apetrei commented 5 years ago

@awkay Thank you for answering my questions smile On the abort part. I sometimes use a marker in state but feels like I'm coupling things and most of the time I go with having a mutation implementation mutation-name* and just compose those in a single mutation. It's pretty ok, but just feels like there's something missing.

I get why adding flow control might me a bad idea and source of complexity. Would you consider passing the return of mutations to the next one in the new implementation ? Ex: (transact! this `[(a) (b) (c {:x 1})]) the return value from a would be available in b and the return value from b be available in c. For ptransact if there is no remote action fired it will pass the optimistic return value otherwise the ok or error return value.

awkay commented 5 years ago

Hi, I got on a bit of a ramble here. The short answer to your question is "you can already do that via app state", the slightly longer answer is "nesting mutations might solve your problem", and then I go off on some related thoughts and rambling... :)

It really would not be hard to put an atom in env that can be used to track the progress of a tx, but it would have to be an atom (the env is actually closed over when the mutation is dispatched, not every time a section runs). I'm not sure this makes much more sense than just leveraging what you already have (the state atom) as a means of communication.

That said, I am introducing (with queues) a formalism that allows for a transact to safely appear within an action. This alone is huge change, as it allows the decision logic of a mutation in the top-level transaction to submit additional top-level transactions (to the end of the queue, of course). This is a form of application-centric flow control in a way, since logic in an action can choose additional transactions to submit that are really not visible from the original. I've resisted this for a long time, because I see it as a way for chains of complicated logic to get out of control at the mutation layer. That said, I've come to the conclusion that it is not necessary or desirable to constrict where a new top-level transaction can be submitted, as long as you control how that is interpreted.

So, for your return value question: I think there are two answers in Fulcro 3:

;; somewhere in UI
(transact! x `[(f)]`)

(defmutation f [params]
  (action [env] 
    ;; Fulcro 3 only, or sort-of ok as `ptransact!` in 2.
    (transact! env `[(g {:data-from-f 42})]`)))

In terms of "feels like something is missing". I get that for Fulcro 2. It's been nagging at me for years as well, and I have seen it as a nesting problem. What is the best way to deal with the idea that there is logic "within" a transaction that needs to be expressed?

In the past the answer has been "write mutation helpers", but then we found cases where only ptransact! would work. With even more complicated scenarios (composition of dynamic routers with state machines that allows their own form of nesting and cross-communication) has it finally become more obvious that the real central problem is that transact! itself "runs when called" instead of using a queue. This means that the actions inside of that call should not call transact! because that has the effect of "interrupting" the top-level transaction and processing a nested one within it. Nested transactions are just a big ball of complexity. The top-level reasoning just gets destroyed. It is my hope that moving this to a queue will give us a more solid model (that isn't based on the opaque js setTimeout event queue). Reasoning about a single tx (no matter from where it is run) is now able to be "locally atomic" in the sense that (at least for local optimistic effects) an optimistic transaction will run as a single serialized unit at some future time that will not mix with or interfere with those that were submitted before it. (like the SERIALIZABLE isolation level of SQL). At the end of the day this is really what we mean when we talk about "transactions": a group of operations that runs together. We can further ensure that the remote effects of a transaction preserve their serial ordering and grouping (per remote).

Note that the setTimeout solution (which is what ptransact! and others do to avoid this problem) was already essentially a queue solution: js does in fact run these in the "order of submission". The proposed solution makes this "our queue", and therefore amenable to customization, analysis, and finer control.

My primary goal for this expansion is to clean up the API and make the use of transact! make more sense throughout the entire program. I'm almost certainly not going to implement the :after clauses (they are too complex and mostly not necessary), but I am giving transactions a "findable" ID, allowing for some kinds of transaction middleware and algorithm customization, and making the API usable in a consistent manner everywhere. This is also leading to a natural way to further minimize network traffic through "grouping", which isn't going to require the option described in the RFC...I've found a better way to do it I think.

awkay commented 5 years ago

The transaction processing has been rewritten in F3, and contains many of the ideas in this issue. It is also somewhat pluggable so that it can be tuned more easily over time.