-
Notifications
You must be signed in to change notification settings - Fork 527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add multi-node auth information sharing function #1350
Conversation
@@ -101,7 +101,7 @@ | |||
|
|||
private final HugeGraph hugegraph; | |||
private final TaskScheduler taskScheduler; | |||
private final UserManager userManager; | |||
private UserManager userManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set final
@@ -652,6 +652,12 @@ public UserManager userManager() { | |||
return this.userManager; | |||
} | |||
|
|||
@Override | |||
public void swichUserManager(UserManager userManager) { | |||
verifyAdminPermission(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.verifyAdminPermission()
|
||
String remoteUrl = config.get(ServerOptions.AUTH_REMOTE_URL); | ||
if (StringUtils.isNotEmpty(remoteUrl)) { | ||
this.graph.swichUserManager(new RpcClientProvider(config). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
define RpcClientProvider provider
first
|
||
public static final ConfigOption<Integer> RPC_SERVER_PORT = | ||
new ConfigOption<>( | ||
"rpc.server.port", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rpcserver.port, same as other options
public static final ConfigOption<Integer> RPC_SERVER_TIMEOUT = | ||
new ConfigOption<>( | ||
"rpc.server.timeout", | ||
"Rpc server connction timeout", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
connection
|
||
public class RpcConsumerConfig { | ||
|
||
private final Map<String, ConsumerConfig> CONSUMER_CONFIG = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename config
private final static SofaRpcClient INSTANCE = new SofaRpcClient(); | ||
} | ||
|
||
public static SofaRpcClient getInstance() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to instance(), because our style is no "get" prefix
} | ||
|
||
public static SofaRpcClient getInstance() { | ||
return SofaRpcClientHolder.INSTANCE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to create one instance for each connection?
RpcConfigs.putValue("logger.impl", | ||
conf.get(ServerOptions.RPC_LOGGER_IMPL)); | ||
this.serverConfig = new ServerConfig() | ||
.setProtocol(conf.get(ServerOptions.RPC_PROTOCOL)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
align with new ServerConfig
} | ||
} | ||
|
||
public int getBindPort() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
port()
new ConfigOption<>( | ||
"rpc.server.timeout", | ||
"Rpc server connction timeout", | ||
"rpc.connection_timeout", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rpcclient or rpc.client_connection_timeout
this.rpcServerProvider.destroy(); | ||
} | ||
|
||
public void close() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to line 164
@@ -30,7 +30,7 @@ | |||
|
|||
public class RpcConsumerConfig { | |||
|
|||
private final Map<String, ConsumerConfig> CONSUMER_CONFIG = | |||
private final Map<String, ConsumerConfig> CONFIG = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to config, only const var(static final) use capital letters
|
||
public static final ConfigOption<Integer> RPC_SERVER_PORT = | ||
new ConfigOption<>( | ||
"rpcserver.port", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rpc.server_port
RpcConfigs.putValue("logger.impl", | ||
conf.get(ServerOptions.RPC_LOGGER_IMPL)); | ||
this.serverConfig = new ServerConfig() | ||
.setProtocol(conf.get(ServerOptions.RPC_PROTOCOL)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RPC_SERVER_PROTOCOL
public ConsumerConfig consumerConfig(String serverName) { | ||
if (!CONFIG.containsKey(serverName)) { | ||
throw new SofaRpcException(RpcErrorType.CLIENT_UNDECLARED_ERROR, | ||
String.format("Invalid server name " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
define var error
"rpc.config.order", | ||
"Sofa rpc loading order, the larger the more after " + | ||
"loading", | ||
rangeInt(-1, Integer.MAX_VALUE), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check the range
public static final ConfigOption<Integer> RPC_RETRIES = | ||
new ConfigOption<>( | ||
"rpc.retries", | ||
"Rpc server failed and retries times", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
improve
new ConfigOption<>( | ||
"rpc.retries", | ||
"Rpc server failed and retries times", | ||
rangeInt(-1, Integer.MAX_VALUE), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check the range
@@ -225,76 +225,80 @@ public static synchronized ServerOptions instance() { | |||
public static final ConfigOption<String> AUTH_REMOTE_URL = | |||
new ConfigOption<>( | |||
"auth.remote_url", | |||
"Auth server address", | |||
"Auth server address. If the address is empty, it is " + | |||
"only auth Service, otherwise it is auth server and auth " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the address is empty, it provide auth service, otherwise it is auth client and also provide auth service through rpc forwarding.
"Rpc server port", | ||
rangeInt(-1, Integer.MAX_VALUE), | ||
"rpc.server_port", | ||
"The port of rpc server for rpc client call.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
calling
"Rpc connection timeout", | ||
rangeInt(-1, Integer.MAX_VALUE), | ||
"rpc.client_connection_timeout", | ||
"The rpc client connect rpc server timeout(seconds).", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timeout(in seconds) of rpc client connect to rpc server
"Rpc read timeout", | ||
rangeInt(-1, Integer.MAX_VALUE), | ||
"rpc.client_read_timeout", | ||
"The rpc client read from rpc server timeout(seconds).", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
3 | ||
); | ||
|
||
public static final ConfigOption<String> RPC_PROTOCOL = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we rename to RPC_SERVER_PROTOCOL?
and move to place of server options
@@ -42,6 +44,13 @@ public RpcServerProvider(HugeConfig conf, UserManager userManager) { | |||
this.rpcServer.port()); | |||
} | |||
|
|||
private void initRpcConfigs(HugeConfig conf) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how client call this method
|
||
private void startRpcServer(HugeConfig conf) { | ||
if (this.authenticator != null) { | ||
this.rpcServerProvider = new RpcServerProvider(conf, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set rpcServerProvider final and return RpcServerProvider here
new RpcProviderConfig(UserManager.class, userManager); | ||
this.rpcServer = new SofaRpcServer(conf, rpcProviderConfig); | ||
this.rpcServer.exportAll(); | ||
LOG.info("rpcServer start success, bind port is {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RpcServer
@@ -0,0 +1,39 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unneeded exception package
@@ -0,0 +1,43 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename package sofarpc to rpc
@@ -77,7 +77,7 @@ public GraphManager(HugeConfig conf) { | |||
// Raft will load snapshot firstly then launch election and replay log | |||
this.waitGraphsStarted(); | |||
this.checkBackendVersionOrExit(); | |||
this.startRpcServer(conf); | |||
this.rpcServerProvider = startRpcServer(conf); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.startRpcServer()
@@ -33,13 +33,13 @@ | |||
private final SofaRpcServer rpcServer; | |||
|
|||
public RpcServerProvider(HugeConfig conf, UserManager userManager) { | |||
LOG.info("rpcServer start {}", conf.get(ServerOptions.RPC_SERVER_PORT)); | |||
LOG.info("RpcServer start {}", conf.get(ServerOptions.RPC_SERVER_PORT)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"RpcServer starting on port {}"
LOG.debug() is ok
RpcCommonConfig.initRpcConfigs(conf); | ||
RpcProviderConfig rpcProviderConfig = | ||
new RpcProviderConfig(UserManager.class, userManager); | ||
this.rpcServer = new SofaRpcServer(conf, rpcProviderConfig); | ||
this.rpcServer.exportAll(); | ||
LOG.info("rpcServer start success, bind port is {}", | ||
LOG.info("RpcServer start success, bind port is {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RpcServer started success on port {}
"rpc.protocol", | ||
"Rpc communication protocol, client and server need to " + | ||
"be specified at the same time, and can match.", | ||
disallowEmpty(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allowedValues()
public class RpcConsumerConfig { | ||
|
||
private final Map<String, ConsumerConfig> config = | ||
Maps.newHashMap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
align with config
|
||
public class RpcProviderConfig { | ||
|
||
private final Map<String, ProviderConfig> PROVIDER_CONFIG = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
this.buildProviderConfig(clazz, serviceImpl); | ||
} | ||
|
||
public <T, E extends T> void buildProviderConfig(Class<T> clazz, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
PROVIDER_CONFIG.put(clazz.getName(), providerConfig); | ||
} | ||
|
||
public Map<String, ProviderConfig> providerConfigMap() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
providerConfigs()
private final Map<String, ProviderConfig> PROVIDER_CONFIG = | ||
Maps.newHashMap(); | ||
|
||
public <T, E extends T> RpcProviderConfig(Class<T> clazz, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this constructor
private final Map<String, ConsumerConfig> config = | ||
Maps.newHashMap(); | ||
|
||
public <T> RpcConsumerConfig(Class<T> clazz, HugeConfig conf) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this constructor
this.rpcServer = new SofaRpcServer(conf, rpcProviderConfig); | ||
this.rpcServer.exportAll(); | ||
LOG.info("RpcServer start success, bind port is {}", | ||
this.rpcServer.port()); | ||
LOG.debug("RpcServer started success on port {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
info
@@ -33,15 +33,15 @@ | |||
private final SofaRpcServer rpcServer; | |||
|
|||
public RpcServerProvider(HugeConfig conf, UserManager userManager) { | |||
LOG.debug("RpcServer starting on port {}", | |||
conf.get(ServerOptions.RPC_SERVER_PORT)); | |||
LOG.info("RpcServer starting on port {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep LOG.debug here
RpcCommonConfig.initRpcConfigs(conf); | ||
RpcProviderConfig rpcProviderConfig = new RpcProviderConfig(); | ||
rpcProviderConfig.addProviderConfig(UserManager.class, userManager); | ||
this.rpcServer = new SofaRpcServer(conf, rpcProviderConfig); | ||
this.rpcServer.exportAll(); | ||
LOG.debug("RpcServer started success on port {}", | ||
this.rpcServer.port()); | ||
LOG.info("RpcServer started success on port {}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep LOG.info here
@@ -222,6 +222,85 @@ public static synchronized ServerOptions instance() { | |||
"hugegraph:9fd95c9c-711b-415b-b85f-d4df46ba5c31" | |||
); | |||
|
|||
public static final ConfigOption<String> AUTH_REMOTE_URL = | |||
new ConfigOption<>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
define as ListConfigOption for multi auth servers
|
||
public class SofaRpcServer { | ||
|
||
private Map<String, ProviderConfig> providerConfigMap; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
providerConfigs
public static final ConfigOption<Integer> RPC_SERVER_PORT = | ||
new ConfigOption<>( | ||
"rpc.server_port", | ||
"The port of rpc server for rpc client calling.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The port bound by rpc server to provide services
public static final ConfigOption<String> RPC_SERVER_HOST = | ||
new ConfigOption<>( | ||
"rpc.server_host", | ||
"The specified host of rpc server for rpc client " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The host/ip bound by rpc server to provide services
new ConfigOption<>( | ||
"rpc.config_order", | ||
"Sofa rpc loading order, the larger the more after." + | ||
"loading", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to previous line
public static final ConfigOption<Integer> RPC_CONFIG_ORDER = | ||
new ConfigOption<>( | ||
"rpc.config_order", | ||
"Sofa rpc loading order, the larger the more after." + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems "loading" is behind "."
"rpc.client_load_balancer", | ||
"The rpc client uses a load-balancing algorithm to " + | ||
"configure the addresses of multiple rpc servers. Default " + | ||
"value is 'consistentHash', means forwording by parameters.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by request parameters
rpc.server_port=8899 | ||
rpc.server_host=172.24.174.30 | ||
rpc.server_host=172.24.174.28 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep 127.0.0.1
"The specified host of rpc server for rpc client " + | ||
"call.", | ||
"The multiple hosts/ips bound by rpc server to provide " + | ||
"services. Multiple hosts/ips link them with ',' together.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multiple host -- seems describe AUTH_REMOTE_URL
|
||
public void unExport(String serviceName) { | ||
if (!this.providerConfigs.containsKey(serviceName)) { | ||
throw new RpcException("Service name '%s' is not exist, please " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The service name '%s' doesn't exist
} | ||
|
||
public Map<String, ProviderConfig> providerConfigs() { | ||
return configs; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.configs
|
||
public void exportAll() { | ||
if (MapUtils.isEmpty(this.providerConfigs)) { | ||
throw new RpcException("Server provider config map is empty"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The server provider config map can't be empty
.setRetries(conf.get(ServerOptions.RPC_CLIENT_RETRIES)) | ||
.setLoadBalancer(conf.get( | ||
ServerOptions.RPC_CLIENT_LOAD_BALANCER)); | ||
configs.put(clazz.getName(), consumerConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.configs
if (!configs.containsKey(serverName)) { | ||
throw new RpcException("Invalid server name '%s'", serverName); | ||
} | ||
return configs.get(serverName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
ea74775
to
b6664c5
Compare
@@ -76,7 +76,7 @@ public String system() { | |||
@Timed | |||
@Path("backend") | |||
@Produces(APPLICATION_JSON_WITH_CHARSET) | |||
@RolesAllowed({"admin", "$owner= $action=metrics_read"}) | |||
// @RolesAllowed({"admin", "$owner= $action=metrics_read"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep the origin
"auth.remote_url", | ||
"If the address is empty, it provide auth service, " + | ||
"otherwise it is auth client and also provide auth service " + | ||
"through rpc forwarding. The remote url can set multiple " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remote url can be set to multiple addresses
new ConfigOption<>( | ||
"rpc.client_load_balancer", | ||
"The rpc client uses a load-balancing algorithm to " + | ||
"configure the addresses of multiple rpc servers. Default " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to access multiple rpc servers in one cluster
} | ||
|
||
private void destoryRpcServer() { | ||
this.rpcServerProvider.destroy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call if rpcServerProvider != null
|
||
public void exportAll() { | ||
if (MapUtils.isEmpty(this.providerConfigs)) { | ||
throw new RpcException("The server provider config map can't be " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrap line after "(" and align with RpcException
"bolt" | ||
); | ||
|
||
public static final ConfigOption<Integer> RPC_CONFIG_ORDER = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the option required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this option is not set, the sofa-rpc log may be lost.
|
||
public static final ConfigOption<Integer> RPC_CLIENT_CONNECTION_TIMEOUT = | ||
new ConfigOption<>( | ||
"rpc.client_connection_timeout", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to client_connect_timeout
and add client_reconnect_timeout
hugegraph-api/src/main/java/com/baidu/hugegraph/config/ServerOptions.java
Show resolved
Hide resolved
"The timeout(in seconds) of rpc client read from rpc " + | ||
"server.", | ||
rangeInt(1, Integer.MAX_VALUE), | ||
8000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
40s
"The timeout(in seconds) of rpc client connect to rpc " + | ||
"server.", | ||
rangeInt(1, Integer.MAX_VALUE), | ||
8000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20s
2ed6974
to
3335585
Compare
3335585
to
eec6ae2
Compare
@@ -35,9 +35,11 @@ | |||
.setInterfaceId(clazz.getName()) | |||
.setProtocol(conf.get(ServerOptions.RPC_PROTOCOL)) | |||
.setDirectUrl(conf.get(ServerOptions.AUTH_REMOTE_URL)) | |||
.setTimeout(conf.get(ServerOptions.RPC_CLIENT_READ_TIMEOUT)) | |||
.setTimeout(conf.get(ServerOptions.CLIENT_READ_TIMEOUT)*1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add space
.setConnectTimeout(conf.get( | ||
ServerOptions.RPC_CLIENT_CONNECTION_TIMEOUT)) | ||
ServerOptions.CLIENT_CONNECTION_TIMEOUT)*1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add space
ServerOptions.RPC_CLIENT_CONNECTION_TIMEOUT)) | ||
ServerOptions.CLIENT_CONNECTION_TIMEOUT)*1000) | ||
.setReconnectPeriod(conf.get( | ||
ServerOptions.CLIENT_RECONNECTION_TIMEOUT) *1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add space
|
||
public static final ConfigOption<Integer> CLIENT_RECONNECTION_TIMEOUT = | ||
new ConfigOption<>( | ||
"rpc.client_reconnection_timeout", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
client_connect_timeout and client_reconnect_timeout, not connection
@@ -35,6 +35,7 @@ | |||
|
|||
private Map<String, ProviderConfig> providerConfigs; | |||
private ServerConfig serverConfig; | |||
private final int rpcServerTimeout; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also set final for serverConfig and providerConfigs
rpc.client_read_timeout=8000 | ||
rpc.server_timeout=30 | ||
rpc.client_connection_timeout=20 | ||
rpc.client_reconnection_timeout=20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
…timize some code.
int connectTimeout = conf.get(ServerOptions | ||
.CLIENT_CONNECT_TIMEOUT) * 1000; | ||
int reconnectPeriod = conf.get(ServerOptions | ||
.CLIENT_RECONNECT_TIMEOUT) * 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to CLIENT_RECONNECT_PERIOD
@@ -258,18 +258,18 @@ public static synchronized ServerOptions instance() { | |||
30 | |||
); | |||
|
|||
public static final ConfigOption<Integer> CLIENT_CONNECTION_TIMEOUT = | |||
public static final ConfigOption<Integer> CLIENT_CONNECT_TIMEOUT = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add RPC prefix: RPC_CLIENT_CONNECT_TIMEOUT
Codecov Report
@@ Coverage Diff @@
## master #1350 +/- ##
============================================
- Coverage 62.49% 62.39% -0.11%
- Complexity 5816 5818 +2
============================================
Files 379 385 +6
Lines 31766 31922 +156
Branches 4443 4451 +8
============================================
+ Hits 19853 19917 +64
- Misses 9895 9984 +89
- Partials 2018 2021 +3
Continue to review full report at Codecov.
|
@@ -13,3 +13,14 @@ graphs=[hugegraph:conf/hugegraph.properties] | |||
|
|||
server.id=server-1 | |||
server.role=master | |||
|
|||
#auth.remote_url=10.103.168.25:8899,10.103.168.25:8898,10.103.168.25:8897 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use 127.0.0.1 instead of test machine ip
|
||
#auth.remote_url=10.103.168.25:8899,10.103.168.25:8898,10.103.168.25:8897 | ||
rpc.server_port=8899 | ||
rpc.server_host=127.0.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move server_host
before server_host
@Override | ||
public void onEvent(ApplicationEvent event) { | ||
if (event.getType() == this.EVENT_INITED) { | ||
manager = new GraphManager(conf); | ||
} else if(event.getType() == this.EVENT_DESTROY) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a space after "if"
20 | ||
); | ||
|
||
public static final ConfigOption<Integer> CLIENT_RECONNECT_PERIOD = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer adding "RPC_" prefix also
20 | ||
); | ||
|
||
public static final ConfigOption<Integer> CLIENT_READ_TIMEOUT = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -76,7 +76,7 @@ public String system() { | |||
@Timed | |||
@Path("backend") | |||
@Produces(APPLICATION_JSON_WITH_CHARSET) | |||
@RolesAllowed({"admin", "$owner= $action=metrics_read"}) | |||
// @RolesAllowed({"admin", "$owner= $action=metrics_read"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this comment expected?
@@ -267,7 +267,7 @@ public static synchronized ServerOptions instance() { | |||
20 | |||
); | |||
|
|||
public static final ConfigOption<Integer> CLIENT_RECONNECT_PERIOD = | |||
public static final ConfigOption<Integer> RPC_CLIENT_RECONNECT_PERIOD = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also update rpc.client_reconnect_timeout
new ConfigOption<>( | ||
"rpc.protocol", | ||
"Rpc communication protocol, client and server need to " + | ||
"be specified at the same time, and can match.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
client and server need to be specified the same value
new ConfigOption<>( | ||
"rpc.config_order", | ||
"Sofa rpc configuration file loading order, the larger " + | ||
"the more later loading", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add "."
public static final ConfigOption<Integer> RPC_CLIENT_RETRIES = | ||
new ConfigOption<>( | ||
"rpc.client_retries", | ||
"Failed retry number of rpc client calls to rpc Server.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Server => server -- to keep the same style with other options
new ConfigOption<>( | ||
"rpc.client_retries", | ||
"Failed retry number of rpc client calls to rpc Server.", | ||
rangeInt(1, Integer.MAX_VALUE), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can the min value be 0?
@@ -0,0 +1,23 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this file
90e6d1b
to
fd508d6
Compare
@@ -309,7 +309,7 @@ public static synchronized ServerOptions instance() { | |||
new ConfigOption<>( | |||
"rpc.protocol", | |||
"Rpc communication protocol, client and server need to " + | |||
"be specified at the same time, and can match.", | |||
"be specified the same value, and can match.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove " and can match"
"The hosts/ips bound by rpc server to provide " + | ||
"services.", | ||
disallowEmpty(), | ||
"0.0.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set default value 127.0.0.1
@@ -13,3 +13,14 @@ graphs=[hugegraph:conf/hugegraph.properties] | |||
|
|||
server.id=server-1 | |||
server.role=master | |||
|
|||
#auth.remote_url=127.0.0.1:8899,127.0.0.1:8898,127.0.0.1:8897 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to line 13
@@ -652,6 +652,12 @@ public UserManager userManager() { | |||
return this.userManager; | |||
} | |||
|
|||
@Override | |||
public void swichUserManager(UserManager userManager) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switch -> switch
@Override | ||
public void swichUserManager(UserManager userManager) { | ||
this.verifyAdminPermission(); | ||
this.userManager.swichUserManager(userManager); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swich -> switch
@@ -1345,6 +1351,11 @@ public RolePermission loginUser(String username, String password) { | |||
setContext(context); | |||
} | |||
} | |||
|
|||
private void swichUserManager(UserManager userManager) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swich -> switch
|
||
private void swichUserManager(UserManager userManager) { | ||
this.userManager = userManager; | ||
hugegraph.swichUserManager(userManager); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -155,6 +155,7 @@ | |||
public HugeFeatures features(); | |||
|
|||
public UserManager userManager(); | |||
public void swichUserManager(UserManager userManager); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swich -> switch
@@ -883,6 +883,11 @@ public UserManager userManager() { | |||
return this.userManager; | |||
} | |||
|
|||
@Override | |||
public void swichUserManager(UserManager userManager) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swich -> switch
Map<String, ProviderConfig> configs = this.configs.configs(); | ||
if (!configs.containsKey(serviceName)) { | ||
throw new RpcException("The service name '%s' doesn't exist, " + | ||
"please change others", serviceName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove "please change others"
LOG.debug("RpcServer starting on port {}", this.port()); | ||
Map<String, ProviderConfig> configs = this.configs.configs(); | ||
if (MapUtils.isEmpty(configs)) { | ||
LOG.info("RpcServer configs is empty, skip RpcServer starting"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RpcServer config is empty
No description provided.