Merge pull request 'Restart btcdata after losing its writer session' (#3) from fix/restart-closed-writer-session into master

btcdata-deploy.md

@@ -1,5 +1,60 @@
 # btcdata Kubernetes deployment plan
 
+## 2026-10-09 incident: closed writer session never restarted
+
+The citadel pod was Running but not Ready, and both btcprice pods were unready.
+All writer queries failed with `Connection is closed`. Readiness correctly
+returned 503, but `/api/health` always returned 200, so Kubernetes did not restart
+the process. The initial connection-loss cause was not established from the
+available logs. Fenced connections deliberately never reconnect in place.
+
+Recovery deleted only the failed pod (normal graceful deletion); its Deployment
+recreated it, and btcdata and both btcprice pods became Ready. No lock was
+bypassed or database backend terminated for this incident.
+
+The source fix makes `/api/health` return 503 when the writer session is closed
+(or absent). This checks local JDBC state only: no SQL, external APIs, or Binance
+freshness are used in liveness. Existing probes will then restart an irrecoverable
+writer after three failed checks. Readiness retains database/freshness checks.
+Unit tests passed: 21 tests, 63 assertions. PostgreSQL integration tests were
+not run because the local Docker daemon was unavailable.
+
+The recovered pod still runs `main-20260930155338`, not the new source changes.
+Flux apps reconciliation is blocked on infrastructure health checks for image
+repositories; building/publishing and a successful Flux rollout remain necessary
+to activate the liveness and keepalive source fixes.
+
+## 2026-10-07 incident: orphaned writer session
+
+The `sejs/btcdata` pod on citadel entered CrashLoopBackOff with
+`Another btcdata writer holds the database lock`. The prior pod on srv1893094
+had been removed after readiness/network failures, but its PostgreSQL backend
+(PID 3531439, client 100.75.252.111) still held advisory lock 728194106. No old
+btcdata Java process or database TCP connection remained on that node.
+
+Recovery terminated only that verified orphaned backend. Kubernetes restarted
+btcdata successfully; btcdata and both btcprice pods became Ready. Do not bypass
+the advisory lock or terminate its owner without first verifying the old writer
+is stopped.
+
+Production database `btcprod` now has connection defaults:
+`tcp_keepalives_idle=60`, `tcp_keepalives_interval=10`,
+`tcp_keepalives_count=3`, and `tcp_user_timeout=90000` (milliseconds).
+These apply to **new connections**, not the already-running recovered session.
+Server-side dead-peer detection prevents a disappeared node from retaining the
+lock for the default multi-hour TCP timeout; it does not evict a live competing
+writer. The application now also sets these values on its lock-owning session
+before acquiring the fence (source change; not yet built/deployed).
+
+Validation: 20 unit tests / 58 assertions passed; disposable PostgreSQL integration
+tests were not run because the local Docker daemon was unavailable. New
+production connection settings were verified with `SHOW`.
+
+Rollback database defaults, if necessary, with `ALTER DATABASE btcprod RESET
+<setting>` for each of the four settings above; new connections inherit the
+server defaults again. Reverting the application change requires a normal image
+build and Flux rollout. The orphan-session termination is not reversible.
+
 ## 2026-09-19 status and proposed recovery
 
 Kubernetes deployment already exists: one `Recreate` replica on citadel, eligible

scripts/fetch-year.sh

@@ -43,38 +43,86 @@ cleanup() {
   if [[ -n $forward_pid ]]; then
     kill "$forward_pid" 2>/dev/null || true
     wait "$forward_pid" 2>/dev/null || true
+    forward_pid=
   fi
 }
 trap cleanup EXIT
 trap 'exit 130' INT
 trap 'exit 143' TERM
 
-if ! service_ready; then
+wait_for_pod() {
+  local selector
+  # Use the service's selector rather than assuming a particular pod label.
+  if ! selector=$(kubectl -n sejs get svc btcdata --request-timeout=10s \
+    -o go-template='{{range $key, $value := .spec.selector}}{{$key}}={{$value}},{{end}}'); then
+    return 1
+  fi
+  selector=${selector%,}
+  if [[ -z $selector ]]; then
+    echo "Service sejs/btcdata has no pod selector." >&2
+    return 1
+  fi
+  echo "Waiting up to 5 minutes for btcdata pods to become Ready..." >&2
+  kubectl -n sejs wait --for=condition=Ready pod --selector="$selector" \
+    --timeout=300s --request-timeout=310s >&2
+}
+
+start_forward() {
+  local attempt
+  cleanup
   if ! command -v kubectl >/dev/null 2>&1; then
     echo "Service unavailable on localhost:4101 and kubectl is not installed." >&2
-    exit 1
+    return 1
+  fi
+  if ! wait_for_pod; then
+    echo "btcdata pod readiness check failed; not opening a tunnel." >&2
+    return 1
   fi
   echo "Starting kubectl port-forward for sejs/btcdata..." >&2
   kubectl -n sejs port-forward svc/btcdata 4101:4101 >&2 &
   forward_pid=$!
 
-  ready=false
   for (( attempt = 0; attempt < 30; attempt++ )); do
     if ! kill -0 "$forward_pid" 2>/dev/null; then
       echo "Port-forward exited before the service became ready." >&2
-      exit 1
+      cleanup
+      return 1
     fi
     if service_ready; then
-      ready=true
-      break
+      return 0
     fi
     sleep 1
   done
-  if [[ $ready != true ]]; then
   echo "Timed out waiting for the service on localhost:4101." >&2
-    exit 1
+  cleanup
+  return 1
+}
+
+ensure_service() {
+  local retry
+  if service_ready; then
+    return 0
   fi
+  for (( retry = 1; retry <= 3; retry++ )); do
+    echo "Recovering port-forward (attempt $retry/3)..." >&2
+    if start_forward; then
+      return 0
     fi
+    if (( retry < 3 )); then
+      sleep 2
+    fi
+  done
+  echo "Unable to restore the service on localhost:4101." >&2
+  return 1
+}
+
+fetch_date() {
+  curl --fail --silent --show-error --connect-timeout 5 -X GET \
+    "http://localhost:4101/api/price/day/$1" \
+    -H "accept: application/json" --write-out '\n'
+}
+
+ensure_service || exit 1
 
 # Generate dates without relying on platform-specific date command options.
 status=0
@@ -90,9 +138,14 @@ for (( month_index = 0; month_index < 12; month_index++ )); do
       day=$((days - day_index))
     fi
     printf -v date '%s-%02d-%02d' "$year" "$month" "$day"
-    if ! curl --fail --silent --show-error -X GET \
-      "http://localhost:4101/api/price/day/$date" \
-      -H "accept: application/json" --write-out '\n'; then
+    if ! fetch_date "$date"; then
+      if ! service_ready; then
+        ensure_service || exit 1
+        echo "Retrying $date after tunnel recovery..." >&2
+        if fetch_date "$date"; then
+          continue
+        fi
+      fi
       echo "Failed to fetch $date" >&2
       status=1
     fi

src/clj/pmagnus/btcdata/web/controllers/health.clj

@@ -2,15 +2,21 @@
   (:require
    [clojure.tools.logging :as log]
    [ring.util.http-response :as response]
-   [pmagnus.btcdata.ws.binance :as binance])
+   [pmagnus.btcdata.ws.binance :as binance]
+   [pmagnus.btcdata.writer :as writer])
   (:import
    [java.lang.management ManagementFactory]))
 
-(defn healthcheck! [_req]
+(defn healthcheck!
+  "No remote dependency checks. A permanently closed writer session needs restart."
+  [query-fn _req]
+  (if (writer/session-live? query-fn)
     (response/ok
      {:time    (str (java.time.Instant/now))
       :up-time (.. ManagementFactory getRuntimeMXBean getUptime)
-    :app     {:status "up"}}))
+      :app     {:status "up"}})
+    (response/service-unavailable {:status "restart required"
+                                   :reason "writer session closed"})))
 
 (defn readiness-check!
   "Require the lock-owning database session and a fresh upstream stream."

src/clj/pmagnus/btcdata/web/routes/api.clj

@@ -180,7 +180,7 @@
            :swagger {:info {:title "btcdata API"}}
            :handler (swagger/create-swagger-handler)}}]
    ["/health"
-    {:get health/healthcheck!}]
+    {:get (fn [req] (health/healthcheck! (:query-fn opts) req))}]
    ["/ready"
     {:get (fn [req] (health/readiness-check! (:query-fn opts) (:binance opts) req))}]
    ["/price/ws"

src/clj/pmagnus/btcdata/writer.clj

@@ -7,6 +7,24 @@
 
 (def lock-id 728194106)
 
+(defn configure-session! [connection]
+  ;; Client-side network timeouts cannot reap a server session left behind by a
+  ;; dead node. Server TCP keepalives release its fence after ~90s of silence.
+  (doseq [sql ["SET tcp_keepalives_idle = 60"
+               "SET tcp_keepalives_interval = 10"
+               "SET tcp_keepalives_count = 3"
+               "SET tcp_user_timeout = 90000"]]
+    (jdbc/execute-one! connection [sql])))
+
+(defn session-live?
+  "Local-only liveness check; a closed fenced session can never recover in place."
+  [query-fn]
+  (if-let [connection (:connection (meta query-fn))]
+    (try
+      (not (.isClosed ^java.sql.Connection connection))
+      (catch Exception _ false))
+    false))
+
 (defn guarded-query [connection query-fn]
   (fn [query params]
     (locking connection
@@ -20,6 +38,7 @@
       (.setNetworkTimeout connection
         (reify java.util.concurrent.Executor (execute [_ task] (.run ^Runnable task)))
         10000)
+      (configure-session! connection)
       (when-not (:locked (jdbc/execute-one! connection
                            ["select pg_try_advisory_lock(?) as locked" lock-id]))
         (throw (ex-info "Another btcdata writer holds the database lock" {})))

test/clj/pmagnus/btcdata/web/controllers/health_test.clj

@@ -4,8 +4,18 @@
 
 (def disabled-stream {:state (atom {:enabled? false}) :latest-price (atom nil)})
 
-(deftest healthcheck-is-process-only
-  (is (= 200 (:status (health/healthcheck! {})))))
+(deftest liveness-check-is-local-and-detects-lost-writer
+  (let [closed? (atom false)
+        connection (proxy [java.sql.Connection] []
+                     (isClosed [] @closed?))
+        query-fn (with-meta (fn [& _] (throw (Exception. "Must not query database")))
+                   {:connection connection})]
+    (is (= 200 (:status (health/healthcheck! query-fn {}))))
+    (reset! closed? true)
+    (is (= 503 (:status (health/healthcheck! query-fn {}))))
+    (is (= "writer session closed"
+           (get-in (health/healthcheck! query-fn {}) [:body :reason]))))
+  (is (= 503 (:status (health/healthcheck! (fn [& _]) {})))))
 
 (deftest readiness-check-tests-database
   (let [called (atom nil)

test/clj/pmagnus/btcdata/writer_test.clj

@@ -13,6 +13,25 @@
     (is (= :result (query :insert-price {:price 1})))
     (is (= [connection :insert-price {:price 1}] @seen))))
 
+(deftest session-liveness-fails-closed
+  (is (false? (writer/session-live? (fn [& _]))))
+  (let [connection (proxy [java.sql.Connection] []
+                     (isClosed [] (throw (java.sql.SQLException. "broken"))))]
+    (is (false? (writer/session-live? (with-meta (fn [& _])
+                                       {:connection connection}))))))
+
+(deftest server-session-has-bounded-dead-peer-detection
+  (let [connection (Object.)
+        calls (atom [])]
+    (with-redefs [jdbc/execute-one! (fn [conn sql] (swap! calls conj [conn sql]))]
+      (writer/configure-session! connection))
+    (is (= (mapv #(vector connection [%])
+                 ["SET tcp_keepalives_idle = 60"
+                  "SET tcp_keepalives_interval = 10"
+                  "SET tcp_keepalives_count = 3"
+                  "SET tcp_user_timeout = 90000"])
+           @calls))))
+
 (deftest postgres-session-fencing
   ;; Optional disposable PostgreSQL integration test; never use production here.
   (if-let [url (System/getenv "BTCDATA_TEST_JDBC_URL")]
@@ -22,10 +41,19 @@
           first-writer (ig/init-key :writer/query-fn opts)]
       (try
         (is (= 1 (:healthy (first-writer :check {}))))
+        (let [connection (:connection (meta first-writer))]
+          (doseq [[setting expected] [["tcp_keepalives_idle" "60"]
+                                     ["tcp_keepalives_interval" "10"]
+                                     ["tcp_keepalives_count" "3"]
+                                     ["tcp_user_timeout" "90000"]]]
+            (is (= expected (:value (jdbc/execute-one! connection
+                                     ["select current_setting(?) as value" setting]))))))
         (is (thrown-with-msg? clojure.lang.ExceptionInfo #"Another btcdata writer"
               (ig/init-key :writer/query-fn opts)))
         ;; Closing the owning session cannot transparently reconnect for writes.
+        (is (writer/session-live? first-writer))
         (.close (:connection (meta first-writer)))
+        (is (false? (writer/session-live? first-writer)))
         (is (thrown? Exception (first-writer :check {})))
         (let [replacement (ig/init-key :writer/query-fn opts)]
           (try