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/09/16 09:18:50 UTC

[GitHub] [shardingsphere] azexcy opened a new pull request, #21019: Show migration status add processed records column

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

   
   Changes proposed in this pull request:
     - Show migration status add processed records column
     - Add IT case
   
   ---
   
   Before committing this PR, I'm sure that I have checked the following options:
   - [x] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
   - [x] I have self-reviewed the commit code.
   - [x] I have triggered maven check: `mvn clean install -B -T2C -DskipTests -Dmaven.javadoc.skip=true -e`.
   - [x] I have made corresponding changes to the documentation.
   - [x] 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 #21019: Show migration status add processed records column

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


##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/context/InventoryIncrementalJobItemContext.java:
##########
@@ -44,4 +45,11 @@ public interface InventoryIncrementalJobItemContext extends PipelineJobItemConte
      * @return incremental tasks
      */
     Collection<IncrementalTask> getIncrementalTasks();
+    
+    /**
+     * Get processed record count.
+     *
+     * @return processed record count.
+     */
+    long getProcessedRecordCount();

Review Comment:
   Could we change it to `getProcessedRecordsCount`?



##########
shardingsphere-features/shardingsphere-sharding/shardingsphere-sharding-distsql/shardingsphere-sharding-distsql-handler/src/main/java/org/apache/shardingsphere/migration/distsql/handler/query/ShowMigrationJobStatusQueryResultSet.java:
##########
@@ -60,14 +61,15 @@ public void init(final ShardingSphereDatabase database, final SQLStatement sqlSt
                         result.add("");
                         result.add("");
                         result.add("");
+                        result.add("");
                     }
                     return result;
                 }).collect(Collectors.toList()).iterator();
     }
     
     @Override
     public Collection<String> getColumnNames() {
-        return Arrays.asList("item", "data_source", "status", "active", "inventory_finished_percentage", "incremental_idle_seconds");
+        return Arrays.asList("item", "data_source", "status", "active", "processed_records", "inventory_finished_percentage", "incremental_idle_seconds");

Review Comment:
   Could we use `processed_records_count`? Keep consistency with variable



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-api/src/main/java/org/apache/shardingsphere/data/pipeline/api/job/progress/listener/PipelineJobProcessUpdateParameter.java:
##########
@@ -15,24 +15,19 @@
  * limitations under the License.
  */
 
-package org.apache.shardingsphere.data.pipeline.core.job.progress.listener;
+package org.apache.shardingsphere.data.pipeline.api.job.progress.listener;
 
+import lombok.Getter;
 import lombok.RequiredArgsConstructor;
-import org.apache.shardingsphere.data.pipeline.api.job.progress.listener.PipelineJobProgressListener;
-import org.apache.shardingsphere.data.pipeline.core.job.progress.persist.PipelineJobProgressPersistService;
 
 /**
- * Default pipeline job progress listener implementation.
+ * Pipeline job process update parameter.
  */
 @RequiredArgsConstructor
-public final class DefaultPipelineJobProgressListener implements PipelineJobProgressListener {
+@Getter
+public final class PipelineJobProcessUpdateParameter {

Review Comment:
   Could we rename class name `PipelineJobProcessUpdateParameter` to `PipelineJobProgressUpdatedParameter`? Similar to method name `onProgressUpdated`



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/job/progress/yaml/YamlInventoryIncrementalJobItemProgress.java:
##########
@@ -37,4 +37,6 @@ public final class YamlInventoryIncrementalJobItemProgress implements YamlConfig
     private YamlJobItemInventoryTasksProgress inventory;
     
     private YamlJobItemIncrementalTasksProgress incremental;
+    
+    private long processedRecordCount;

Review Comment:
   processedRecordsCount too, and also yaml class



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-api/src/main/java/org/apache/shardingsphere/data/pipeline/api/job/progress/InventoryIncrementalJobItemProgress.java:
##########
@@ -39,4 +39,6 @@ public final class InventoryIncrementalJobItemProgress implements PipelineJobIte
     private JobItemInventoryTasksProgress inventory;
     
     private JobItemIncrementalTasksProgress incremental;
+    
+    private long processedRecordCount;

Review Comment:
   Could use `processedRecordsCount` to replace `processedRecordCount`?



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-api/src/main/java/org/apache/shardingsphere/data/pipeline/api/job/progress/listener/PipelineJobProcessUpdateParameter.java:
##########
@@ -15,24 +15,19 @@
  * limitations under the License.
  */
 
-package org.apache.shardingsphere.data.pipeline.core.job.progress.listener;
+package org.apache.shardingsphere.data.pipeline.api.job.progress.listener;
 
+import lombok.Getter;
 import lombok.RequiredArgsConstructor;
-import org.apache.shardingsphere.data.pipeline.api.job.progress.listener.PipelineJobProgressListener;
-import org.apache.shardingsphere.data.pipeline.core.job.progress.persist.PipelineJobProgressPersistService;
 
 /**
- * Default pipeline job progress listener implementation.
+ * Pipeline job process update parameter.
  */
 @RequiredArgsConstructor
-public final class DefaultPipelineJobProgressListener implements PipelineJobProgressListener {
+@Getter
+public final class PipelineJobProcessUpdateParameter {
     
-    private final String jobId;
+    private final int insertRecordNumber;
     
-    private final int shardingItem;
-    
-    @Override
-    public void onProgressUpdated() {
-        PipelineJobProgressPersistService.notifyPersist(jobId, shardingItem);
-    }
+    private final int deleteRecordNumber;

Review Comment:
   1, Could we replace `RecordNumber` to `RecordsCount`?
   
   2, insert and deleted could inserted and deleted
   
   



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/scenario/migration/MigrationJobItemContext.java:
##########
@@ -92,6 +97,9 @@ public MigrationJobItemContext(final MigrationJobConfiguration jobConfig, final
         this.shardingItem = shardingItem;
         this.dataSourceName = taskConfig.getDataSourceName();
         this.initProgress = initProgress;
+        if (null != initProgress) {
+            processedRecordCount.addAndGet(initProgress.getProcessedRecordCount());
+        }

Review Comment:
   It's better to use `set` instead of `addAndGet`



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/scenario/migration/MigrationJobItemContext.java:
##########
@@ -125,4 +133,16 @@ public PipelineTableMetaDataLoader getSourceMetaDataLoader() {
     public boolean isSourceTargetDatabaseTheSame() {
         return jobConfig.getSourceDatabaseType().equalsIgnoreCase(jobConfig.getTargetDatabaseType());
     }
+    
+    @Override
+    public void onProgressUpdated(final PipelineJobProcessUpdateParameter parameter) {
+        int needAddNumber = parameter.getInsertRecordNumber() - parameter.getDeleteRecordNumber();
+        processedRecordCount.addAndGet(needAddNumber);
+        PipelineJobProgressPersistService.notifyPersist(jobId, shardingItem);
+    }
+    
+    @Override
+    public long getProcessedRecordCount() {
+        return processedRecordCount.get();
+    }

Review Comment:
   processedRecordsCount too



-- 
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 #21019: Show migration status add processed records column

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


##########
shardingsphere-test/shardingsphere-integration-test/shardingsphere-integration-test-scaling/src/test/java/org/apache/shardingsphere/integration/data/pipeline/cases/migration/general/MySQLMigrationGeneralIT.java:
##########
@@ -105,7 +107,10 @@ public void assertMigrationSuccess() throws SQLException, InterruptedException {
     }
     
     private void assertMigrationSuccessById(final String jobId) throws SQLException, InterruptedException {
-        waitJobFinished(String.format("SHOW MIGRATION STATUS '%s'", jobId));
+        List<Map<String, Object>> jobStatus = waitJobFinished(String.format("SHOW MIGRATION STATUS '%s'", jobId));
+        for (Map<String, Object> each : jobStatus) {
+            assertTrue(Integer.parseInt(each.get("processed_records").toString()) > 0);
+        }

Review Comment:
   `processed_records` not changed, it caused NPE



-- 
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 #21019: Show migration status add processed_records_count column

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


-- 
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] codecov-commenter commented on pull request #21019: Show migration status add processed records column

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #21019:
URL: https://github.com/apache/shardingsphere/pull/21019#issuecomment-1249237721

   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/21019?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 [#21019](https://codecov.io/gh/apache/shardingsphere/pull/21019?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e8c8410) into [master](https://codecov.io/gh/apache/shardingsphere/commit/3d2d99d6e18bd726fac465a35a18de0f73f55635?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3d2d99d) will **decrease** coverage by `0.15%`.
   > The diff coverage is `9.09%`.
   
   > :exclamation: Current head e8c8410 differs from pull request most recent head 9370bd6. Consider uploading reports for the commit 9370bd6 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #21019      +/-   ##
   ============================================
   - Coverage     61.68%   61.52%   -0.16%     
     Complexity     2469     2469              
   ============================================
     Files          4051     4054       +3     
     Lines         55587    55725     +138     
     Branches       9416     9446      +30     
   ============================================
   - Hits          34287    34284       -3     
   - Misses        18423    18568     +145     
   + Partials       2877     2873       -4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/21019?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...er/query/ShowMigrationJobStatusQueryResultSet.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctZGlzdHNxbC9zaGFyZGluZ3NwaGVyZS1zaGFyZGluZy1kaXN0c3FsLWhhbmRsZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL21pZ3JhdGlvbi9kaXN0c3FsL2hhbmRsZXIvcXVlcnkvU2hvd01pZ3JhdGlvbkpvYlN0YXR1c1F1ZXJ5UmVzdWx0U2V0LmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...infra/datasource/state/DataSourceStateManager.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9kYXRhc291cmNlL3N0YXRlL0RhdGFTb3VyY2VTdGF0ZU1hbmFnZXIuamF2YQ==) | `10.86% <0.00%> (-0.50%)` | :arrow_down: |
   | [.../progress/InventoryIncrementalJobItemProgress.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvYXBpL2pvYi9wcm9ncmVzcy9JbnZlbnRvcnlJbmNyZW1lbnRhbEpvYkl0ZW1Qcm9ncmVzcy5qYXZh) | `0.00% <ø> (ø)` | |
   | [...e/api/impl/InventoryIncrementalJobItemAPIImpl.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvYXBpL2ltcGwvSW52ZW50b3J5SW5jcmVtZW50YWxKb2JJdGVtQVBJSW1wbC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../exception/job/PrepareJobWithoutUserException.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvZXhjZXB0aW9uL2pvYi9QcmVwYXJlSm9iV2l0aG91dFVzZXJFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...e/data/pipeline/core/importer/DefaultImporter.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvaW1wb3J0ZXIvRGVmYXVsdEltcG9ydGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../yaml/YamlInventoryIncrementalJobItemProgress.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvam9iL3Byb2dyZXNzL3lhbWwvWWFtbEludmVudG9yeUluY3JlbWVudGFsSm9iSXRlbVByb2dyZXNzLmphdmE=) | `0.00% <ø> (ø)` | |
   | [...amlInventoryIncrementalJobItemProgressSwapper.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvam9iL3Byb2dyZXNzL3lhbWwvWWFtbEludmVudG9yeUluY3JlbWVudGFsSm9iSXRlbVByb2dyZXNzU3dhcHBlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...a/pipeline/core/prepare/InventoryTaskSplitter.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvcHJlcGFyZS9JbnZlbnRvcnlUYXNrU3BsaXR0ZXIuamF2YQ==) | `0.00% <ø> (ø)` | |
   | [...ne/scenario/migration/MigrationJobItemContext.java](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL3NjZW5hcmlvL21pZ3JhdGlvbi9NaWdyYXRpb25Kb2JJdGVtQ29udGV4dC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [24 more](https://codecov.io/gh/apache/shardingsphere/pull/21019/diff?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