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 2021/07/03 03:08:56 UTC

[GitHub] [shardingsphere] sandynz opened a new pull request #11123: Support SQL execution process delayed report or no report for better performance

sandynz opened a new pull request #11123:
URL: https://github.com/apache/shardingsphere/pull/11123


   Fixes #9568 part 7.
   
   Changes proposed in this pull request:
   - Add ExecuteProcessReportContext
   - Add process report lock for the same execution process report, with different execution unit
   - Add `show-process-list-no-report-threshold-millis` in `server.yaml`, default value: 100
   - Report situations: 1) if total execution time not reach threshold, then no report, 2) else, report will be delayed to threshold or later
   - Refactor `ExecuteProcessReporter.report` execution unit, use unified dataMap to report execution unit, to avoid thread local issue
   - Additional: Limit show processlist `Info` column maximum length to 100
   


-- 
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 #11123: Support SQL execution process delayed report or no report for better performance

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


   Close it for now, it will be done later if necessary.


-- 
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 closed pull request #11123: Support SQL execution process delayed report or no report for better performance

Posted by GitBox <gi...@apache.org>.
sandynz closed pull request #11123:
URL: https://github.com/apache/shardingsphere/pull/11123


   


-- 
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 change in pull request #11123: Support SQL execution process delayed report or no report for better performance

Posted by GitBox <gi...@apache.org>.
sandynz commented on a change in pull request #11123:
URL: https://github.com/apache/shardingsphere/pull/11123#discussion_r663509905



##########
File path: shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/process/subscriber/ProcessRegistrySubscriber.java
##########
@@ -68,8 +69,20 @@ public void loadShowProcessListData(final ShowProcessListRequestEvent event) {
      */
     @Subscribe
     public void reportExecuteProcessSummary(final ExecuteProcessSummaryReportEvent event) {
-        ExecuteProcessContext executeProcessContext = event.getExecuteProcessContext();
-        repository.persist(ProcessNode.getExecutionPath(executeProcessContext.getExecutionID()), YamlEngine.marshal(new YamlExecuteProcessContext(executeProcessContext)));
+        String executionID = event.getExecutionID();
+        synchronized (executionID) {
+            Map<String, Object> dataMap = event.getDataMap();
+            ExecuteProcessReportContext reportContext = (ExecuteProcessReportContext) dataMap.get(ExecuteProcessConstants.EXECUTE_ID.name());
+            if (null == reportContext) {
+                return;
+            }
+            YamlExecuteProcessContext yamlExecuteProcessContext = reportContext.getYamlExecuteProcessContext();
+            if (System.currentTimeMillis() - yamlExecuteProcessContext.getStartTimeMillis() <= reportContext.getShowProcessListNoReportThresholdMillis()) {
+                return;
+            }
+            repository.persist(ProcessNode.getExecutionPath(executionID), YamlEngine.marshal(yamlExecuteProcessContext));
+            reportContext.setReportToGovernanceDonePartially(true);

Review comment:
       In general, it's used to indicate whether later report of the same sql need to persist to governance or not.
   
   In details, there are 2 places will use `reportToGovernanceDonePartially`:
   - In `reportExecuteProcessUnit` (sharding sqls execution in progress), if `total execution cost time not reach threshold && ! reportExecuteProcessUnit`, then it's not necessary to report to governance immediately, it might be reported later
   - In `reportExecuteProcess` (execution completed), it need to delete process from governance, if there was no persistence to governance, then delete process from governance will be ignored for better performance
   




-- 
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 edited a comment on pull request #11123: Support SQL execution process delayed report or no report for better performance

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11123:
URL: https://github.com/apache/shardingsphere/pull/11123#issuecomment-873338519


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11123?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 [#11123](https://codecov.io/gh/apache/shardingsphere/pull/11123?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3aa0ac0) into [master](https://codecov.io/gh/apache/shardingsphere/commit/940e053f45b5c1e2acd3c18bb9baf88c2b244b4f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (940e053) will **decrease** coverage by `1.25%`.
   > The diff coverage is `3.07%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11123/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11123      +/-   ##
   ============================================
   - Coverage     65.16%   63.90%   -1.26%     
   - Complexity      701     1023     +322     
   ============================================
     Files          1827     2176     +349     
     Lines         30862    33366    +2504     
     Branches       5556     5844     +288     
   ============================================
   + Hits          20112    21324    +1212     
   - Misses         9159    10342    +1183     
   - Partials       1591     1700     +109     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...text/process/GovernanceExecuteProcessReporter.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvbnRleHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29udGV4dC9wcm9jZXNzL0dvdmVybmFuY2VFeGVjdXRlUHJvY2Vzc1JlcG9ydGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../process/subscriber/ProcessRegistrySubscriber.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9wcm9jZXNzL3N1YnNjcmliZXIvUHJvY2Vzc1JlZ2lzdHJ5U3Vic2NyaWJlci5qYXZh) | `7.14% <0.00%> (-7.15%)` | :arrow_down: |
   | [...ecute/engine/driver/jdbc/JDBCExecutorCallback.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9leGVjdXRlL2VuZ2luZS9kcml2ZXIvamRiYy9KREJDRXhlY3V0b3JDYWxsYmFjay5qYXZh) | `65.71% <0.00%> (ø)` | |
   | [...te/engine/raw/callback/RawSQLExecutorCallback.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9leGVjdXRlL2VuZ2luZS9yYXcvY2FsbGJhY2svUmF3U1FMRXhlY3V0b3JDYWxsYmFjay5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...fra/executor/sql/process/ExecuteProcessEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9wcm9jZXNzL0V4ZWN1dGVQcm9jZXNzRW5naW5lLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../admin/mysql/executor/ShowProcessListExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2FkbWluL215c3FsL2V4ZWN1dG9yL1Nob3dQcm9jZXNzTGlzdEV4ZWN1dG9yLmphdmE=) | `79.54% <0.00%> (-3.79%)` | :arrow_down: |
   | [...ra/config/properties/ConfigurationPropertyKey.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9jb25maWcvcHJvcGVydGllcy9Db25maWd1cmF0aW9uUHJvcGVydHlLZXkuamF2YQ==) | `100.00% <100.00%> (ø)` | |
   | [...sql/process/model/ExecuteProcessReportContext.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9wcm9jZXNzL21vZGVsL0V4ZWN1dGVQcm9jZXNzUmVwb3J0Q29udGV4dC5qYXZh) | `100.00% <100.00%> (ø)` | |
   | [...tor/dml/impl/ShardingSelectStatementValidator.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcm91dGUvZW5naW5lL3ZhbGlkYXRvci9kbWwvaW1wbC9TaGFyZGluZ1NlbGVjdFN0YXRlbWVudFZhbGlkYXRvci5qYXZh) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
   | [...ckend/text/distsql/rdl/impl/RDLBackendHandler.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmRsL2ltcGwvUkRMQmFja2VuZEhhbmRsZXIuamF2YQ==) | `58.62% <0.00%> (-18.66%)` | :arrow_down: |
   | ... and [522 more](https://codecov.io/gh/apache/shardingsphere/pull/11123/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11123?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/11123?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 [940e053...3aa0ac0](https://codecov.io/gh/apache/shardingsphere/pull/11123?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] tristaZero commented on a change in pull request #11123: Support SQL execution process delayed report or no report for better performance

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #11123:
URL: https://github.com/apache/shardingsphere/pull/11123#discussion_r663484023



##########
File path: shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/process/subscriber/ProcessRegistrySubscriber.java
##########
@@ -68,8 +69,20 @@ public void loadShowProcessListData(final ShowProcessListRequestEvent event) {
      */
     @Subscribe
     public void reportExecuteProcessSummary(final ExecuteProcessSummaryReportEvent event) {
-        ExecuteProcessContext executeProcessContext = event.getExecuteProcessContext();
-        repository.persist(ProcessNode.getExecutionPath(executeProcessContext.getExecutionID()), YamlEngine.marshal(new YamlExecuteProcessContext(executeProcessContext)));
+        String executionID = event.getExecutionID();
+        synchronized (executionID) {
+            Map<String, Object> dataMap = event.getDataMap();
+            ExecuteProcessReportContext reportContext = (ExecuteProcessReportContext) dataMap.get(ExecuteProcessConstants.EXECUTE_ID.name());
+            if (null == reportContext) {
+                return;
+            }
+            YamlExecuteProcessContext yamlExecuteProcessContext = reportContext.getYamlExecuteProcessContext();
+            if (System.currentTimeMillis() - yamlExecuteProcessContext.getStartTimeMillis() <= reportContext.getShowProcessListNoReportThresholdMillis()) {
+                return;
+            }
+            repository.persist(ProcessNode.getExecutionPath(executionID), YamlEngine.marshal(yamlExecuteProcessContext));
+            reportContext.setReportToGovernanceDonePartially(true);

Review comment:
       Why do we need this `ReportToGovernanceDonePartially`?

##########
File path: shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/process/subscriber/ProcessRegistrySubscriber.java
##########
@@ -98,13 +123,21 @@ public void reportExecuteProcessUnit(final ExecuteProcessUnitReportEvent event)
      */
     @Subscribe
     public void reportExecuteProcess(final ExecuteProcessReportEvent event) {
-        String executionPath = ProcessNode.getExecutionPath(event.getExecutionID());
-        YamlExecuteProcessContext yamlExecuteProcessContext = YamlEngine.unmarshal(repository.get(executionPath), YamlExecuteProcessContext.class);
-        for (YamlExecuteProcessUnit unit : yamlExecuteProcessContext.getUnitStatuses()) {
-            if (unit.getStatus() != ExecuteProcessConstants.EXECUTE_STATUS_DONE) {
+        String executionID = event.getExecutionID();
+        synchronized (executionID) {
+            Map<String, Object> dataMap = event.getDataMap();
+            ExecuteProcessReportContext reportContext = (ExecuteProcessReportContext) dataMap.get(ExecuteProcessConstants.EXECUTE_ID.name());

Review comment:
       The value of `dataMap.get(ExecuteProcessConstants.EXECUTE_ID.name())` is `ExecuteProcessReportContext`? Sorry, I can not catch your intend here. Is that a only way? or Is it necessary to do so?




-- 
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 #11123: Support SQL execution process delayed report or no report for better performance

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


   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11123?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 [#11123](https://codecov.io/gh/apache/shardingsphere/pull/11123?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eb6c038) into [master](https://codecov.io/gh/apache/shardingsphere/commit/940e053f45b5c1e2acd3c18bb9baf88c2b244b4f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (940e053) will **decrease** coverage by `1.26%`.
   > The diff coverage is `3.03%`.
   
   > :exclamation: Current head eb6c038 differs from pull request most recent head 3aa0ac0. Consider uploading reports for the commit 3aa0ac0 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/shardingsphere/pull/11123/graphs/tree.svg?width=650&height=150&src=pr&token=ZvlXpWa7so&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/shardingsphere/pull/11123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #11123      +/-   ##
   ============================================
   - Coverage     65.16%   63.90%   -1.27%     
   - Complexity      701     1023     +322     
   ============================================
     Files          1827     2176     +349     
     Lines         30862    33367    +2505     
     Branches       5556     5844     +288     
   ============================================
   + Hits          20112    21324    +1212     
   - Misses         9159    10343    +1184     
   - Partials       1591     1700     +109     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/11123?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...text/process/GovernanceExecuteProcessReporter.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvbnRleHQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29udGV4dC9wcm9jZXNzL0dvdmVybmFuY2VFeGVjdXRlUHJvY2Vzc1JlcG9ydGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../process/subscriber/ProcessRegistrySubscriber.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtZ292ZXJuYW5jZS9zaGFyZGluZ3NwaGVyZS1nb3Zlcm5hbmNlLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2dvdmVybmFuY2UvY29yZS9yZWdpc3RyeS9wcm9jZXNzL3N1YnNjcmliZXIvUHJvY2Vzc1JlZ2lzdHJ5U3Vic2NyaWJlci5qYXZh) | `7.14% <0.00%> (-7.15%)` | :arrow_down: |
   | [...ecute/engine/driver/jdbc/JDBCExecutorCallback.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9leGVjdXRlL2VuZ2luZS9kcml2ZXIvamRiYy9KREJDRXhlY3V0b3JDYWxsYmFjay5qYXZh) | `65.71% <0.00%> (ø)` | |
   | [...te/engine/raw/callback/RawSQLExecutorCallback.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9leGVjdXRlL2VuZ2luZS9yYXcvY2FsbGJhY2svUmF3U1FMRXhlY3V0b3JDYWxsYmFjay5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...fra/executor/sql/process/ExecuteProcessEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9wcm9jZXNzL0V4ZWN1dGVQcm9jZXNzRW5naW5lLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...sql/process/model/yaml/YamlExecuteProcessUnit.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9wcm9jZXNzL21vZGVsL3lhbWwvWWFtbEV4ZWN1dGVQcm9jZXNzVW5pdC5qYXZh) | `0.00% <ø> (ø)` | |
   | [.../admin/mysql/executor/ShowProcessListExecutor.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2FkbWluL215c3FsL2V4ZWN1dG9yL1Nob3dQcm9jZXNzTGlzdEV4ZWN1dG9yLmphdmE=) | `79.54% <0.00%> (-3.79%)` | :arrow_down: |
   | [...ra/config/properties/ConfigurationPropertyKey.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9jb25maWcvcHJvcGVydGllcy9Db25maWd1cmF0aW9uUHJvcGVydHlLZXkuamF2YQ==) | `100.00% <100.00%> (ø)` | |
   | [...sql/process/model/ExecuteProcessReportContext.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtZXhlY3V0b3Ivc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2luZnJhL2V4ZWN1dG9yL3NxbC9wcm9jZXNzL21vZGVsL0V4ZWN1dGVQcm9jZXNzUmVwb3J0Q29udGV4dC5qYXZh) | `100.00% <100.00%> (ø)` | |
   | [...tor/dml/impl/ShardingSelectStatementValidator.java](https://codecov.io/gh/apache/shardingsphere/pull/11123/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvcm91dGUvZW5naW5lL3ZhbGlkYXRvci9kbWwvaW1wbC9TaGFyZGluZ1NlbGVjdFN0YXRlbWVudFZhbGlkYXRvci5qYXZh) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
   | ... and [523 more](https://codecov.io/gh/apache/shardingsphere/pull/11123/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/11123?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/11123?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 [940e053...3aa0ac0](https://codecov.io/gh/apache/shardingsphere/pull/11123?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 commented on a change in pull request #11123: Support SQL execution process delayed report or no report for better performance

Posted by GitBox <gi...@apache.org>.
sandynz commented on a change in pull request #11123:
URL: https://github.com/apache/shardingsphere/pull/11123#discussion_r663511182



##########
File path: shardingsphere-governance/shardingsphere-governance-core/src/main/java/org/apache/shardingsphere/governance/core/registry/process/subscriber/ProcessRegistrySubscriber.java
##########
@@ -98,13 +123,21 @@ public void reportExecuteProcessUnit(final ExecuteProcessUnitReportEvent event)
      */
     @Subscribe
     public void reportExecuteProcess(final ExecuteProcessReportEvent event) {
-        String executionPath = ProcessNode.getExecutionPath(event.getExecutionID());
-        YamlExecuteProcessContext yamlExecuteProcessContext = YamlEngine.unmarshal(repository.get(executionPath), YamlExecuteProcessContext.class);
-        for (YamlExecuteProcessUnit unit : yamlExecuteProcessContext.getUnitStatuses()) {
-            if (unit.getStatus() != ExecuteProcessConstants.EXECUTE_STATUS_DONE) {
+        String executionID = event.getExecutionID();
+        synchronized (executionID) {
+            Map<String, Object> dataMap = event.getDataMap();
+            ExecuteProcessReportContext reportContext = (ExecuteProcessReportContext) dataMap.get(ExecuteProcessConstants.EXECUTE_ID.name());

Review comment:
       Yes, the value is `ExecuteProcessReportContext`.
   
   Since execution process report might be delayed or ignored for better performance, the related data is cached in `ExecuteProcessReportContext`, it will be used in the whole lifecyle of current sql execution.
   
   I've considered to add a new key-value pair in `dataMap`, considering that there's `EXECUTE_ID` already, so I reused 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 pull request #11123: Support SQL execution process delayed report or no report for better performance

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


   Some discussion notes with @tristaZero 
   
   #### Disadvanges of current solution
   - Reduce processlist accurateness (sql process that execute completed quickly might not be reported to governance, and could not be shown to end user)
   - If proxy instance crash, execution process cached on local will be lost, it's difficult to recover
   - It might hurt current architecture and code readability, not sure it's worth to do it or not
   
   #### Current actions
   - Go on to enable process report for DML statement
   - Optimize processlist columns
   - Wait for performance testing after DML statement enabled
   


-- 
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] tristaZero commented on pull request #11123: Support SQL execution process delayed report or no report for better performance

Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #11123:
URL: https://github.com/apache/shardingsphere/pull/11123#issuecomment-875194015






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