Clojurians Log v2

I tried to replicate the issue in a small setup, but the results looks as expected:

(pc/defresolver inner-call [{:keys [parser] :as env} _]
  {::pc/output [:inner-result]}
  {:inner-result (parser env [{[:foo 1] [:foo]}])})

;; define a list will our resolvers
(def my-resolvers [inner-call])

;; setup for a given connect system
(def parser
  (p/parser
    {::p/env     {::p/reader               [p/map-reader
                                            pc/reader2
                                            pc/open-ident-reader
                                            p/env-placeholder-reader]
                  ::p/placeholder-prefixes #{">"}}
     ::p/mutate  pc/mutate
     ::p/plugins [(pc/connect-plugin {::pc/register my-resolvers})
                  p/error-handler-plugin
                  p/request-cache-plugin
                  p/trace-plugin]}))

(comment
  (parser {} [:inner-result]))

thanks! it normally works as expected. it’s just when i have many resolvers nested that I get this strange behavior

if I can’t trace it down, i’ll try to come up with a reproducible example

Looks like I was able to reproduce the bug in a smaller project. I think it has to do with parallel-parser or parallel-reader b/c when I switch to using the regular reader / parser the bug goes away. Here’s the example project I setup. https://github.com/dehli/pathom2-bug/blob/main/src/dev/dehli/pathom_bug.cljs

yeah, looks off, started an issue for it https://github.com/wilkerlucio/pathom/issues/198

ok, I did some investigation, and I think I got it

the problem is that in the parallel case the entity gets modified upon each request

Awesome! That’s great news 🙂 Thanks!

just replied on the issue with details, please let me know if the solution proposed there works for you

Thanks! I think that would work. I was initially using placeholders, and the bug only started to surface as I moved to idents. Is this intended behavior for the parallel-parser?

using parser inside parser has some of this repercusions, its kinda expected

if you are using it too much, would be nice to see why, ,and if it could be avoied

but there cases (specially on pathom 2) that its just the only way (a hack way for optional and/or nested inputs)

cool! we don’t use them too often (almost always it’s due to nested inputs). pathom3 will fix that, so we are excited to make the switch 🙂

Hello again! I’m still seeing similar behavior (even when using the (with-entity) function you had suggested. I’ve also tried clearing out ::p/request-cache and I’m still getting a subset of my query returned in my nested parser call. Is there some other key in env that I could look at to debug? If not, I can try to find another reproducible example.

I’ve got another example showing the bug I’m seeing. For this one, I am using the with-entity function. I’ll keep playing around with it to try to better figure out what’s going on.

(ns dev.dehli.pathom-bug
  (:require [clojure.core.async :refer [go <! <!!]]
            [com.wsscode.pathom.core :as p]
            [com.wsscode.pathom.connect :as pc]
            [com.wsscode.pathom.parser :as pp]))

(defn with-entity [env ent]
  (assoc env ::p/entity (atom ent)))

(pc/defresolver one
  [{id :id}]
  {:one (str "one " id)})

(pc/defresolver state
  [{:keys [parser] :as env} {id :id}]
  {::pc/output [:state]}
  (go
    {:state (<! (parser (with-entity env {:id id}) [:one]))}))

(def parser
  (p/parallel-parser
    {::p/env     {::p/reader               [p/map-reader
                                            pc/parallel-reader
                                            pc/open-ident-reader
                                            p/env-placeholder-reader]
                  ::p/placeholder-prefixes #{">"}
                  ::pp/external-wait-ignore-timeout 100}
     ::p/plugins [(pc/connect-plugin {::pc/register [state
                                                     one]})
                  p/error-handler-plugin
                  p/trace-plugin]}))

(comment
  (<!! (parser {} [{[:id "foo"] [:one :state]}]))

  ;; Returns
  ;; {[:id "foo"] {:one "one foo" :state {}}}
  )

It seems to be returning empty state b/c it’s reaching the ::pp/external-wait-ignore-timeout

I think I was able to fix the bug by adding ::pp/processing-recheck-timer

although it does seem like a bandaid rather than a full solution

yeah, I'm taking a deeper look, it doesn't look very good

the issue is that the parallel processor has all these "tricks" add for performance tuning over time

and these examples are exposing their sensitivity for recursive calls

I see. That lines up with what I’m seeing on my end as well. Our recently added resolvers utilize recursion which must be why we’re starting to see some unexpected behavior.

did you recently changed to parallel parser? or recently add recursive calls like this?

we’ve been using parallel parser but recently added recursive calls like this

and the reason you use the parallel parser is related to long attribute dependencies? or because of processing many entities in parallel? ou maybe both?

we’re using the parallel parser for the second reason (needing to execute many entities in parallel)

i guess the first as well

we saw pretty good performance improvements when we switched to parallel

gotcha, we can try to hack a bit on the parallel and "reset" the things that need reset on the recursive call, we may be able to make this work for now

ya, i was looking through the env map to try and figure out what could be reset to give it a “fresh” view

additionally the bandaid might be enough as well until we make the switch to pathom3. it seems that the parallel parser can be implemented much simpler with the new architecture

yes, parallel processing in pathom 3 should be much simpler

I'm thinking of doing something in pathom 2 that I did for pathom 3 regarding env

there I just save a "backup" version of the env on entry, so you can restore to it

but may not be enough, because of some mutable state things, like active-paths

ok, I find something that works with your example:

(defn with-entity [env ent]
  (assoc env ::p/entity (atom ent)))

(pc/defresolver one
  [{id :id}]
  {:one (str "one " id)})

(defn reset-env [env]
  (assoc env
    :com.wsscode.pathom.parser/active-paths (atom #{})
    :com.wsscode.pathom.parser/waiting #{}
    :com.wsscode.pathom.parser/key-watchers (atom {})
    :com.wsscode.pathom.core/entity-path-cache (atom {})))

(pc/defresolver state
  [{:keys [parser] :as env} {id :id}]
  {::pc/output [:state]}
  (go
    {:state (<! (parser (-> env
                            (reset-env)
                            (with-entity {:id id})) [:one]))}))

(def parser
  (p/parallel-parser
    {::p/env     {::p/reader                        [p/map-reader
                                                     pc/parallel-reader
                                                     pc/open-ident-reader
                                                     p/env-placeholder-reader]
                  ::p/placeholder-prefixes          #{">"}
                  ::pp/external-wait-ignore-timeout 100}
     ::p/plugins [(pc/connect-plugin {::pc/register [state
                                                     one]})
                  p/error-handler-plugin
                  p/trace-plugin]}))

I can make reset-env part of the Pathom API, so we can move it with the library (and if we find more things to reset, an update will fix)

awesome! ya, i’ll give that a go. I have a quick question about the processing-recheck-timer how does it know it’s “stuck”? does that just mean the resolver is still executing? Trying to determine what’s a good value so that I don’t retry database calls, etc.

Im reading again on the code to remember what thats about

for reasons I don't remember now, the parser may get stuck in some part of the process, this is probably due to the channel madness going on in the orchestration there, looking at the code, its a kinda of "timeout" case, the parallel parser will walk all children of the AST, and try to make sure every attribute is "started" (not every single attribute will trigger a process, for each process pathom knows all "expected" attributes, and will skip those during the scan in case a previous attribute ended up hitting it in the process)

so after start everything, it waits for some response, to merge as they come

but if it is stuck, that will not finish, this is where the process-recheck-timer enters, it its like a timeout case for this step, and if the code reaches it, it will re-trigger the process for the attributes that still pending

makes sense?

ya, that makes sense! thanks so much! i’ve also gotten your reset-env function added to our subquery function so i think that will fix the issues we’ve run into. will test it out and let you know!

a little polishing on the api:

(defn with-entity [env ent]
  (assoc env ::p/entity (atom ent)))

(pc/defresolver one
  [{id :id}]
  {:one (str "one " id)})

(defn reset-env [env]
  (assoc env
    :com.wsscode.pathom.parser/active-paths (atom #{})
    :com.wsscode.pathom.parser/waiting #{}
    :com.wsscode.pathom.parser/key-watchers (atom {})
    :com.wsscode.pathom.core/entity-path-cache (atom {})))

(defn call-parser [{:keys [parser] :as env} entity tx]
  (parser
    (-> (reset-env env) (with-entity entity))
    tx))

(pc/defresolver state
  [env {id :id}]
  {::pc/output [:state]}
  (go
    {:state (<! (call-parser env {:id id} [:one]))}))

Thanks again for all your help! Have deployed with the changes and the unexpected behavior is now gone 🙂

great to hear! 🙂 later I'll put the reset-env at the library, so you can use from it