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/09/16 09:15:00 UTC
[GitHub] [shardingsphere] RaigorJiang opened a new issue #12485: [DistSQL][Scaling] `RALBackendHandlerFactory` needs to be optimized.
RaigorJiang opened a new issue #12485:
URL: https://github.com/apache/shardingsphere/issues/12485
When the input DistSQL is a RAL statement, RALBackendHandlerFactory will be used to construct the TextProtocolBackendHandler.
Further, if the input RAL statement is an instance of UpdatableRALStatement, UpdatableRALBackendHandlerFactory will be used. **[picture 1]**
In UpdatableRALBackendHandlerFactory, the static block is used to load the SPI of RALUpdater. Here are some information:
1. The RAL statement only contained Scaling previously, but now there can be more types;
2. When loading the updater SPI of Scaling, the Scaling configuration will be checked through a static method, and an exception will be thrown if there is no configuration; **[picture 2]**
This brings up a new problem:
When adding a new RALUpdate statement, if the Scaling rule is not configured, users will get an exception, even if users do not need the Scaling feature.
This affected the development of new features.
I hope this problem can be resolved by refactoring.
--
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 closed issue #12485: [DistSQL][Scaling] `ScalingAPIFactory.getScalingAPI()` exception when scaling feature not enabled should be optimized to support new RALUpdater implementation
Posted by GitBox <gi...@apache.org>.
tristaZero closed issue #12485:
URL: https://github.com/apache/shardingsphere/issues/12485
--
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 issue #12485: [DistSQL][Scaling] `ScalingAPIFactory.getScalingAPI()` exception when scaling feature not enabled should be optimized to support new RALUpdater implementation
Posted by GitBox <gi...@apache.org>.
sandynz commented on issue #12485:
URL: https://github.com/apache/shardingsphere/issues/12485#issuecomment-922212306
Verification:
scaling feature not enabled in `server.yaml`
### Test with old code:
```
mysql> show scaling job list;
ERROR 1999 (C1999): Unknown exception: [Scaling server configuration is required.]
```
As new added DistSQL, it will show scaling error, which is wrong.
Exception:
```
[ShardingSphere-Command-1] o.a.s.p.f.c.CommandExecutorTask - Exception occur:
java.lang.NullPointerException: Scaling server configuration is required.
at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:897)
at org.apache.shardingsphere.scaling.core.api.ScalingAPIFactory.checkServerConfig(ScalingAPIFactory.java:106)
at org.apache.shardingsphere.scaling.core.api.ScalingAPIFactory.access$000(ScalingAPIFactory.java:48)
at org.apache.shardingsphere.scaling.core.api.ScalingAPIFactory$ScalingAPIHolder.getInstance(ScalingAPIFactory.java:119)
at org.apache.shardingsphere.scaling.core.api.ScalingAPIFactory.getScalingAPI(ScalingAPIFactory.java:56)
at org.apache.shardingsphere.scaling.distsql.handler.ShowScalingJobListQueryResultSet.init(ShowScalingJobListQueryResultSet.java:41)
at org.apache.shardingsphere.proxy.backend.text.distsql.ral.query.QueryableRALBackendHandler.execute(QueryableRALBackendHandler.java:48)
at org.apache.shardingsphere.proxy.backend.text.distsql.ral.query.QueryableRALBackendHandler.execute(QueryableRALBackendHandler.java:37)
at org.apache.shardingsphere.proxy.backend.text.SchemaRequiredBackendHandler.execute(SchemaRequiredBackendHandler.java:51)
at org.apache.shardingsphere.proxy.frontend.mysql.command.query.text.query.MySQLComQueryPacketExecutor.execute(MySQLComQueryPacketExecutor.java:57)
at org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask.executeCommand(CommandExecutorTask.java:99)
at org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask.run(CommandExecutorTask.java:72)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
```
### After moving `checkServerConfig()` to `ScalingWorker.init`:
```
mysql> show scaling job list;
ERROR 1999 (C1999): Unknown exception: [null]
```
Exception:
```
[ShardingSphere-Command-1] o.a.s.p.f.c.CommandExecutorTask - Exception occur:
java.lang.NullPointerException: null
at org.apache.shardingsphere.scaling.core.api.ScalingAPIFactory$ElasticJobAPIHolder.<init>(ScalingAPIFactory.java:157)
at org.apache.shardingsphere.scaling.core.api.ScalingAPIFactory$ElasticJobAPIHolder.getInstance(ScalingAPIFactory.java:168)
at org.apache.shardingsphere.scaling.core.api.ScalingAPIFactory.getJobStatisticsAPI(ScalingAPIFactory.java:72)
at org.apache.shardingsphere.scaling.core.api.impl.ScalingAPIImpl.list(ScalingAPIImpl.java:55)
at org.apache.shardingsphere.scaling.distsql.handler.ShowScalingJobListQueryResultSet.init(ShowScalingJobListQueryResultSet.java:41)
at org.apache.shardingsphere.proxy.backend.text.distsql.ral.query.QueryableRALBackendHandler.execute(QueryableRALBackendHandler.java:48)
at org.apache.shardingsphere.proxy.backend.text.distsql.ral.query.QueryableRALBackendHandler.execute(QueryableRALBackendHandler.java:37)
at org.apache.shardingsphere.proxy.backend.text.SchemaRequiredBackendHandler.execute(SchemaRequiredBackendHandler.java:51)
at org.apache.shardingsphere.proxy.frontend.mysql.command.query.text.query.MySQLComQueryPacketExecutor.execute(MySQLComQueryPacketExecutor.java:57)
at org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask.executeCommand(CommandExecutorTask.java:99)
at org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask.run(CommandExecutorTask.java:72)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
```
There's no error message for user, it's better to keep meaningful error message.
### Consider other solutions:
1. Scaling sub-classes of `RALUpdater` invoke `ScalingAPIFactory.getScalingAPI()` on demand, but not when class initialization.
2. Any sub-class of `RALUpdater` belongs to a feature, if certain feature is not enabled, then `ShardingSphereServiceLoader.register` should ignore to initialize related `RALUpdater` implementation.
Currently, I'll implement it with solution 1.
@RaigorJiang Could you have a look at solution 2, is it possible to implement?
--
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 issue #12485: [DistSQL][Scaling] `RALBackendHandlerFactory` needs to be optimized.
Posted by GitBox <gi...@apache.org>.
sandynz commented on issue #12485:
URL: https://github.com/apache/shardingsphere/issues/12485#issuecomment-922178576
In `RALBackendHandlerFactory.java`
```
public static TextProtocolBackendHandler newInstance(final RALStatement sqlStatement, final BackendConnection backendConnection) throws SQLException {
if (sqlStatement instanceof QueryableRALStatement) {
return QueryableRALBackendHandlerFactory.newInstance((QueryableRALStatement) sqlStatement, backendConnection);
}
if (sqlStatement instanceof UpdatableRALStatement) {
return UpdatableRALBackendHandlerFactory.newInstance((UpdatableRALStatement) sqlStatement);
}
if (sqlStatement instanceof CommonDistSQLStatement) {
return CommonDistSQLBackendHandlerFactory.newInstance((CommonDistSQLStatement) sqlStatement, backendConnection);
}
if (sqlStatement instanceof AdvancedDistSQLStatement) {
return AdvancedDistSQLBackendHandlerFactory.newInstance((AdvancedDistSQLStatement) sqlStatement, backendConnection);
}
throw new UnsupportedOperationException(sqlStatement.getClass().getCanonicalName());
}
```
Currently, `UpdatableRALStatement` sub-classes are: `DropScalingJobStatement`, `ResetScalingJobStatement`, `StartScalingJobStatement`, `StopScalingJobStatement`, all of them belong to scaling feature.
If new sub-class of `UpdatableRALStatement` is added in other feature and scaling feature is not enabled, `checkServerConfig()` in scaling will have side effect.
Possible solution:
- Move `checkServerConfig()` outside of `ScalingAPIFactory`, check it in `ScalingWorker.init` could be better.
--
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 issue #12485: [DistSQL][Scaling] `RALBackendHandlerFactory` needs to be optimized.
Posted by GitBox <gi...@apache.org>.
sandynz commented on issue #12485:
URL: https://github.com/apache/shardingsphere/issues/12485#issuecomment-922174768
Server config check:
```
private static void checkServerConfig() {
ServerConfiguration serverConfig = ScalingContext.getInstance().getServerConfig();
Preconditions.checkNotNull(serverConfig, "Scaling server configuration is required.");
Preconditions.checkNotNull(serverConfig.getModeConfiguration(), "Mode configuration is required.");
Preconditions.checkArgument("Cluster".equals(serverConfig.getModeConfiguration().getType()), "Mode must be `Cluster`.");
}
```
Call hierarchy:
```
ScalingAPIFactory.checkServerConfig() (org.apache.shardingsphere.scaling.core.api)
getInstance() in ScalingAPIHolder in ScalingAPIFactory (org.apache.shardingsphere.scaling.core.api)
ScalingAPIFactory.getScalingAPI() (org.apache.shardingsphere.scaling.core.api)
```
--
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