yuqi1129 commented on code in PR #8242:
URL: https://github.com/apache/gravitino/pull/8242#discussion_r2297930968


##########
core/src/main/java/org/apache/gravitino/storage/relational/service/UserMetaService.java:
##########
@@ -285,12 +285,12 @@ public int deleteUserMetasByLegacyTimeline(long 
legacyTimeline, int limit) {
     SessionUtils.doMultipleWithCommit(
         () ->
             userDeletedCount[0] =
-                SessionUtils.doWithoutCommitAndFetchResult(
+                SessionUtils.getWithoutCommit(

Review Comment:
   Why did you change here? `getWithoutCommit` will not return the modified 
count.



##########
core/src/main/java/org/apache/gravitino/storage/relational/session/SqlSessions.java:
##########
@@ -62,53 +72,65 @@ public static SqlSession getSqlSession() {
    * thread local storage.
    */
   public static void commitAndCloseSqlSession() {
-    SqlSession sqlSession = sessions.get();
-    if (sqlSession != null) {
-      try {
-        sqlSession.commit();
-        sqlSession.close();
-      } finally {
-        sessions.remove();
-      }
-    }
+    handleSessionClose(true, false);
   }
 
   /**
    * Rollback the SqlSession object and close it. It also removes the 
SqlSession object from the
    * thread local storage.
    */
   public static void rollbackAndCloseSqlSession() {
-    SqlSession sqlSession = sessions.get();
-    if (sqlSession != null) {
-      try {
-        sqlSession.rollback();
-        sqlSession.close();
-      } finally {
-        sessions.remove();
-      }
-    }
+    handleSessionClose(false, true);
   }
 
   /** Close the SqlSession object and remove it from the thread local storage. 
*/
   public static void closeSqlSession() {
-    SqlSession sqlSession = sessions.get();
-    if (sqlSession != null) {
-      try {
-        sqlSession.close();
-      } finally {
-        sessions.remove();
-      }
-    }
+    handleSessionClose(false, false);
   }
 
   /**
-   * Get the Mapper object from the SqlSession object.
+   * Get the Mapper object from the SqlSession object. This method will open a 
session if one is not
+   * already opened.
    *
    * @param <T> the type of the mapper interface.
    * @param className the class name of the Mapper object.
    * @return the Mapper object.
    */
   public static <T> T getMapper(Class<T> className) {
+    // getSqlSession() is called to ensure a session exists and increment the 
count.
     return getSqlSession().getMapper(className);
   }
+
+  private static void handleSessionClose(boolean commit, boolean rollback) {
+    SqlSession sqlSession = sessions.get();
+    if (sqlSession == null) {
+      return;
+    }
+
+    int count = sessionCount.get() - 1;
+    sessionCount.set(count);
+
+    if (count == 0) {
+      try {
+        if (commit) {
+          sqlSession.commit();
+        } else if (rollback) {
+          sqlSession.rollback();
+        }
+        // Always close the session when the count reaches 0
+        sqlSession.close();
+      } finally {
+        sessions.remove();
+        sessionCount.remove();
+      }
+    } else if (count < 0) {
+      // This should not happen if the session management is correct.

Review Comment:
   When will this happen? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to