✨ Relax transaction requirement on comment thread creation rpc method

rochmadnf · Oct 8, 2024 · f777845 · f777845
1 parent a1f5bca
commit f777845
Showing 1 changed file with 32 additions and 24 deletions.
diff --git a/backend/src/app/rpc/commands/comments.clj b/backend/src/app/rpc/commands/comments.clj
@@ -71,10 +71,15 @@
   [conn comment-id & {:as opts}]
   (db/get-by-id conn :comment comment-id opts))
 
+(def ^:private sql:get-next-seqn
+  "SELECT (f.comment_thread_seqn + 1) AS next_seqn
+     FROM file AS f
+    WHERE f.id = ?
+      FOR UPDATE")
+
 (defn- get-next-seqn
   [conn file-id]
-  (let [sql "select (f.comment_thread_seqn + 1) as next_seqn from file as f where f.id = ?"
-        res (db/exec-one! conn [sql file-id])]
+  (let [res (db/exec-one! conn [sql:get-next-seqn file-id])]
     (:next-seqn res)))
 
 (def sql:upsert-comment-thread-status
@@ -297,21 +302,16 @@
    [:frame-id ::sm/uuid]
    [:share-id {:optional true} [:maybe ::sm/uuid]]])
 
-;; FIXME: relax transaction requirements
-
 (sv/defmethod ::create-comment-thread
   {::doc/added "1.15"
    ::webhooks/event? true
    ::rtry/enabled true
    ::rtry/when rtry/conflict-exception?
-   ::sm/params schema:create-comment-thread
-   ::db/transaction true}
-  [{:keys [::db/conn] :as cfg}
-   {:keys [::rpc/profile-id ::rpc/request-at file-id page-id share-id position content frame-id]}]
-
+   ::sm/params schema:create-comment-thread}
+  [cfg {:keys [::rpc/profile-id ::rpc/request-at file-id page-id share-id position content frame-id]}]
   (files/check-comment-permissions! cfg profile-id file-id share-id)
 
-  (let [{:keys [team-id project-id page-name]} (get-file conn file-id page-id)]
+  (let [{:keys [team-id project-id page-name]} (get-file cfg file-id page-id)]
 
     (-> cfg
         (assoc ::quotes/profile-id profile-id)
@@ -321,21 +321,29 @@
         (quotes/check! {::quotes/id ::quotes/comment-threads-per-file}
                        {::quotes/id ::quotes/comments-per-file}))
 
-    (create-comment-thread conn {:created-at request-at
-                                 :profile-id profile-id
-                                 :file-id file-id
-                                 :page-id page-id
-                                 :page-name page-name
-                                 :position position
-                                 :content content
-                                 :frame-id frame-id})))
+    (let [params {:created-at request-at
+                  :profile-id profile-id
+                  :file-id file-id
+                  :page-id page-id
+                  :page-name page-name
+                  :position position
+                  :content content
+                  :frame-id frame-id}]
+      (db/tx-run! cfg create-comment-thread params))))
 
 (defn- create-comment-thread
-  [conn {:keys [profile-id file-id page-id page-name created-at position content frame-id]}]
+  [{:keys [::db/conn] :as cfg}
+   {:keys [profile-id file-id page-id page-name created-at position content frame-id]}]
+
+  (let [;; NOTE: we take the next seq number from a separate query
+        ;; because we need to lock the file for avoid race conditions
+
+        ;; FIXME: this method touches and locks the file table,which
+        ;; is already heavy-update tablel; we need to think on move
+        ;; the sequence state management to a different table or
+        ;; different storage (example: redis) for alivate the update
+        ;; pression on the file table
 
-  (let [;; NOTE: we take the next seq number from a separate query because the whole
-        ;; operation can be retried on conflict, and in this case the new seq shold be
-        ;; retrieved from the database.
         seqn      (get-next-seqn conn file-id)
         thread-id (uuid/next)
         thread    (db/insert! conn :comment-thread
@@ -364,7 +372,8 @@
     ;; Optimistic update of current seq number on file.
     (db/update! conn :file
                 {:comment-thread-seqn seqn}
-                {:id file-id})
+                {:id file-id}
+                {::db/return-keys false})
 
     (-> thread
         (select-keys [:id :file-id :page-id])
@@ -387,7 +396,6 @@
                       (files/check-comment-permissions! conn profile-id file-id share-id)
                       (upsert-comment-thread-status! conn profile-id id)))))
 
-
 ;; --- COMMAND: Update Comment Thread
 
 (def ^:private