You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "wizhuo (via GitHub)" <gi...@apache.org> on 2023/02/21 05:27:34 UTC

[GitHub] [shardingsphere-elasticjob] wizhuo opened a new pull request, #2184: make startup faster

wizhuo opened a new pull request, #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184

   Fixes #ISSUSE_ID.
   
   
   I think the same dataSource just need one RDBJobEventStorage instance 。after all, init RDBJobEventStorage  will take some time to get a rdb connection  and check whether the table and index exist
   
   The benefit of the change is that the startup will be  #faster
   
   In the experiment, starting 100 jobs can save about 8 seconds, of course, this depends on the environment, if the database and application are on different networks may be slower
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a diff in pull request #2184: make startup faster

Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on code in PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184#discussion_r1136462376


##########
elasticjob-ecosystem/elasticjob-tracing/elasticjob-tracing-api/src/main/java/org/apache/shardingsphere/elasticjob/tracing/exception/WrapException.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.tracing.exception;
+
+/**
+ * use to wrap Exception.
+ * @author willJo
+ */
+
+public class WrapException extends RuntimeException {
+    public WrapException(final Throwable cause) {
+        super(cause);
+    }

Review Comment:
   ```suggestion
   /**
    * Use to wrap Exception.
    */
   public class WrapException extends RuntimeException {
       
       public WrapException(final Throwable cause) {
           super(cause);
       }
   ```
   
   `@author` is not applicable here.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a diff in pull request #2184: make startup faster

Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on code in PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184#discussion_r1115145807


##########
elasticjob-ecosystem/elasticjob-tracing/elasticjob-tracing-rdb/src/main/java/org/apache/shardingsphere/elasticjob/tracing/rdb/storage/RDBJobEventStorage.java:
##########
@@ -61,14 +63,41 @@ public final class RDBJobEventStorage {
     private final DatabaseType databaseType;
     
     private final RDBStorageSQLMapper sqlMapper;
+
+    private static final Map<DataSource, RDBJobEventStorage> STORAGE_MAP = new ConcurrentHashMap<>();
+
     
     static {
         for (DatabaseType each : ServiceLoader.load(DatabaseType.class)) {
             DATABASE_TYPES.put(each.getType(), each);
         }
     }
+
+    public static RDBJobEventStorage getInstance(final DataSource dataSource) throws SQLException {
+        return wrapException(() -> STORAGE_MAP.computeIfAbsent(dataSource, (ds) -> {
+            try {
+                return new RDBJobEventStorage(ds);
+            } catch (SQLException e) {
+                throw new RuntimeException(e);
+            }
+        }));
+    }
+
+    public static RDBJobEventStorage wrapException(Callable<RDBJobEventStorage> callable) throws SQLException {

Review Comment:
   How about using `Supplier`?



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] wizhuo commented on a diff in pull request #2184: make startup faster

Posted by "wizhuo (via GitHub)" <gi...@apache.org>.
wizhuo commented on code in PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184#discussion_r1115187104


##########
elasticjob-ecosystem/elasticjob-tracing/elasticjob-tracing-rdb/src/main/java/org/apache/shardingsphere/elasticjob/tracing/rdb/storage/RDBJobEventStorage.java:
##########
@@ -61,14 +63,41 @@ public final class RDBJobEventStorage {
     private final DatabaseType databaseType;
     
     private final RDBStorageSQLMapper sqlMapper;
+
+    private static final Map<DataSource, RDBJobEventStorage> STORAGE_MAP = new ConcurrentHashMap<>();
+
     
     static {
         for (DatabaseType each : ServiceLoader.load(DatabaseType.class)) {
             DATABASE_TYPES.put(each.getType(), each);
         }
     }
+
+    public static RDBJobEventStorage getInstance(final DataSource dataSource) throws SQLException {
+        return wrapException(() -> STORAGE_MAP.computeIfAbsent(dataSource, (ds) -> {
+            try {
+                return new RDBJobEventStorage(ds);
+            } catch (SQLException e) {
+                throw new RuntimeException(e);
+            }
+        }));
+    }
+
+    public static RDBJobEventStorage wrapException(Callable<RDBJobEventStorage> callable) throws SQLException {

Review Comment:
   I would love to! I've modified it, please check it out



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a diff in pull request #2184: make startup faster

Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on code in PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184#discussion_r1136462376


##########
elasticjob-ecosystem/elasticjob-tracing/elasticjob-tracing-api/src/main/java/org/apache/shardingsphere/elasticjob/tracing/exception/WrapException.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.tracing.exception;
+
+/**
+ * use to wrap Exception.
+ * @author willJo
+ */
+
+public class WrapException extends RuntimeException {
+    public WrapException(final Throwable cause) {
+        super(cause);
+    }

Review Comment:
   ```suggestion
   /**
    * use to wrap Exception.
    */
   public class WrapException extends RuntimeException {
       
       public WrapException(final Throwable cause) {
           super(cause);
       }
   ```
   
   `@author` is not applicable here.



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] wizhuo commented on a diff in pull request #2184: make startup faster

Posted by "wizhuo (via GitHub)" <gi...@apache.org>.
wizhuo commented on code in PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184#discussion_r1115708932


##########
elasticjob-ecosystem/elasticjob-tracing/elasticjob-tracing-rdb/src/main/java/org/apache/shardingsphere/elasticjob/tracing/rdb/storage/RDBJobEventStorage.java:
##########
@@ -61,14 +63,41 @@ public final class RDBJobEventStorage {
     private final DatabaseType databaseType;
     
     private final RDBStorageSQLMapper sqlMapper;
+
+    private static final Map<DataSource, RDBJobEventStorage> STORAGE_MAP = new ConcurrentHashMap<>();
+
     
     static {
         for (DatabaseType each : ServiceLoader.load(DatabaseType.class)) {
             DATABASE_TYPES.put(each.getType(), each);
         }
     }
+
+    public static RDBJobEventStorage getInstance(final DataSource dataSource) throws SQLException {
+        return wrapException(() -> STORAGE_MAP.computeIfAbsent(dataSource, (ds) -> {
+            try {
+                return new RDBJobEventStorage(ds);
+            } catch (SQLException e) {
+                throw new RuntimeException(e);
+            }
+        }));
+    }
+
+    public static RDBJobEventStorage wrapException(Callable<RDBJobEventStorage> callable) throws SQLException {

Review Comment:
   ![image](https://user-images.githubusercontent.com/46399833/220915814-6d22d016-c86d-4eef-83fc-1af8c8749eb3.png)
   https://github.com/apache/shardingsphere-elasticjob/actions/runs/4248881615/jobs/7388688271
    
   The above link is the information about the failure of the last build. From the error information, we can see that zookeeper was closed for some reason, which caused the unit test to fail. I tried executing the command locally with success: 
   
   ```
   ./mvnw --batch-mode --no-transfer-progress '-Dmaven.javadoc.skip=true' clean install
   ```
   I  can not reproduce the above error, what do you suggest?
   



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] codecov-commenter commented on pull request #2184: make startup faster

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184#issuecomment-1469313976

   ## [Codecov](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/2184?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2184](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/2184?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2969d17) into [master](https://codecov.io/gh/apache/shardingsphere-elasticjob/commit/f2a73a89e8290a04bcd4d9751a1948bd0e02e7dc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f2a73a8) will **decrease** coverage by `0.04%`.
   > The diff coverage is `71.42%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2184      +/-   ##
   ============================================
   - Coverage     84.61%   84.58%   -0.04%     
   - Complexity     1915     1918       +3     
   ============================================
     Files           286      287       +1     
     Lines          6279     6291      +12     
     Branches        695      696       +1     
   ============================================
   + Hits           5313     5321       +8     
   - Misses          646      649       +3     
   - Partials        320      321       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/2184?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...re/elasticjob/tracing/exception/WrapException.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/2184?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZWxhc3RpY2pvYi1lY29zeXN0ZW0vZWxhc3RpY2pvYi10cmFjaW5nL2VsYXN0aWNqb2ItdHJhY2luZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvdHJhY2luZy9leGNlcHRpb24vV3JhcEV4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...ticjob/tracing/rdb/storage/RDBJobEventStorage.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/2184?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZWxhc3RpY2pvYi1lY29zeXN0ZW0vZWxhc3RpY2pvYi10cmFjaW5nL2VsYXN0aWNqb2ItdHJhY2luZy1yZGIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvdHJhY2luZy9yZGIvc3RvcmFnZS9SREJKb2JFdmVudFN0b3JhZ2UuamF2YQ==) | `85.18% <81.81%> (-0.26%)` | :arrow_down: |
   | [...icjob/tracing/rdb/listener/RDBTracingListener.java](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/2184?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZWxhc3RpY2pvYi1lY29zeXN0ZW0vZWxhc3RpY2pvYi10cmFjaW5nL2VsYXN0aWNqb2ItdHJhY2luZy1yZGIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VsYXN0aWNqb2IvdHJhY2luZy9yZGIvbGlzdGVuZXIvUkRCVHJhY2luZ0xpc3RlbmVyLmphdmE=) | `100.00% <100.00%> (ø)` | |
   
   ... and [2 files with indirect coverage changes](https://codecov.io/gh/apache/shardingsphere-elasticjob/pull/2184/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a diff in pull request #2184: make startup faster

Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on code in PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184#discussion_r1115145807


##########
elasticjob-ecosystem/elasticjob-tracing/elasticjob-tracing-rdb/src/main/java/org/apache/shardingsphere/elasticjob/tracing/rdb/storage/RDBJobEventStorage.java:
##########
@@ -61,14 +63,41 @@ public final class RDBJobEventStorage {
     private final DatabaseType databaseType;
     
     private final RDBStorageSQLMapper sqlMapper;
+
+    private static final Map<DataSource, RDBJobEventStorage> STORAGE_MAP = new ConcurrentHashMap<>();
+
     
     static {
         for (DatabaseType each : ServiceLoader.load(DatabaseType.class)) {
             DATABASE_TYPES.put(each.getType(), each);
         }
     }
+
+    public static RDBJobEventStorage getInstance(final DataSource dataSource) throws SQLException {
+        return wrapException(() -> STORAGE_MAP.computeIfAbsent(dataSource, (ds) -> {
+            try {
+                return new RDBJobEventStorage(ds);
+            } catch (SQLException e) {
+                throw new RuntimeException(e);
+            }
+        }));
+    }
+
+    public static RDBJobEventStorage wrapException(Callable<RDBJobEventStorage> callable) throws SQLException {

Review Comment:
   How about using `Supplier` instead of `Callable`?



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a diff in pull request #2184: make startup faster

Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on code in PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184#discussion_r1136462977


##########
elasticjob-ecosystem/elasticjob-tracing/elasticjob-tracing-rdb/src/main/java/org/apache/shardingsphere/elasticjob/tracing/rdb/storage/RDBJobEventStorage.java:
##########
@@ -55,26 +58,63 @@ public final class RDBJobEventStorage {
     private static final String TASK_ID_STATE_INDEX = "TASK_ID_STATE_INDEX";
     
     private static final Map<String, DatabaseType> DATABASE_TYPES = new HashMap<>();
-    
+
+    private static final Map<DataSource, RDBJobEventStorage> STORAGE_MAP = new ConcurrentHashMap<>();
+
     private final DataSource dataSource;
-    
+
     private final DatabaseType databaseType;
-    
+
     private final RDBStorageSQLMapper sqlMapper;
-    
+
     static {
         for (DatabaseType each : ServiceLoader.load(DatabaseType.class)) {
             DATABASE_TYPES.put(each.getType(), each);
         }
     }
-    
-    public RDBJobEventStorage(final DataSource dataSource) throws SQLException {
+
+    private RDBJobEventStorage(final DataSource dataSource) throws SQLException {
         this.dataSource = dataSource;
         databaseType = getDatabaseType(dataSource);
         sqlMapper = new RDBStorageSQLMapper(databaseType.getSQLPropertiesFile());
         initTablesAndIndexes();
     }
-    
+
+    /**
+     * the same dataSource  always return the same RDBJobEventStorage instance.
+     *
+     * @param dataSource dataSource
+     * @return RDBJobEventStorage instance
+     * @throws SQLException SQLException
+     */
+    public static RDBJobEventStorage getInstance(final DataSource dataSource) throws SQLException {
+        return wrapException(() -> STORAGE_MAP.computeIfAbsent(dataSource, ds -> {
+            try {
+                return new RDBJobEventStorage(ds);
+            } catch (SQLException e) {
+                throw new WrapException(e);
+            }
+        }));
+    }
+
+    /**
+     * wrapException util method.

Review Comment:
   ```suggestion
        * WrapException util method.
   ```



##########
elasticjob-ecosystem/elasticjob-tracing/elasticjob-tracing-rdb/src/main/java/org/apache/shardingsphere/elasticjob/tracing/rdb/storage/RDBJobEventStorage.java:
##########
@@ -55,26 +58,63 @@ public final class RDBJobEventStorage {
     private static final String TASK_ID_STATE_INDEX = "TASK_ID_STATE_INDEX";
     
     private static final Map<String, DatabaseType> DATABASE_TYPES = new HashMap<>();
-    
+
+    private static final Map<DataSource, RDBJobEventStorage> STORAGE_MAP = new ConcurrentHashMap<>();
+
     private final DataSource dataSource;
-    
+
     private final DatabaseType databaseType;
-    
+
     private final RDBStorageSQLMapper sqlMapper;
-    
+
     static {
         for (DatabaseType each : ServiceLoader.load(DatabaseType.class)) {
             DATABASE_TYPES.put(each.getType(), each);
         }
     }
-    
-    public RDBJobEventStorage(final DataSource dataSource) throws SQLException {
+
+    private RDBJobEventStorage(final DataSource dataSource) throws SQLException {
         this.dataSource = dataSource;
         databaseType = getDatabaseType(dataSource);
         sqlMapper = new RDBStorageSQLMapper(databaseType.getSQLPropertiesFile());
         initTablesAndIndexes();
     }
-    
+
+    /**
+     * the same dataSource  always return the same RDBJobEventStorage instance.

Review Comment:
   ```suggestion
        * The same dataSource always return the same RDBJobEventStorage instance.
   ```



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere-elasticjob] TeslaCN merged pull request #2184: make startup faster

Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN merged PR #2184:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/2184


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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