-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Posh with Datomic #17
base: master
Are you sure you want to change the base?
Conversation
Ok I'll take a look at it on Monday, though I'm not very experienced with Datomic. Also, I was wondering, why did you have to redo the Reagent ratom and reaction stuff for the backend? Posh core already functionally calculates changed q's and pull's, and it looks like you're using that: (https://github.com/mpdairy/posh/blob/master/src/posh/plugin_base.cljc#L41) |
So plugin_base.cljc is simply a platform-agnostic version of what was formerly reagent.cljs — I didn't write any of it (the diff is misleading and should really just say I renamed the file and added a few things here and there). I just added |
So how are you planning to use the reactions on the server side? Is this for front-end stuff in clj or is it for communicating changes to the front-end? if it's for the latter, wouldn't you want to send the datoms that caused the reaction rather than the result of the pull or q (which is returned by the reaction). |
The latter, and yes, returning the datoms (i.e. only the diff) would be nice (really, only the datoms affected by the registered query/pull expressions — not all the datoms from the transaction report). However, what I'd optimally like to do is to have the various clients register (and deregister) their query/pull expressions, have posh figure out who needs to get notified with what datoms, and send those to the appropriate clients. I suppose I haven't fully thought through yet how to do that. I haven't dug through posh's internals (planning to soon), but I assume there's quite a bit of logic in there to figure out, given some new datoms from a transaction report, which reactive expressions need to be notified. Maybe we can leverage that? Again, I haven't fully thought this through yet. Thoughts from your side? Oh, and also, as for uses of reactive expressions — frontend stuff in clj would potentially be useful, yes, but another thing I'm envisioning with reactive queries is in situations in which the frontend isn't even involved. This might be something like incremental query recalculation for an expensive query (say, in an analytics context where no frontend is involved). In a simpler situation, it could also allow non-incremental query recalculation with efficient change notifications (i.e. no 'query polling'). There are likely other situations in which reactive queries would be useful. |
Well you'll be happy to that I had a similar vision for the posh backend regarding sending only the necessary datoms and I added some core functionality to make that possible. It's a complicated problem because often times you have to send more than just the tx'd datom; for example, a So, although I don't remember all the details, what I imagined you'd do on the backend is set up a filtered db for each user (for security restrictions) and then start a query graph from posh core for each user, The query graph can be synced between client and server because you can get it as plain data (through core), but you'd have to do something to make sure the entity ids are in sync. I think @metasoarous said he was getting something like this working with Datsync ("optimized updates", I think he said), but I'm not sure. |
Any updates on this? I know you guys are going for the more ambitious "send all relevant datoms to the client", but I'd just be happy with the forwarding the answer to the query. Is that already working? |
Howdy; @alexandergunnarson Awesome work! So far, keeping entity ids in sync is one of the primary concerns of Datsync. And I've recently taken a new approach on this for true sync (local first transactions, synced based on the tx-log), with lookup refs instead of manual tracking of entity ids. I made this switch so that P2P sync wouldn't be forcluded in the design. As for the filters and such (optimized updates; whatevs); Yes, I've been in hammock mode trying to think things through carefully this go around, and think I'm on the verge of settling some of these issues. One of my big realizations of late is that I've decided to leverage Onyx for describing the flow of data on clients and server, now that we have the local runtime. This has some nice consequences:
I wish I was further along on this than I am, but times have been busy. I've been working on a blog post to try and get the hammock portion of this work all sketched out for feedback. After that I hope to start building out some of the basic building blocks. @alexandergunnarson Your work here will be a major help! Feel free to chat me up more about the bits I've been working on over Datsync Gitter chat or GH issues. @seantempesta I'd like to have direct syncing of query results (without transacting into a db) as an option for Datsync. |
Okay, so I don't really understand the Posh codebase, but when I run the test this is the tx-report I'm seeing:
It looks like the attribute |
Looking up the attribute via /posh/clj/datomic.clj -(defn datom->seq [^datomic.Datom d]
- [(.e d) (.a d) (.v d) (.tx d) (.added d)])
+(defn datom->seq [db-after ^datomic.Datom d]
+ [(.e d) (d/ident db-after (.a d)) (.v d) (.tx d) (.added d)]) - (try (let [txn-report' (update txn-report :tx-data
- #(map datom->seq %))]
+ (try (let [db-after (:db-after txn-report)
+ txn-report' (update txn-report :tx-data
+ (fn [datoms]
+ (map #(datom->seq db-after %) datoms)))] ;; Execute each of these statements one at a time and the test works.
;; Looks like there's just a race condition between when the thread
;; see the change and updates the ratom
(d/create-database "datomic:mem://test")
(def conn* (d/connect "datomic:mem://test"))
(def poshed (db/posh! conn*))
(def conn (-> poshed :conns :conn0))
(db/start conn)
(instance? PoshableConnection conn)
(db/transact! conn (install-partition default-partition))
(transact-schemas! conn
[{:db/ident :test/attr
:db/valueType :db.type/string
:db/cardinality :db.cardinality/one}])
(def sub (db/q '[:find ?e
:where [?e :test/attr]]
conn))
(= @sub #{})
(db/transact! conn
[{:db/id (tempid)
:test/attr "Abcde"}])
(= @sub
@(db/q [:find '?e
:where ['?e :test/attr]]
conn)
(d/q [:find '?e
:where ['?e :test/attr]]
(db/db* conn)))
(db/stop conn) |
Super helpful @seantempesta ! Great job finding the issue. I'll add that commit in today. |
Thanks! I'm planning on using this feature so let me know if there's other stuff you need help on. |
Um, so how do we detect changes to the ratom subscription? I'm used to having reagent do this automagically as long as I deref'ed it in the render function. |
@seantempesta It's possible to detect changes if you deref within a reaction, or (anywhere, including outside of a reaction) if you use |
@seantempesta Very astute point about the race condition. I was wondering whether the single-threadedness of the ratoms might pose an issue and it looks like indeed it does. I'll make a thread-safe version later today (no I guess the ratoms are being modified by the |
So I've tried both
Now I'm changing the underlying data
At this point, neither the
I think somewhere in the datomic Posh code we need to deref the underlying ratom/reaction to kick off any registered listeners. No idea where to look though. |
That makes sense that the watches wouldn't be called until the reactions are derefed, at least in the case of a reaction. For a reaction, the cases where the watches are called are on
This could be true, but if it were true for CLJ Datomic, it should be true for CLJ DataScript, right (which it's not)? In the |
Wait, so for your use case, you want to get notified every time something changes in the reactive query, right? Why don't you do something like this? : (def notified-times (atom 0))
(def query (db/q ...))
(run! @query
(swap! notified-times inc))
(transact! ...)
(is (= @notified-times 2)) The reaction will re-run every time the query data changes. Alternatively, if you want to have the notification run only on change (no initial run), use the following: (make-reaction
(fn [] @query
(swap! notified-times inc))
:auto-run true)
(transact! ...)
(is (= @notified-times 1))
(transact! ...)
(is (= @notified-times 2)) UPDATE : it works in DataScript but not Datomic, probably due to that race condition you were talking about. |
Yeah. I want to have the client send the queries they want to execute the server, execute them on the server and send the changes down to the client whenever the results change. I just wrote this test for ratoms: (deftest ratom-watch-test
(try (let [ratom (r/atom nil)
_ (is (= @ratom nil))
add-watch (add-watch ratom :watcher (fn [key atom old-state new-state]
(throw (Exception. "change-detected"))))
testing-watch (reset! ratom "test")
_ (is false)]) ;; should not get to this point as the exception should be thrown
(catch Exception e
(println "Exception caught. Test successful")))) And it works fine. Isn't Posh using an |
…sy possibility of changing back. Perf test
…osh.datomic/transact!` finishes
I haven't looked extensively at all the internals of posh. But it seems that yes.
This ties into the first question. If Posh doesn't do too much additional magic and acts just like a normal reaction, then the query will not be run on every deref. It will only recalculate when needed. |
@mpdairy @seantempesta @metasoarous : The Datomic tests (minimal as of yet, but even so) officially work! Using what @seantempesta said as a very helpful starting place, I was able to figure out that the Posh listeners needed to be run/notified before Another thing that you might notice is that the Clojure implementation of reactions/ratoms is now thread-safe (using clojure.core/atoms). It's theoretically possible that I'll look over all your comments (shout out to @metasoarous , who I haven't yet responded to!) and such to get a feel for next steps and I'll tackle those this week. |
Very cool! Great work @alexandergunnarson. I'm still not able to get my query subscriptions to trigger on transact! (still trying via (deftest query-watch-test
(with-conn conn*
(let [poshed (db/posh! conn*) ; This performs a `with-meta` so the result is needed
conn (-> poshed :conns :conn0) ; Has the necessary meta ; TODO simplify this
_ (is (instance? PoshableConnection conn))]
(try (let [txn-report (db/transact! conn (install-partition default-partition))
txn-report (transact-schemas! conn
[{:db/ident :test/attr
:db/valueType :db.type/string
:db/cardinality :db.cardinality/one}])
sub (db/q [:find '?e
:where ['?e :test/attr]]
conn)
_ (is (= @sub #{}))
add-watch (add-watch sub :watcher (fn [key atom old-state new-state]
(throw (Exception. "change-detected"))))
txn-report (db/transact! conn
[{:db/id (tempid)
:test/attr "Abcde"}])
_ (is (= "db/transact! should trigger the watch which should throw an exception" false))])
(catch Exception e
(println "Exception caught. Test successful"))
(finally (db/stop conn)))))) ; TODO `unposh!` |
Thanks @seantempesta ! So I just created a function called |
Hey @alexandergunnarson. The eager stuff isn't working for me. In fact, if you split out just that part of the test it fails. (defn just-eager-test [conn dcfg]
(let [sub-no-deref ((:q dcfg) [:find '?e :where ['?e :test/attr]] conn)
notified-no-deref (atom 0)
_ (r/add-eager-watch sub-no-deref :k-no-deref (fn [_ _ _ _] (swap! notified-no-deref inc)))]
(testing "Eager watch test"
((:transact! dcfg) conn
[{:db/id ((:tempid dcfg))
:test/attr "Abcde"}])
(is (= @notified-no-deref 1)))))
I'm not sure why it isn't working, but it's also matching my real world use. Until I deref the reaction I'm not getting any watches to run. |
Well that's upsetting... I suppose the This:
prints |
Yeah, I figured there was some interplay between the code. Are we missing something obvious here? When I used reagent ratoms in frontend code I always had to deref them at least once (I assumed that somewhere this registered a watch). Is the |
Hey Alex. Any updates on this? I've hacked together a manual pattern matching solution and while it works it's pretty ugly. |
Hey @seantempesta ! Sorry, just got through a long semester and I'm just getting back to working on this. I should push some updates over the next few weeks. |
Sweet. No worries. I appreciate everything you've done. The manual pattern matching solution I've got is actually working pretty well, so there's no rush. |
UPDATE: Posh transactions and reactive queries work with both DataScript and Datomic! This can be merged if you'd like.