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/01/22 09:25:53 UTC
[GitHub] [shardingsphere] RaigorJiang opened a new pull request #14998: Fixes #14814, optimize query result of `SHOW SCHEMA RESOURCES`
RaigorJiang opened a new pull request #14998:
URL: https://github.com/apache/shardingsphere/pull/14998
Fixes #14814.
### Example
```sql
ADD RESOURCE ds_0 (
HOST=127.0.0.1,
PORT=3306,
DB=demo_ds_0,
USER=root,
PASSWORD=123456,
PROPERTIES("maximumPoolSize"=6,"idleTimeoutMilliseconds"="10000")
), ds_1 (
URL="jdbc:mysql://127.0.0.1:3306/demo_ds_1?serverTimezone=UTC&useSSL=false",
USER=root,PASSWORD=123456,
PROPERTIES("maxPoolSize"=5,"idleTimeout"="30000")
);
```
#### Before
```sql
mysql> SHOW SCHEMA RESOURCES;
+------+-------+-----------+------+-----------+-------------------------------------------------------+
| name | type | host | port | db | attribute |
+------+-------+-----------+------+-----------+-------------------------------------------------------+
| ds_0 | MySQL | 127.0.0.1 | 3306 | demo_ds_0 | {"idleTimeoutMilliseconds":"10000","maxPoolSize":"6"} |
| ds_1 | MySQL | 127.0.0.1 | 3306 | demo_ds_1 | {"idleTimeoutMilliseconds":"30000","maxPoolSize":"5"} |
+------+-------+-----------+------+-----------+-------------------------------------------------------+
2 rows in set (0.10 sec)
```
#### After
```sql
mysql> SHOW SCHEMA RESOURCES;
mysql> show schema resources;
+------+-------+-----------+------+-----------+---------------------------------+---------------------------+---------------------------+---------------+---------------+-----------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| name | type | host | port | db | connection_timeout_milliseconds | idle_timeout_milliseconds | max_lifetime_milliseconds | max_pool_size | min_pool_size | read_only | other_attributes |
+------+-------+-----------+------+-----------+---------------------------------+---------------------------+---------------------------+---------------+---------------+-----------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| ds_0 | MySQL | 127.0.0.1 | 3306 | demo_ds_0 | 30000 | 10000 | 2100000 | 6 | 1 | false | {"allowPoolSuspension":false,"autoCommit":true,"healthCheckProperties":{},"initializationFailTimeout":1,"isolateInternalQueries":false,"leakDetectionThreshold":0,"validationTimeout":5000} |
| ds_1 | MySQL | 127.0.0.1 | 3306 | demo_ds_1 | 30000 | 30000 | 2100000 | 5 | 1 | false | {"allowPoolSuspension":false,"autoCommit":true,"healthCheckProperties":{},"initializationFailTimeout":1,"isolateInternalQueries":false,"leakDetectionThreshold":0,"validationTimeout":5000} |
+------+-------+-----------+------+-----------+---------------------------------+---------------------------+---------------------------+---------------+---------------+-----------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
2 rows in set (0.01 sec)
```
--
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] terrymanu commented on a change in pull request #14998: Fixes #14814, optimize query result of `SHOW SCHEMA RESOURCES`
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #14998:
URL: https://github.com/apache/shardingsphere/pull/14998#discussion_r790615225
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rql/resource/DataSourceQueryResultSet.java
##########
@@ -54,22 +61,17 @@
@Override
public void init(final ShardingSphereMetaData metaData, final SQLStatement sqlStatement) {
resource = metaData.getResource();
- Optional<MetaDataPersistService> persistService = ProxyContext.getInstance().getContextManager().getMetaDataContexts().getMetaDataPersistService();
-
- if (persistService.isPresent()) {
- dataSourcePropsMap = persistService.get().getDataSourceService().load(metaData.getName());
- } else {
- dataSourcePropsMap = new LinkedHashMap<>(metaData.getResource().getDataSources().size(), 1);
- for (Entry<String, DataSource> entry : metaData.getResource().getDataSources().entrySet()) {
- dataSourcePropsMap.put(entry.getKey(), DataSourcePropertiesCreator.create(entry.getValue()));
- }
+ dataSourcePropsMap = new LinkedHashMap<>(metaData.getResource().getDataSources().size(), 1);
+ for (Map.Entry<String, DataSource> entry : metaData.getResource().getDataSources().entrySet()) {
Review comment:
Please use `Entry` instead of `Map.Entry`
--
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] RaigorJiang commented on pull request #14998: Fixes #14814, optimize query result of `SHOW SCHEMA RESOURCES`
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on pull request #14998:
URL: https://github.com/apache/shardingsphere/pull/14998#issuecomment-1019716709
> Please fix the CI
@terrymanu Fixed, please review again, thanks!
--
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 #14998: Fixes #14814, optimize query result of `SHOW SCHEMA RESOURCES`
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #14998:
URL: https://github.com/apache/shardingsphere/pull/14998#issuecomment-1019723082
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/14998?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 [#14998](https://codecov.io/gh/apache/shardingsphere/pull/14998?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2a4d6b1) into [master](https://codecov.io/gh/apache/shardingsphere/commit/f82c8c25c444938289b644607d2d60e3858161d5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f82c8c2) will **decrease** coverage by `0.00%`.
> The diff coverage is `91.66%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/14998?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 #14998 +/- ##
============================================
- Coverage 59.60% 59.60% -0.01%
Complexity 1891 1891
============================================
Files 3142 3142
Lines 46891 46900 +9
Branches 7944 7943 -1
============================================
+ Hits 27951 27956 +5
- Misses 16729 16732 +3
- Partials 2211 2212 +1
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/14998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...distsql/rql/resource/DataSourceQueryResultSet.java](https://codecov.io/gh/apache/shardingsphere/pull/14998/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcnFsL3Jlc291cmNlL0RhdGFTb3VyY2VRdWVyeVJlc3VsdFNldC5qYXZh) | `90.90% <91.66%> (-0.76%)` | :arrow_down: |
| [...d/text/distsql/ral/common/hint/HintSourceType.java](https://codecov.io/gh/apache/shardingsphere/pull/14998/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvYmFja2VuZC90ZXh0L2Rpc3RzcWwvcmFsL2NvbW1vbi9oaW50L0hpbnRTb3VyY2VUeXBlLmphdmE=) | `0.00% <0.00%> (-42.86%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/14998?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/14998?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 [f82c8c2...2a4d6b1](https://codecov.io/gh/apache/shardingsphere/pull/14998?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] terrymanu commented on a change in pull request #14998: Fixes #14814, optimize query result of `SHOW SCHEMA RESOURCES`
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #14998:
URL: https://github.com/apache/shardingsphere/pull/14998#discussion_r790146254
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rql/resource/DataSourceQueryResultSet.java
##########
@@ -72,18 +90,66 @@ public boolean next() {
@Override
public Collection<Object> getRowData() {
String dataSourceName = dataSourceNames.next();
+ DataSourceProperties dataSourceProperties = dataSourcePropsMap.get(dataSourceName);
+ DataSourcePoolMetaData poolMetaData = DataSourcePoolMetaDataFactory.newInstance(dataSourceProperties.getDataSourceClassName());
+ Map<String, Object> allProperties = getAllProperties(resource.getDataSources().get(dataSourceName));
Review comment:
It is unnecessary to get all data source properties, display PoolPropertySynonyms is enough for display.
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rql/resource/DataSourceQueryResultSet.java
##########
@@ -72,18 +90,66 @@ public boolean next() {
@Override
public Collection<Object> getRowData() {
String dataSourceName = dataSourceNames.next();
+ DataSourceProperties dataSourceProperties = dataSourcePropsMap.get(dataSourceName);
+ DataSourcePoolMetaData poolMetaData = DataSourcePoolMetaDataFactory.newInstance(dataSourceProperties.getDataSourceClassName());
+ Map<String, Object> allProperties = getAllProperties(resource.getDataSources().get(dataSourceName));
+ Map<String, Object> standardProps = getStandardProperties(dataSourceProperties, poolMetaData, allProperties);
+ return getRowData(dataSourceName, poolMetaData, allProperties, standardProps);
+ }
+
+ private Collection<Object> getRowData(final String dataSourceName, final DataSourcePoolMetaData poolMetaData, final Map<String, Object> allProperties, final Map<String, Object> standardProps) {
DataSourceMetaData metaData = resource.getDataSourcesMetaData().getDataSourceMetaData(dataSourceName);
- return Arrays.asList(dataSourceName, resource.getDatabaseType().getName(), metaData.getHostname(), metaData.getPort(), metaData.getCatalog(),
- new Gson().toJson(getFilteredUndisplayedProperties(dataSourcePropsMap.get(dataSourceName))));
+ Collection<Object> result = new LinkedList<>();
+ result.add(dataSourceName);
+ result.add(resource.getDatabaseType().getName());
+ result.add(metaData.getHostname());
+ result.add(metaData.getPort());
+ result.add(metaData.getCatalog());
+ result.add(getStandardProperty(standardProps, CONNECTION_TIMEOUT_MILLISECONDS));
+ result.add(getStandardProperty(standardProps, IDLE_TIMEOUT_MILLISECONDS));
+ result.add(getStandardProperty(standardProps, MAX_LIFETIME_MILLISECONDS));
+ result.add(getStandardProperty(standardProps, MAX_POOL_SIZE));
+ result.add(getStandardProperty(standardProps, MIN_POOL_SIZE));
+ result.add(getStandardProperty(standardProps, READ_ONLY));
+ result.add(new Gson().toJson(getFilteredUndisplayedProperties(poolMetaData, allProperties, standardProps)));
+ return result;
+ }
+
+ private Map<String, Object> getAllProperties(final DataSource dataSource) {
+ DataSourceReflection dataSourceReflection = new DataSourceReflection(dataSource);
+ Map<String, Object> result = dataSourceReflection.convertToProperties();
+ return result;
}
- private Map<String, Object> getFilteredUndisplayedProperties(final DataSourceProperties dataSourceProperties) {
- Map<String, Object> result = new HashMap<>(dataSourceProperties.getPoolPropertySynonyms().getStandardProperties());
- for (Entry<String, Object> entry : dataSourceProperties.getCustomDataSourceProperties().getProperties().entrySet()) {
- if (!(entry.getValue() instanceof Collection) && !(entry.getValue() instanceof Map)) {
- result.put(entry.getKey(), entry.getValue());
+ private Map<String, Object> getStandardProperties(final DataSourceProperties dataSourceProperties, final DataSourcePoolMetaData poolMetaData, final Map<String, Object> allProperties) {
Review comment:
It is unnecessary to call poolMetaData.getPropertySynonyms() again, just use dataSourceProperties.getPoolPropertySynonyms(), which has already set all necessary informations.
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rql/resource/DataSourceQueryResultSet.java
##########
@@ -72,18 +90,66 @@ public boolean next() {
@Override
public Collection<Object> getRowData() {
String dataSourceName = dataSourceNames.next();
+ DataSourceProperties dataSourceProperties = dataSourcePropsMap.get(dataSourceName);
+ DataSourcePoolMetaData poolMetaData = DataSourcePoolMetaDataFactory.newInstance(dataSourceProperties.getDataSourceClassName());
+ Map<String, Object> allProperties = getAllProperties(resource.getDataSources().get(dataSourceName));
+ Map<String, Object> standardProps = getStandardProperties(dataSourceProperties, poolMetaData, allProperties);
+ return getRowData(dataSourceName, poolMetaData, allProperties, standardProps);
+ }
+
+ private Collection<Object> getRowData(final String dataSourceName, final DataSourcePoolMetaData poolMetaData, final Map<String, Object> allProperties, final Map<String, Object> standardProps) {
DataSourceMetaData metaData = resource.getDataSourcesMetaData().getDataSourceMetaData(dataSourceName);
- return Arrays.asList(dataSourceName, resource.getDatabaseType().getName(), metaData.getHostname(), metaData.getPort(), metaData.getCatalog(),
- new Gson().toJson(getFilteredUndisplayedProperties(dataSourcePropsMap.get(dataSourceName))));
+ Collection<Object> result = new LinkedList<>();
+ result.add(dataSourceName);
+ result.add(resource.getDatabaseType().getName());
+ result.add(metaData.getHostname());
+ result.add(metaData.getPort());
+ result.add(metaData.getCatalog());
+ result.add(getStandardProperty(standardProps, CONNECTION_TIMEOUT_MILLISECONDS));
+ result.add(getStandardProperty(standardProps, IDLE_TIMEOUT_MILLISECONDS));
+ result.add(getStandardProperty(standardProps, MAX_LIFETIME_MILLISECONDS));
+ result.add(getStandardProperty(standardProps, MAX_POOL_SIZE));
+ result.add(getStandardProperty(standardProps, MIN_POOL_SIZE));
+ result.add(getStandardProperty(standardProps, READ_ONLY));
+ result.add(new Gson().toJson(getFilteredUndisplayedProperties(poolMetaData, allProperties, standardProps)));
+ return result;
+ }
+
+ private Map<String, Object> getAllProperties(final DataSource dataSource) {
+ DataSourceReflection dataSourceReflection = new DataSourceReflection(dataSource);
+ Map<String, Object> result = dataSourceReflection.convertToProperties();
+ return result;
Review comment:
Just use `DataSourcePropertiesCreator.create()` to create DataSourceProperties, which include all processed properties.
Please do not call `DataSourceReflection` directly, which is for DataSourceProperties's internal call only.
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/datasource/pool/metadata/impl/HikariDataSourcePoolMetaData.java
##########
@@ -71,6 +71,8 @@ private void buildPropertySynonyms() {
}
private void buildTransientFieldNames() {
+ transientFieldNames.add("username");
+ transientFieldNames.add("password");
Review comment:
The attributes of `username` and `password` should not add here.
They are not transient fields, they just belong to connection properties.
--
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] terrymanu merged pull request #14998: Fixes #14814, optimize query result of `SHOW SCHEMA RESOURCES`
Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #14998:
URL: https://github.com/apache/shardingsphere/pull/14998
--
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] RaigorJiang commented on a change in pull request #14998: Fixes #14814, optimize query result of `SHOW SCHEMA RESOURCES`
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on a change in pull request #14998:
URL: https://github.com/apache/shardingsphere/pull/14998#discussion_r790633331
##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/text/distsql/rql/resource/DataSourceQueryResultSet.java
##########
@@ -54,22 +61,17 @@
@Override
public void init(final ShardingSphereMetaData metaData, final SQLStatement sqlStatement) {
resource = metaData.getResource();
- Optional<MetaDataPersistService> persistService = ProxyContext.getInstance().getContextManager().getMetaDataContexts().getMetaDataPersistService();
-
- if (persistService.isPresent()) {
- dataSourcePropsMap = persistService.get().getDataSourceService().load(metaData.getName());
- } else {
- dataSourcePropsMap = new LinkedHashMap<>(metaData.getResource().getDataSources().size(), 1);
- for (Entry<String, DataSource> entry : metaData.getResource().getDataSources().entrySet()) {
- dataSourcePropsMap.put(entry.getKey(), DataSourcePropertiesCreator.create(entry.getValue()));
- }
+ dataSourcePropsMap = new LinkedHashMap<>(metaData.getResource().getDataSources().size(), 1);
+ for (Map.Entry<String, DataSource> entry : metaData.getResource().getDataSources().entrySet()) {
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