so just (.close borrowed-client) and nothing else?

interestingly it all works if I just get rid of the while true

;; ----------------------------------------------------------------------------
(defn create-pub-sub-listener
  "Creates a listener for pub-sub messages. Borrows a connection
  from the pool and doesn't return it until it is shut down."
  [pool]
  (let [client ^Jedis (.getResource pool)
        logic (proxy [JedisPubSub] []
                (onMessage [channel message]
                  (log/info :channel channel :message message)))
        executor ^ExecutorService (Executors/newSingleThreadExecutor)]
    (.submit
      executor
      (reify Runnable
        (run [_]
          (.subscribe client
                      logic
                      ^{:tag "[Ljava.lang.String;"}
                      (into-array String [post-notification-channel])))))
    {:borrowed-client client
     :pub-sub logic
     :executor executor}))

;; ----------------------------------------------------------------------------
(defn shutdown-pub-sub-listener!
  [pub-sub-listener]
  (let [{:keys [borrowed-client pub-sub executor]} pub-sub-listener]
    (.shutdownNow executor)
    (.unsubscribe pub-sub (into-array String [post-notification-channel]))
    (.close borrowed-client)))

but this means that any tasks I do based on messages better not fail

since it would stop consumption of messages

yeah, because the try/catch is inside the while it just creates the subscription again after you unsubscribe

and I would have no way to know

but I don't get a log of the exception when I unsubscribe

you can instead of (while true ), use an atom

so it doesn't seem like anything is thrown

it is likely a InterruptedException or wrapped in one

(while true
            (try
              (.subscribe client
                          logic
                          ^{:tag "[Ljava.lang.String;"}
                          (into-array String [post-notification-channel]))
              (catch InterruptedException e
                (throw e))
              (catch Exception e
                (log/error ::jedis-pub-sub-error e
                           ::action-to-take "retry subscription")))))))

i wouldn't think so

since in both cases there should be some observable side effect

it may not throw any exception

so (while @whatever ...

and (reset! whatever false) on shutdown

yeah but subscribe is already an infinite loop within itself

the unsubscribe will stop it

and you will stop the while

and even adding a log to the interupttedexception case doesn't show anything

add a log after the subscribe call

or in a finally

what is happening is the subscribe call isn't throwing

just returning

and you loop around

ahhhh that makes sense

yep thats whats happening

yep that did the trick

;; ----------------------------------------------------------------------------
(defn create-pub-sub-listener
  "Creates a listener for pub-sub messages. Borrows a connection
  from the pool and doesn't return it until it is shut down."
  [pool]
  (let [client ^Jedis (.getResource pool)
        logic (proxy [JedisPubSub] []
                (onMessage [channel message]
                  (log/info :channel channel :message message)))
        executor ^ExecutorService (Executors/newSingleThreadExecutor)]
    (.submit
      executor
      (reify Runnable
        (run [_]
          (let [completed (atom false)]
            (while (not @completed)
              (try
                (.subscribe client
                            logic
                            ^{:tag "[Ljava.lang.String;"}
                            (into-array String [post-notification-channel]))
                (catch Exception e
                  (log/error ::jedis-pub-sub-error e
                             ::action-to-take "retry subscription"))
                (finally
                  (reset! completed true))))))))
    {:borrowed-client client
     :pub-sub logic
     :executor executor}))

still open question marks on the multi threaded behavior of calling unsubscribe

but this should only happen in dev so it won't matter that much

...hopefully

The finally there means the while loop will never loop

Also if you want the while there you likely need to move the get resource call inside the while, otherwise if the connection dies you'll never get a new one

;; ----------------------------------------------------------------------------
(defn create-pub-sub-listener
  "Creates a listener for pub-sub messages. Borrows a connection
  from the pool and doesn't return it until it is shut down."
  [pool]
  (let [client ^Jedis (.getResource pool)
        logic (proxy [JedisPubSub] []
                (onMessage [channel message]
                  (log/info :channel channel :message message)))
        executor ^ExecutorService (Executors/newSingleThreadExecutor)]
    (.submit
      executor
      (reify Runnable
        (run [_]
          (let [completed (atom false)]
            (while (not @completed)
              (try
                (do (.subscribe client
                                logic
                                ^{:tag "[Ljava.lang.String;"}
                                (into-array String [post-notification-channel]))
                    (reset! completed true))
                (catch Exception e
                  (log/error ::jedis-pub-sub-error e
                             ::action-to-take "retry subscription"))))))))
    {:borrowed-client client
     :pub-sub logic
     :executor executor}))

yep so if it exits normally, thats another thread calling unsubscribe

otherwise its an exception

You still have the issue of that while loop continuing to try on a dead connection

yeah, but there are only like 8 connections in the pool

if i keep getting new ones i will mess up pushing other stuff to redis

the real issue is visibility

like, if i could know that i messed up handling and shut down and restart this component

or if i had it in my dashboard (i'm using heroku for everything right now) then I could manually handle it

idk - i just don't want to do this in a "bad" way

maybe for this component i shouldn't touch the pool at all

and just give it its own Jedis it recreates on failure

The pool isn't limited to the same N connections

I believe when you return a connection to it, it tests it and if it is bad it throws it away and adds a new one to the pool

stateful things are always nightmares

the structure of this in our code at work looks something like this using core.async:

(async/go-loop []
  (async/alt!
    exit ([_] ...)
    (get-connection redis-pool) ([connection]
                                 (async/alt!
                                   (async/thread
                                     (try
                                       (.subscribe connection ...)
                                       (finally
                                         (close-connection connection)))) ([_] (recur))
                                   exit ([_]
                                         (.unsubscribe connection ...)
                                         (close-connection connection)
                                         ....)))))

Are there any libraries (e.g. on top of rewrite-clj) that can merge EDN files and write back to a file, while preserving order / whitespace / comments etc? Maybe I'll have to write my own, but just asking in case there's already one

The use case: I want to write a package manager, but both the user and the package manager should be able to put stuff in an .edn file

Kind of like npm does this with package.json

I guess npm has it easier since comments aren't allowed in JSON, but still, it seems to preserve some of the user-defined order in maps

order is relatively easy to preserve, but only if you tweaked the reading code to use something like the ordered library https://github.com/clj-commons/ordered as long as you were then also careful about what operations you did on those maps so the ordering wasn't lost (e.g. select-keys I think always returns a built-in Clojure unordered map, so you would need to avoid using that, and/or write your own variant that returned the type of map it was given. The same goes for many other core functions on maps).

preserving white space and/or comments is a special purpose reader that only a few libraries implement, like rewrite-clj.

I suspect it would be less work to take a library that already preserved whitespace and comments, and add preservation of map/set order, than vice versa.

if I would drop the comments/whitespace requirement, ordered maps could work. not sure if I can read ordered maps from plain edn maps or if I'll have to represent them using an ordered coll in .edn with a reader tag, which is kind of ugly

I think it would be a fairly small change to something like tools.reader lib to make it return ordered maps/sets, rather than the core unordered maps/sets.

yeah, I think I could also support it in edamame (which is already built on top of tools.reader)

I haven't dug into rewrite-clj, but perhaps it already maintains order, but perhaps it does so by not returning regular Clojure data structures, but a custom one representing the sequence of 'things' in the input file?

that's correct

and writing merge, update-in, etc on top of that is a lot of work

Attempting to preserve white space/comments, and returning data that one can do regular Clojure data manipulation on, sounds like magic.

can be done, I think. Just a lot of work

or writing your own custom implementation of those data manipulation functions on new data structures that contain representations of the white space/comments?

@borkdude I tried this. It's called ednup. I used it in a couple of little things.

I suspect your deep rewrite-clj knowledge will give you a better implementation here

I guess this could become useful pretty quickly to update deps.edn files, fixing some stuff like unqualified symbols

I wrote it for deps, yah. I have an alternative tag resolver that preserves comments.

Something I handled btw, was detection for that thing people do where they align their keys. That was feedback I got a lot of :)

In one of my commits I accidently deleted a file; how do I tell git that I want this file? I read the instructions for `git ammend` — you basically choose a commit and git will rebase on top of it. However; I’m unsure how to add back a file. This is not an Clojure question; but I didn’t know where else to go;

@dominicm Fully qualify dep symbols in deps.edn: https://github.com/borkdude/rewrite-edn#examples

Also added example of adding dependency: https://github.com/borkdude/rewrite-edn#add-dependency