mchades commented on code in PR #8242:
URL: https://github.com/apache/gravitino/pull/8242#discussion_r2298158145
##########
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.
+ // Reset the count and remove the session to avoid further issues.
+ LOG.warn(
+ "Session count is negative: {}. Resetting session count and removing
session.",
+ count + 1);
Review Comment:
`count` should not be negative, unless an unknown bug is introduced.
--
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]
