github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-08-07 05:18:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Eliminate N+1 query storm in unread comment threads

Browse Source 
The dashboard's unread-comment-threads flow fired one
get-profiles-for-file-comments RPC call per file-id, producing
20+ concurrent HTTP requests and 20+ SQL queries for users with
unread threads across multiple files.

Widen the existing get-profiles-for-file-comments RPC to accept a
set of file ids (max 100) instead of a single id. The SQL uses
ANY(?::uuid[]) so a 1-element set has the same query plan as the
previous file_id = ?. Update the frontend unread-threads flow to
send a single batch request, and the workspace fetch-profiles
event to wrap the current file id in a 1-element set. The 100 cap
is enforced in both the frontend (with a log/warn on overflow) and
the schema (:max 100).

Fixes #10587

AI-assisted-by: opencode-go/minimax-m3
This commit is contained in:
Andrey Antukh 2026-07-08 11:12:45 +00:00 committed by Luis de Dios
parent 707cfac375
commit bf58ac0b9e
4 changed files with 108 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
backend/src/app/rpc/commands/comments.clj
View File

@ -371,44 +371,51 @@
;; --- COMMAND: Get file comments users
;; All the profiles that had comment the file, plus the current
;; profile.
;; All the profiles that had comment any of the given files, plus the
;; current profile. The :file-id param is a set (max 100) of file ids
;; so the same method serves both single-file and dashboard batch
;; callers.
(def ^:private sql:file-comment-users
"WITH available_profiles AS (
SELECT DISTINCT owner_id AS id
FROM comment
WHERE thread_id IN (SELECT id FROM comment_thread WHERE file_id=?)
)
SELECT p.id,
p.email,
p.fullname AS name,
p.fullname AS fullname,
p.photo_id,
p.is_active
FROM profile AS p
WHERE p.id IN (SELECT id FROM available_profiles) OR p.id=?")
SELECT DISTINCT c.owner_id AS id
FROM comment AS c
INNER JOIN comment_thread AS ct ON (ct.id = c.thread_id)
WHERE ct.file_id = ANY(?::uuid[])
)
SELECT p.id,
p.email,
p.fullname AS name,
p.fullname AS fullname,
p.photo_id,
p.is_active
FROM profile AS p
WHERE p.id IN (SELECT id FROM available_profiles) OR p.id=?")
(defn get-file-comments-users
[conn file-id profile-id]
(db/exec! conn [sql:file-comment-users file-id profile-id]))
(defn- get-file-comments-users
[conn file-ids profile-id]
(let [file-ids (db/create-array conn "uuid" file-ids)]
(db/exec! conn [sql:file-comment-users file-ids profile-id])))
(def ^:private
schema:get-profiles-for-file-comments
[:map {:title "get-profiles-for-file-comments"}
[:file-id ::sm/uuid]
[:file-id [::sm/set {:max 100} ::sm/uuid]]
[:share-id {:optional true} [:maybe ::sm/uuid]]])
(sv/defmethod ::get-profiles-for-file-comments
"Retrieves a list of profiles with limited set of properties of all
participants on comment threads of the file."
participants on comment threads of the given file(s)."
{::doc/added "1.15"
::doc/changes ["1.15" "Imported from queries and renamed."]
::doc/changes ["1.15.0" "Imported from queries and renamed."
"2.17.1" "Schema widened: :file-id now accepts a set (max 100) of file ids."]
::sm/params schema:get-profiles-for-file-comments}
[cfg {:keys [::rpc/profile-id file-id share-id]}]
(db/run! cfg (fn [{:keys [::db/conn]}]
(files/check-comment-permissions! conn profile-id file-id share-id)
(get-file-comments-users conn file-id profile-id))))
(db/run! cfg
(fn [{:keys [::db/conn] :as cfg}]
(doseq [fid file-id]
(files/check-comment-permissions! cfg profile-id fid share-id))
(get-file-comments-users conn file-id profile-id))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; MUTATION COMMANDS

55
backend/test/backend_tests/rpc_comment_test.clj
View File

@ -216,7 +216,7 @@
(t/testing "get profiles"
(let [data {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id profile-1)
:file-id (:id file-1)}
:file-id #{(:id file-1)}}
out (th/command! data)]
;; (th/print-result! out)
(t/is (th/success? out))
@ -227,12 +227,63 @@
(t/testing "get profiles 2"
(let [data {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id profile-2)
:file-id (:id file-1)}
:file-id #{(:id file-1)}}
out (th/command! data)]
;; (th/print-result! out)
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "get profiles batch"
(let [data {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id profile-1)
:file-id #{(:id file-1) (:id file-2)}}
out (th/command! data)]
;; (th/print-result! out)
(t/is (th/success? out))
(let [profiles (:result out)]
(t/is (= 1 (count profiles)))
(t/is (= (:id profile-1) (-> profiles first :id))))))
(t/testing "get profiles batch - permission denied"
(let [data {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id profile-2)
:file-id #{(:id file-1) (:id file-2)}}
out (th/command! data)]
;; (th/print-result! out)
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "get profiles batch - across multiple files"
;; Create a second comment thread on file-2 as profile-1, then
;; verify the batch query unions commenters across both files.
(let [page-id-2 (get-in file-2 [:data :pages 0])
_ (th/command! {::th/type :create-comment-thread
::rpc/profile-id (:id profile-1)
:file-id (:id file-2)
:page-id page-id-2
:position (gpt/point 0)
:content "second file"
:frame-id uuid/zero})
data {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id profile-1)
:file-id #{(:id file-1) (:id file-2)}}
out (th/command! data)]
;; (th/print-result! out)
(t/is (th/success? out))
(let [profiles (:result out)]
(t/is (= 1 (count profiles)))
(t/is (= (:id profile-1) (-> profiles first :id))))))
(t/testing "get profiles batch - rejects more than 100 file-ids"
(let [ids (set (repeatedly 101 #(random-uuid)))
data {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id profile-1)
:file-id ids}
out (th/command! data)]
;; (th/print-result! out)
(t/is (not (th/success? out)))
(t/is (th/ex-of-code? (:error out) :params-validation))))
(t/testing "delete comment"
(let [thread (-> (th/db-query :comment-thread {:file-id (:id file-1)}) first)
comment (-> (th/db-query :comment {:thread-id (:id thread) :content "comment 2 mod"}) first)

32
frontend/src/app/main/data/comments.cljs
View File

@ -9,6 +9,7 @@
[app.common.data :as d]
[app.common.data.macros :as dm]
[app.common.geom.point :as gpt]
[app.common.logging :as log]
[app.common.schema :as sm]
[app.common.time :as ct]
[app.common.types.shape-tree :as ctst]
@ -22,6 +23,12 @@
[beicon.v2.core :as rx]
[potok.v2.core :as ptk]))
(def ^:private max-batch-file-ids
"Maximum number of file-ids sent per batch request. Must match
the `:max 100` constraint on the backend schema
`get-profiles-for-file-comments`."
100)
(def ^:private schema:comment-thread
[:map {:title "CommentThread"}
[:id ::sm/uuid]
@ -454,13 +461,24 @@
(->> (rp/cmd! :get-unread-comment-threads {:team-id team-id})
(rx/merge-map
(fn [comments]
(rx/concat
(rx/of (partial fetched-comments comments))
(let [ids (vec (into #{} (map :file-id) comments))
n (count ids)
ids (if (> n max-batch-file-ids)
(do
(log/warn :msg "unread-threads: capping file-ids at max"
:max max-batch-file-ids
:count n
:team-id team-id)
(subvec ids 0 max-batch-file-ids))
ids)]
(rx/concat
(rx/of (partial fetched-comments comments))
(->> (rx/from (into #{} (map :file-id) comments))
(rx/merge-map #(rp/cmd! :get-profiles-for-file-comments {:file-id %}))
(rx/reduce #(merge %1 (d/index-by :id %2)) {})
(rx/map #(partial fetched-users %))))))
(if (seq ids)
(->> (rp/cmd! :get-profiles-for-file-comments {:file-id ids})
(rx/map #(d/index-by :id %))
(rx/map #(partial fetched-users %)))
(rx/of (partial fetched-users {})))))))
(rx/catch #(rx/throw {:type :comment-error})))))))
(defn mark-all-threads-as-read
@ -682,7 +700,7 @@
(let [file-id (:current-file-id state)
share-id (or (-> state :viewer-local :share-id)
(:current-share-id state))]
(->> (rp/cmd! :get-profiles-for-file-comments {:file-id file-id :share-id share-id})
(->> (rp/cmd! :get-profiles-for-file-comments {:file-id #{file-id} :share-id share-id})
(rx/map (fn [profiles]
#(update % :profiles merge (d/index-by :id profiles)))))))))

14
frontend/src/app/main/data/profile.cljs
View File

@ -371,20 +371,6 @@
(js/console.error "delete-photo failed" cause)
(rx/of (refresh-profile))))))))
(defn fetch-file-comments-users
[{:keys [team-id]}]
(assert (uuid? team-id) "expected a valid uuid for `team-id`")
(letfn [(fetched [users state]
(->> users
(d/index-by :id)
(assoc state :file-comments-users)))]
(ptk/reify ::fetch-file-comments-users
ptk/WatchEvent
(watch [_ state _]
(let [share-id (-> state :viewer-local :share-id)]
(->> (rp/cmd! :get-profiles-for-file-comments {:team-id team-id :share-id share-id})
(rx/map #(partial fetched %))))))))
;; --- EVENT: request-account-deletion
(def profile-deleted-event?