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/10 02:51:18 UTC
[GitHub] [shardingsphere] azexcy opened a new pull request, #22048: data consistency check job add position, support start execution from the middle of the task
azexcy opened a new pull request, #22048:
URL: https://github.com/apache/shardingsphere/pull/22048
Changes proposed in this pull request:
- Add table check position
- 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 #22048: Data consistency check job support breakpoint resume transmission
Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#discussion_r1019054039
##########
test/pipeline/src/test/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/DataMatchDataConsistencyCalculateAlgorithmTest.java:
##########
@@ -80,37 +82,43 @@ private void createTableAndInitData(final PipelineDataSourceWrapper dataSource,
}
}
+ @SuppressWarnings("unchecked")
@Test
- public void assertCalculateFromBegin() throws NoSuchFieldException, IllegalAccessException {
+ public void assertCalculateFromBegin() throws NoSuchFieldException, IllegalAccessException, InvocationTargetException, NoSuchMethodException {
DataMatchDataConsistencyCalculateAlgorithm calculateAlgorithm = new DataMatchDataConsistencyCalculateAlgorithm();
ReflectionUtil.setFieldValue(calculateAlgorithm, "chunkSize", 5);
DataConsistencyCalculateParameter sourceParameter = generateParameter(source, "t_order_copy", null);
- Optional<DataConsistencyCalculatedResult> sourceCalculateResult = calculateAlgorithm.calculateChunk(sourceParameter);
+ Optional<DataConsistencyCalculatedResult> sourceCalculateResult = (Optional<DataConsistencyCalculatedResult>) ReflectionUtil.invokeMethod(calculateAlgorithm, "calculateChunk",
+ new Class[]{DataConsistencyCalculateParameter.class}, new Object[]{sourceParameter});
Review Comment:
Why change the test class package and replace `calculateChunk` invocation to reflection
--
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 #22048: Data consistency check job support breakpoint resume transmission
Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#discussion_r1019799359
##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/util/ReflectionUtil.java:
##########
@@ -92,4 +92,33 @@ public static Object invokeMethod(final Object target, final String methodName,
method.setAccessible(true);
return method.invoke(target, parameterValues);
}
+
+ /**
+ * invoke method in parent class.
+ *
+ * @param target target object
+ * @param methodName method name
+ * @param parameterTypes parameter types
+ * @param parameterValues parameter values
+ * @return invoke method result.
+ * @throws NoSuchMethodException no such field exception
+ * @throws InvocationTargetException invocation target exception
+ * @throws IllegalAccessException illegal access exception
+ */
+ public static Object invokeMethodInParentClass(final Object target, final String methodName, final Class<?>[] parameterTypes,
+ final Object[] parameterValues) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
Review Comment:
OK
--
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] azexcy commented on a diff in pull request #22048: Data consistency check job support breakpoint resume transmission
Posted by GitBox <gi...@apache.org>.
azexcy commented on code in PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#discussion_r1018752464
##########
test/integration-test/scaling/src/test/java/org/apache/shardingsphere/integration/data/pipeline/cases/migration/primarykey/TextPrimaryKeyMigrationIT.java:
##########
@@ -92,11 +92,8 @@ public void assertTextPrimaryMigrationSuccess() throws SQLException, Interrupted
sourceExecuteWithLog(String.format("INSERT INTO %s (order_id,user_id,status) VALUES (%s, %s, '%s')", getSourceTableOrderName(), "1000000000", 1, "afterStop"));
waitIncrementTaskFinished(String.format("SHOW MIGRATION STATUS '%s'", jobId));
// TODO The ordering of primary or unique keys for text types is different, but can't reproduce now
- if (DatabaseTypeUtil.isMySQL(getDatabaseType())) {
- assertCheckMigrationSuccess(jobId, "DATA_MATCH");
- } else {
- assertCheckMigrationSuccess(jobId, "DATA_MATCH");
- }
+ proxyExecuteWithLog(String.format("CHECK MIGRATION '%s' BY TYPE (NAME='%s')", jobId, "DATA_MATCH"), 2);
Review Comment:
Because of the addition of case in the unit test, IT will not be modified, rollback it.
--
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 #22048: data consistency check job add position, support start execution from the middle of the task
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#issuecomment-1309716710
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/22048?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 [#22048](https://codecov.io/gh/apache/shardingsphere/pull/22048?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0b64c37) into [master](https://codecov.io/gh/apache/shardingsphere/commit/09f5bf5825aec49d96663bf6f80909f56354ef8e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (09f5bf5) will **decrease** coverage by `0.01%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #22048 +/- ##
============================================
- Coverage 61.01% 60.99% -0.02%
Complexity 2548 2548
============================================
Files 4109 4108 -1
Lines 57241 57252 +11
Branches 9683 9688 +5
============================================
Hits 34923 34923
- Misses 19371 19382 +11
Partials 2947 2947
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/22048?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...on/DatetimeConfigurationFileNotFoundException.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-aW5mcmEvZGF0ZXRpbWUvdHlwZS9kYXRhYmFzZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0ZXRpbWUvZGF0YWJhc2UvZXhjZXB0aW9uL0RhdGV0aW1lQ29uZmlndXJhdGlvbkZpbGVOb3RGb3VuZEV4Y2VwdGlvbi5qYXZh) | `0.00% <ø> (ø)` | |
| [...eator/ShardingSpherePipelineDataSourceCreator.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-amRiYy9jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kcml2ZXIvZGF0YS9waXBlbGluZS9kYXRhc291cmNlL2NyZWF0b3IvU2hhcmRpbmdTcGhlcmVQaXBlbGluZURhdGFTb3VyY2VDcmVhdG9yLmphdmE=) | `0.00% <ø> (ø)` | |
| [...cy/SingleTableInventoryDataConsistencyChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jb3JlL2NoZWNrL2NvbnNpc3RlbmN5L1NpbmdsZVRhYmxlSW52ZW50b3J5RGF0YUNvbnNpc3RlbmN5Q2hlY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...m/CRC32MatchDataConsistencyCalculateAlgorithm.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jb3JlL2NoZWNrL2NvbnNpc3RlbmN5L2FsZ29yaXRobS9DUkMzMk1hdGNoRGF0YUNvbnNpc3RlbmN5Q2FsY3VsYXRlQWxnb3JpdGhtLmphdmE=) | `47.61% <0.00%> (-1.17%)` | :arrow_down: |
| [...hm/DataMatchDataConsistencyCalculateAlgorithm.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jb3JlL2NoZWNrL2NvbnNpc3RlbmN5L2FsZ29yaXRobS9EYXRhTWF0Y2hEYXRhQ29uc2lzdGVuY3lDYWxjdWxhdGVBbGdvcml0aG0uamF2YQ==) | `36.00% <0.00%> (-1.50%)` | :arrow_down: |
| [...urce/creator/PipelineDataSourceCreatorFactory.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jb3JlL2RhdGFzb3VyY2UvY3JlYXRvci9QaXBlbGluZURhdGFTb3VyY2VDcmVhdG9yRmFjdG9yeS5qYXZh) | `75.00% <ø> (ø)` | |
| [...reator/impl/StandardPipelineDataSourceCreator.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jb3JlL2RhdGFzb3VyY2UvY3JlYXRvci9pbXBsL1N0YW5kYXJkUGlwZWxpbmVEYXRhU291cmNlQ3JlYXRvci5qYXZh) | `100.00% <ø> (ø)` | |
| [...ress/yaml/YamlConsistencyCheckJobItemProgress.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jb3JlL2pvYi9wcm9ncmVzcy95YW1sL1lhbWxDb25zaXN0ZW5jeUNoZWNrSm9iSXRlbVByb2dyZXNzLmphdmE=) | `0.00% <ø> (ø)` | |
| [...ml/YamlConsistencyCheckJobItemProgressSwapper.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9jb3JlL2pvYi9wcm9ncmVzcy95YW1sL1lhbWxDb25zaXN0ZW5jeUNoZWNrSm9iSXRlbVByb2dyZXNzU3dhcHBlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...scenario/consistencycheck/ConsistencyCheckJob.java](https://codecov.io/gh/apache/shardingsphere/pull/22048/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-a2VybmVsL2RhdGEtcGlwZWxpbmUvY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9zY2VuYXJpby9jb25zaXN0ZW5jeWNoZWNrL0NvbnNpc3RlbmN5Q2hlY2tKb2IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/apache/shardingsphere/pull/22048/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
[GitHub] [shardingsphere] azexcy commented on a diff in pull request #22048: Data consistency check job support breakpoint resume transmission
Posted by GitBox <gi...@apache.org>.
azexcy commented on code in PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#discussion_r1019765099
##########
test/pipeline/src/test/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/DataMatchDataConsistencyCalculateAlgorithmTest.java:
##########
@@ -80,37 +82,43 @@ private void createTableAndInitData(final PipelineDataSourceWrapper dataSource,
}
}
+ @SuppressWarnings("unchecked")
@Test
- public void assertCalculateFromBegin() throws NoSuchFieldException, IllegalAccessException {
+ public void assertCalculateFromBegin() throws NoSuchFieldException, IllegalAccessException, InvocationTargetException, NoSuchMethodException {
DataMatchDataConsistencyCalculateAlgorithm calculateAlgorithm = new DataMatchDataConsistencyCalculateAlgorithm();
ReflectionUtil.setFieldValue(calculateAlgorithm, "chunkSize", 5);
DataConsistencyCalculateParameter sourceParameter = generateParameter(source, "t_order_copy", null);
- Optional<DataConsistencyCalculatedResult> sourceCalculateResult = calculateAlgorithm.calculateChunk(sourceParameter);
+ Optional<DataConsistencyCalculatedResult> sourceCalculateResult = (Optional<DataConsistencyCalculatedResult>) ReflectionUtil.invokeMethod(calculateAlgorithm, "calculateChunk",
+ new Class[]{DataConsistencyCalculateParameter.class}, new Object[]{sourceParameter});
Review Comment:
moved
--
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] azexcy commented on a diff in pull request #22048: Data consistency check job support breakpoint resume transmission
Posted by GitBox <gi...@apache.org>.
azexcy commented on code in PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#discussion_r1019779994
##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/util/ReflectionUtil.java:
##########
@@ -92,4 +92,33 @@ public static Object invokeMethod(final Object target, final String methodName,
method.setAccessible(true);
return method.invoke(target, parameterValues);
}
+
+ /**
+ * invoke method in parent class.
+ *
+ * @param target target object
+ * @param methodName method name
+ * @param parameterTypes parameter types
+ * @param parameterValues parameter values
+ * @return invoke method result.
+ * @throws NoSuchMethodException no such field exception
+ * @throws InvocationTargetException invocation target exception
+ * @throws IllegalAccessException illegal access exception
+ */
+ public static Object invokeMethodInParentClass(final Object target, final String methodName, final Class<?>[] parameterTypes,
+ final Object[] parameterValues) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
Review Comment:
`ReflectionUtil` only used for test, but if move it to `src/test` and using `test-jar`, need to add dependencies in multiple modules, so I just add unit test now
--
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 #22048: data consistency check job add position, support start execution from the middle of the task
Posted by GitBox <gi...@apache.org>.
sandynz commented on PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#issuecomment-1309828180
More unit tests could be added, since it might be not verified in integration test.
--
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 #22048: Data consistency check job support breakpoint resume transmission
Posted by GitBox <gi...@apache.org>.
sandynz merged PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048
--
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] azexcy commented on a diff in pull request #22048: Data consistency check job support breakpoint resume transmission
Posted by GitBox <gi...@apache.org>.
azexcy commented on code in PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#discussion_r1019762290
##########
test/pipeline/src/test/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/DataMatchDataConsistencyCalculateAlgorithmTest.java:
##########
@@ -80,37 +82,43 @@ private void createTableAndInitData(final PipelineDataSourceWrapper dataSource,
}
}
+ @SuppressWarnings("unchecked")
@Test
- public void assertCalculateFromBegin() throws NoSuchFieldException, IllegalAccessException {
+ public void assertCalculateFromBegin() throws NoSuchFieldException, IllegalAccessException, InvocationTargetException, NoSuchMethodException {
DataMatchDataConsistencyCalculateAlgorithm calculateAlgorithm = new DataMatchDataConsistencyCalculateAlgorithm();
ReflectionUtil.setFieldValue(calculateAlgorithm, "chunkSize", 5);
DataConsistencyCalculateParameter sourceParameter = generateParameter(source, "t_order_copy", null);
- Optional<DataConsistencyCalculatedResult> sourceCalculateResult = calculateAlgorithm.calculateChunk(sourceParameter);
+ Optional<DataConsistencyCalculatedResult> sourceCalculateResult = (Optional<DataConsistencyCalculatedResult>) ReflectionUtil.invokeMethod(calculateAlgorithm, "calculateChunk",
+ new Class[]{DataConsistencyCalculateParameter.class}, new Object[]{sourceParameter});
Review Comment:
<img width="1090" alt="image" src="https://user-images.githubusercontent.com/101622833/201244666-d14d3589-ad53-4746-90a4-633e53d5b15a.png">
Because `calculateChunk ` is protected, pipeline test can't be called directly method
--
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 #22048: Data consistency check job support breakpoint resume transmission
Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#discussion_r1019775307
##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/util/ReflectionUtil.java:
##########
@@ -92,4 +92,33 @@ public static Object invokeMethod(final Object target, final String methodName,
method.setAccessible(true);
return method.invoke(target, parameterValues);
}
+
+ /**
+ * invoke method in parent class.
Review Comment:
Java doc should start with uppercase character
##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/util/ReflectionUtil.java:
##########
@@ -92,4 +92,33 @@ public static Object invokeMethod(final Object target, final String methodName,
method.setAccessible(true);
return method.invoke(target, parameterValues);
}
+
+ /**
+ * invoke method in parent class.
+ *
+ * @param target target object
+ * @param methodName method name
+ * @param parameterTypes parameter types
+ * @param parameterValues parameter values
+ * @return invoke method result.
+ * @throws NoSuchMethodException no such field exception
+ * @throws InvocationTargetException invocation target exception
+ * @throws IllegalAccessException illegal access exception
+ */
+ public static Object invokeMethodInParentClass(final Object target, final String methodName, final Class<?>[] parameterTypes,
+ final Object[] parameterValues) throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {
Review Comment:
`invokeMethodInParentClass` is just used in unit test, could we put it to test package?
Or else could we add unit test for it
--
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 #22048: data consistency check job add position, support start execution from the middle of the task
Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #22048:
URL: https://github.com/apache/shardingsphere/pull/22048#discussion_r1018624536
##########
kernel/data-pipeline/api/src/main/java/org/apache/shardingsphere/data/pipeline/api/job/progress/ConsistencyCheckJobItemProgress.java:
##########
@@ -18,28 +18,34 @@
package org.apache.shardingsphere.data.pipeline.api.job.progress;
import lombok.Getter;
+import lombok.RequiredArgsConstructor;
import lombok.Setter;
import lombok.ToString;
import org.apache.shardingsphere.data.pipeline.api.job.JobStatus;
+import java.util.Map;
+
/**
* Data consistency check job item progress.
*/
// TODO use final for fields
@Getter
-@Setter
Review Comment:
`TODO` could be removed
##########
kernel/data-pipeline/api/src/main/java/org/apache/shardingsphere/data/pipeline/api/check/consistency/DataConsistencyCalculatedResult.java:
##########
@@ -28,4 +28,11 @@ public interface DataConsistencyCalculatedResult {
* @return records count
*/
int getRecordsCount();
+
+ /**
+ * Get max unique value.
+ *
+ * @return max unique value
+ */
+ Object getMaxUniqueKeyValue();
Review Comment:
Java doc should be consistent with method name
##########
kernel/data-pipeline/api/src/main/java/org/apache/shardingsphere/data/pipeline/api/check/consistency/DataConsistencyCalculateParameter.java:
##########
@@ -73,4 +73,9 @@ public final class DataConsistencyCalculateParameter {
* Previous calculated result will be transferred to next call.
*/
private volatile Object previousCalculatedResult;
+
+ /**
+ * The position value of data check.
+ */
+ private final Object dataCheckPositionValue;
Review Comment:
`dataCheckPositionValue` could be `tableCheckPosition`, keep it consistency with other places
##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/core/check/consistency/SingleTableInventoryDataConsistencyChecker.java:
##########
@@ -74,7 +75,7 @@ public final class SingleTableInventoryDataConsistencyChecker {
private final JobRateLimitAlgorithm readRateLimitAlgorithm;
- private final PipelineJobProgressListener jobProgressListener;
+ private final ConsistencyCheckJobItemContext consistencyCheckJobItemContext;
Review Comment:
`consistencyCheckJobItemContext` could be `jobItemContext`
##########
test/integration-test/scaling/src/test/java/org/apache/shardingsphere/integration/data/pipeline/cases/migration/primarykey/TextPrimaryKeyMigrationIT.java:
##########
@@ -92,11 +92,8 @@ public void assertTextPrimaryMigrationSuccess() throws SQLException, Interrupted
sourceExecuteWithLog(String.format("INSERT INTO %s (order_id,user_id,status) VALUES (%s, %s, '%s')", getSourceTableOrderName(), "1000000000", 1, "afterStop"));
waitIncrementTaskFinished(String.format("SHOW MIGRATION STATUS '%s'", jobId));
// TODO The ordering of primary or unique keys for text types is different, but can't reproduce now
- if (DatabaseTypeUtil.isMySQL(getDatabaseType())) {
- assertCheckMigrationSuccess(jobId, "DATA_MATCH");
- } else {
- assertCheckMigrationSuccess(jobId, "DATA_MATCH");
- }
+ proxyExecuteWithLog(String.format("CHECK MIGRATION '%s' BY TYPE (NAME='%s')", jobId, "DATA_MATCH"), 2);
Review Comment:
There're several repeated CHECK MIGRATION DistSQL construction, it's better to extract it in super class
##########
kernel/data-pipeline/api/src/main/java/org/apache/shardingsphere/data/pipeline/api/check/consistency/DataConsistencyCalculatedResult.java:
##########
@@ -28,4 +28,11 @@ public interface DataConsistencyCalculatedResult {
* @return records count
*/
int getRecordsCount();
+
+ /**
+ * Get max unique value.
+ *
+ * @return max unique value
+ */
+ Object getMaxUniqueKeyValue();
Review Comment:
Looks it's not implemented in CRC32_MATCH, could we return `Optional<Object>`?
--
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