Re: [PR] [FLINK-36696] [flink-autoscaler-plugin-jdbc] Switch sql connection usages to datasource [flink-kubernetes-operator]

["1996fanrui (via GitHub)" <gi...@apache.org>]

1996fanrui commented on code in PR #929:
URL: https://github.com/apache/flink-kubernetes-operator/pull/929#discussion_r1909700607


##########
flink-autoscaler-plugin-jdbc/src/test/java/org/apache/flink/autoscaler/jdbc/state/AbstractJdbcStateStoreITCase.java:
##########
@@ -45,24 +45,17 @@
 abstract class AbstractJdbcStateStoreITCase implements DatabaseTest {
 
     private static final String DEFAULT_JOB_KEY = "jobKey";
-    private Connection conn;
+    private DataSource dataSource;
     private CountableJdbcStateInteractor jdbcStateInteractor;
     private JdbcStateStore jdbcStateStore;
 
     @BeforeEach
     void beforeEach() throws Exception {
-        this.conn = getConnection();
-        this.jdbcStateInteractor = new CountableJdbcStateInteractor(conn);
+        this.dataSource = getDataSource();
+        this.jdbcStateInteractor = new CountableJdbcStateInteractor(dataSource);
         this.jdbcStateStore = new JdbcStateStore(jdbcStateInteractor);
     }
 
-    @AfterEach
-    void afterEach() throws SQLException {
-        if (conn != null) {
-            conn.close();
-        }
-    }

Review Comment:
   Why don't close dataSource here?
   
   As I understand, each test will create a new dataSource. The old dataSource will leak some connections if we don't close dataSource here.



##########
flink-autoscaler-plugin-jdbc/src/test/java/org/apache/flink/autoscaler/jdbc/state/AbstractJdbcStateInteractorITCase.java:
##########
@@ -42,33 +42,32 @@ void testAllOperations() throws Exception {
         var value1 = "value1";
         var value2 = "value2";
         var value3 = "value3";
-        try (var conn = getConnection()) {
-            var jdbcStateInteractor = new JdbcStateInteractor(conn);
-            assertThat(jdbcStateInteractor.queryData(jobKey)).isEmpty();
+        var dataSource = getDataSource();

Review Comment:
   It's similar with the last comment, we didn't close the dataSource.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org