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/05/19 06:50:13 UTC
[GitHub] [shardingsphere] azexcy opened a new pull request, #17801: Async execute pipeline job task,avoid blocking zk events
azexcy opened a new pull request, #17801:
URL: https://github.com/apache/shardingsphere/pull/17801
Changes proposed in this pull request:
- Async execute pipeline job task,avoid blocking zk events
- Add lock log at scaling 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 #17801: Async execute pipeline job task,avoid blocking zk events
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #17801:
URL: https://github.com/apache/shardingsphere/pull/17801#issuecomment-1131553206
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/17801?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 [#17801](https://codecov.io/gh/apache/shardingsphere/pull/17801?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47272c8) into [master](https://codecov.io/gh/apache/shardingsphere/commit/0aa12438fc2ae0b38ca1c5fc63a1157c4e78e361?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0aa1243) will **decrease** coverage by `0.01%`.
> The diff coverage is `1.63%`.
> :exclamation: Current head 47272c8 differs from pull request most recent head 9d57b2c. Consider uploading reports for the commit 9d57b2c to get more accurate results
```diff
@@ Coverage Diff @@
## master #17801 +/- ##
============================================
- Coverage 59.12% 59.10% -0.02%
Complexity 2130 2130
============================================
Files 3594 3595 +1
Lines 53416 53425 +9
Branches 9119 9118 -1
============================================
- Hits 31580 31579 -1
- Misses 19167 19176 +9
- Partials 2669 2670 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/17801?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ere/agent/plugin/tracing/ProxyContextRestorer.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUtYWdlbnQvc2hhcmRpbmdzcGhlcmUtYWdlbnQtcGx1Z2lucy9zaGFyZGluZ3NwaGVyZS1hZ2VudC1wbHVnaW4tdHJhY2luZy9zaGFyZGluZ3NwaGVyZS1hZ2VudC10cmFjaW5nLXRlc3Qvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2FnZW50L3BsdWdpbi90cmFjaW5nL1Byb3h5Q29udGV4dFJlc3RvcmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ata/pipeline/core/execute/PipelineJobExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvZXhlY3V0ZS9QaXBlbGluZUpvYkV4ZWN1dG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../core/metadata/generator/PipelineDDLGenerator.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvbWV0YWRhdGEvZ2VuZXJhdG9yL1BpcGVsaW5lRERMR2VuZXJhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...e/scenario/rulealtered/RuleAlteredJobPreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL3NjZW5hcmlvL3J1bGVhbHRlcmVkL1J1bGVBbHRlcmVkSm9iUHJlcGFyZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../opengauss/ddlgenerator/OpenGaussDDLGenerator.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1kaWFsZWN0L3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUtb3BlbmdhdXNzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL29wZW5nYXVzcy9kZGxnZW5lcmF0b3IvT3BlbkdhdXNzRERMR2VuZXJhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...cluster/coordinator/lock/mutex/InterMutexLock.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUtbW9kZS9zaGFyZGluZ3NwaGVyZS1tb2RlLXR5cGUvc2hhcmRpbmdzcGhlcmUtY2x1c3Rlci1tb2RlL3NoYXJkaW5nc3BoZXJlLWNsdXN0ZXItbW9kZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9tb2RlL21hbmFnZXIvY2x1c3Rlci9jb29yZGluYXRvci9sb2NrL211dGV4L0ludGVyTXV0ZXhMb2NrLmphdmE=) | `3.52% <0.00%> (-0.13%)` | :arrow_down: |
| [...nd/text/admin/mysql/MySQLAdminExecutorCreator.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2FkbWluL215c3FsL015U1FMQWRtaW5FeGVjdXRvckNyZWF0b3IuamF2YQ==) | `47.61% <25.00%> (+1.58%)` | :arrow_up: |
| [...ntracing/service/OpenTracingPluginBootService.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUtYWdlbnQvc2hhcmRpbmdzcGhlcmUtYWdlbnQtcGx1Z2lucy9zaGFyZGluZ3NwaGVyZS1hZ2VudC1wbHVnaW4tdHJhY2luZy9zaGFyZGluZ3NwaGVyZS1hZ2VudC10cmFjaW5nLW9wZW50cmFjaW5nL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9hZ2VudC9wbHVnaW4vdHJhY2luZy9vcGVudHJhY2luZy9zZXJ2aWNlL09wZW5UcmFjaW5nUGx1Z2luQm9vdFNlcnZpY2UuamF2YQ==) | `61.53% <0.00%> (-7.70%)` | :arrow_down: |
| [...jaeger/service/JaegerTracingPluginBootService.java](https://codecov.io/gh/apache/shardingsphere/pull/17801/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-c2hhcmRpbmdzcGhlcmUtYWdlbnQvc2hhcmRpbmdzcGhlcmUtYWdlbnQtcGx1Z2lucy9zaGFyZGluZ3NwaGVyZS1hZ2VudC1wbHVnaW4tdHJhY2luZy9zaGFyZGluZ3NwaGVyZS1hZ2VudC10cmFjaW5nLWphZWdlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvYWdlbnQvcGx1Z2luL3RyYWNpbmcvamFlZ2VyL3NlcnZpY2UvSmFlZ2VyVHJhY2luZ1BsdWdpbkJvb3RTZXJ2aWNlLmphdmE=) | `63.15% <0.00%> (-5.27%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/17801?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/17801?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0aa1243...9d57b2c](https://codecov.io/gh/apache/shardingsphere/pull/17801?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] sandynz merged pull request #17801: Async execute pipeline job task,avoid blocking zk events
Posted by GitBox <gi...@apache.org>.
sandynz merged PR #17801:
URL: https://github.com/apache/shardingsphere/pull/17801
--
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 #17801: Async execute pipeline job task,avoid blocking zk events
Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #17801:
URL: https://github.com/apache/shardingsphere/pull/17801#discussion_r877049389
##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/execute/PipelineJobExecutor.java:
##########
@@ -112,15 +112,18 @@ private Optional<JobConfigurationPOJO> getJobConfigPOJO(final DataChangedEvent e
}
private void execute(final JobConfigurationPOJO jobConfigPOJO) {
- if (!RuleAlteredJobSchedulerCenter.existJob(jobConfigPOJO.getJobName())) {
+ RuleAlteredJobConfiguration jobConfig = RuleAlteredJobConfigurationSwapper.swapToObject(jobConfigPOJO.getJobParameter());
+ String databaseName = jobConfig.getDatabaseName();
+ if (PipelineSimpleLock.getInstance().tryLock(databaseName, 3000)) {
log.info("{} added to executing jobs success", jobConfigPOJO.getJobName());
new OneOffJobBootstrap(PipelineAPIFactory.getRegistryCenter(), new RuleAlteredJob(), jobConfigPOJO.toJobConfiguration()).execute();
} else {
- log.info("{} added to executing jobs failed since it already exists", jobConfigPOJO.getJobName());
+ log.info("tryLock failed, databaseName={}", databaseName);
}
}
@Override
protected void doStop() {
+ executor.shutdown();
Review Comment:
Maybe `shutdownNow` should be invoked.
--
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] TeslaCN commented on a diff in pull request #17801: Async execute pipeline job task,avoid blocking zk events
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on code in PR #17801:
URL: https://github.com/apache/shardingsphere/pull/17801#discussion_r876801395
##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/execute/PipelineJobExecutor.java:
##########
@@ -48,7 +50,7 @@ public final class PipelineJobExecutor extends AbstractLifecycleExecutor {
private static final Pattern CONFIG_PATTERN = Pattern.compile(DataPipelineConstants.DATA_PIPELINE_ROOT + "/(\\d{2}[0-9a-f]+)/config");
- private static final RuleAlteredJobConfigurationSwapper JOB_CONFIG_SWAPPER = new RuleAlteredJobConfigurationSwapper();
+ private final Executor executor = Executors.newFixedThreadPool(20);
Review Comment:
Do we need to consider shutting down 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 #17801: Async execute pipeline job task,avoid blocking zk events
Posted by GitBox <gi...@apache.org>.
sandynz commented on code in PR #17801:
URL: https://github.com/apache/shardingsphere/pull/17801#discussion_r876907204
##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/execute/PipelineJobExecutor.java:
##########
@@ -112,15 +109,22 @@ private Optional<JobConfigurationPOJO> getJobConfigPOJO(final DataChangedEvent e
}
private void execute(final JobConfigurationPOJO jobConfigPOJO) {
- if (!RuleAlteredJobSchedulerCenter.existJob(jobConfigPOJO.getJobName())) {
- log.info("{} added to executing jobs success", jobConfigPOJO.getJobName());
- new OneOffJobBootstrap(PipelineAPIFactory.getRegistryCenter(), new RuleAlteredJob(), jobConfigPOJO.toJobConfiguration()).execute();
+ RuleAlteredJobConfiguration jobConfig = RuleAlteredJobConfigurationSwapper.swapToObject(jobConfigPOJO.getJobParameter());
+ String databaseName = jobConfig.getDatabaseName();
+ if (PipelineSimpleLock.getInstance().tryLock(databaseName, 3000)) {
+ if (!RuleAlteredJobSchedulerCenter.existJob(jobConfigPOJO.getJobName())) {
Review Comment:
It's better to put `existJob` up on `tryLock` to reduce lock.
--
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 #17801: Async execute pipeline job task,avoid blocking zk events
Posted by GitBox <gi...@apache.org>.
azexcy commented on code in PR #17801:
URL: https://github.com/apache/shardingsphere/pull/17801#discussion_r876879629
##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/execute/PipelineJobExecutor.java:
##########
@@ -48,7 +50,7 @@ public final class PipelineJobExecutor extends AbstractLifecycleExecutor {
private static final Pattern CONFIG_PATTERN = Pattern.compile(DataPipelineConstants.DATA_PIPELINE_ROOT + "/(\\d{2}[0-9a-f]+)/config");
- private static final RuleAlteredJobConfigurationSwapper JOB_CONFIG_SWAPPER = new RuleAlteredJobConfigurationSwapper();
+ private final Executor executor = Executors.newFixedThreadPool(20);
Review Comment:
Yes,athough the thread is in the whole life cycle, but I think shutting down at `doStop` is better practice
--
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