You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2022/11/08 10:27:50 UTC

[GitHub] [shardingsphere] azexcy opened a new pull request, #22013: Improve migration IT

azexcy opened a new pull request, #22013:
URL: https://github.com/apache/shardingsphere/pull/22013

   
   
   Changes proposed in this pull request:
     - Improve migration IT
     - Fix codestyle
     -
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following options:
   - [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
   - [ ] I have self-reviewed the commit code.
   - [ ] I have (or in comment I request) added corresponding labels for the pull request.
   - [ ] I have passed maven check locally : `mvn clean install -B -T2C -DskipTests -Dmaven.javadoc.skip=true -e`.
   - [ ] I have made corresponding changes to the documentation.
   - [ ] I have added corresponding unit tests for my changes.
   


-- 
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] sandynz commented on a diff in pull request #22013: Improve migration IT

Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #22013:
URL: https://github.com/apache/shardingsphere/pull/22013#discussion_r1016478579


##########
test/integration-test/scaling/src/test/java/org/apache/shardingsphere/integration/data/pipeline/cases/migration/general/MySQLMigrationGeneralIT.java:
##########
@@ -86,20 +91,27 @@ public void assertMigrationSuccess() throws SQLException, InterruptedException {
         createTargetOrderTableRule();
         createTargetOrderTableEncryptRule();
         createTargetOrderItemTableRule();
-        AutoIncrementKeyGenerateAlgorithm keyGenerateAlgorithm = new AutoIncrementKeyGenerateAlgorithm();
+        AutoIncrementKeyGenerateAlgorithm orderKeyGenerate = new AutoIncrementKeyGenerateAlgorithm();
+        AutoIncrementKeyGenerateAlgorithm orderItemKeyGenerate = new AutoIncrementKeyGenerateAlgorithm();
         JdbcTemplate jdbcTemplate = new JdbcTemplate(getSourceDataSource());
-        Pair<List<Object[]>, List<Object[]>> dataPair = ScalingCaseHelper.generateFullInsertData(keyGenerateAlgorithm, parameterized.getDatabaseType(), 3000);
+        Pair<List<Object[]>, List<Object[]>> dataPair = ScalingCaseHelper.generateFullInsertData(orderKeyGenerate, orderItemKeyGenerate, parameterized.getDatabaseType(), 3000);
         log.info("init data begin: {}", LocalDateTime.now());
         jdbcTemplate.batchUpdate(getExtraSQLCommand().getFullInsertOrder(getSourceTableOrderName()), dataPair.getLeft());
         jdbcTemplate.batchUpdate(getExtraSQLCommand().getFullInsertOrderItem(), dataPair.getRight());
         log.info("init data end: {}", LocalDateTime.now());
         startMigration(getSourceTableOrderName(), getTargetTableOrderName());
         startMigration("t_order_item", "t_order_item");
-        // TODO wait preparation done (binlog position got), else insert/update/delete might be consumed in inventory task
-        startIncrementTask(new MySQLIncrementTask(jdbcTemplate, getSourceTableOrderName(), keyGenerateAlgorithm, 30));
         String orderJobId = getJobIdByTableName(getSourceTableOrderName());
-        String orderItemJobId = getJobIdByTableName("t_order_item");
+        for (int i = 0; i < 5; i++) {
+            List<Map<String, Object>> jobStatus = waitIncrementTaskFinished(String.format("SHOW MIGRATION STATUS '%s'", orderJobId));
+            Set<String> statusSet = jobStatus.stream().map(each -> String.valueOf(each.get("status"))).collect(Collectors.toSet());
+            if (statusSet.contains(JobStatus.PREPARING.name())) {
+                ThreadUtil.sleep(2, TimeUnit.SECONDS);
+            }
+        }

Review Comment:
   1, Seems job status might be RUNNING or empty, sleep might not take effect.
   
   2, Could we extract this code block to super class for common usage?
   



##########
test/integration-test/scaling/src/test/java/org/apache/shardingsphere/integration/data/pipeline/framework/helper/ScalingCaseHelper.java:
##########
@@ -53,19 +53,21 @@ public static long generateSnowflakeKey() {
      * Generate MySQL insert data, contains full fields.
      *
      * @param orderIdGenerate order id generate algorithm
+     * @param orderItemIdGenerate order item id generate algorithm
      * @param databaseType database type
      * @param insertRows insert rows
      * @return insert data list
      */
-    public static Pair<List<Object[]>, List<Object[]>> generateFullInsertData(final AutoIncrementKeyGenerateAlgorithm orderIdGenerate, final DatabaseType databaseType, final int insertRows) {
+    public static Pair<List<Object[]>, List<Object[]>> generateFullInsertData(final AutoIncrementKeyGenerateAlgorithm orderIdGenerate, final AutoIncrementKeyGenerateAlgorithm orderItemIdGenerate, 
+                                                                              final DatabaseType databaseType, final int insertRows) {

Review Comment:
   Could we keep one generateAlgorithm parameter, or just remove generateAlgorithm parameter?



-- 
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] sandynz commented on pull request #22013: Improve migration IT

Posted by GitBox <gi...@apache.org>.
sandynz commented on PR #22013:
URL: https://github.com/apache/shardingsphere/pull/22013#issuecomment-1307038289

   The PR title could be more detailed


-- 
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] sandynz merged pull request #22013: Improve migration IT, use a separate key generate and add wait job prepare success check method

Posted by GitBox <gi...@apache.org>.
sandynz merged PR #22013:
URL: https://github.com/apache/shardingsphere/pull/22013


-- 
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] sandynz commented on a diff in pull request #22013: Improve migration IT, use a separate key generate and add wait job prepare success check method

Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #22013:
URL: https://github.com/apache/shardingsphere/pull/22013#discussion_r1016583271


##########
test/integration-test/scaling/src/test/java/org/apache/shardingsphere/integration/data/pipeline/cases/base/BaseITCase.java:
##########
@@ -224,6 +225,16 @@ protected void proxyExecuteWithLog(final String sql, final int sleepSeconds) thr
         ThreadUtil.sleep(Math.max(sleepSeconds, 0), TimeUnit.SECONDS);
     }
     
+    protected void waitJobPrepareSuccess(final String distSQL) {
+        for (int i = 0; i < 5; i++) {
+            List<Map<String, Object>> jobStatus = queryForListWithLog(distSQL);
+            Set<String> statusSet = jobStatus.stream().map(each -> String.valueOf(each.get("status"))).collect(Collectors.toSet());
+            if (statusSet.contains(JobStatus.PREPARING.name()) || statusSet.contains(JobStatus.RUNNING.name()) || statusSet.contains(JobStatus.PREPARE_SUCCESS.name())) {
+                ThreadUtil.sleep(2, TimeUnit.SECONDS);
+            }

Review Comment:
   Seems it does not need to sleep when job status is `PREPARE_SUCCESS`



-- 
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