You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "Nortyr (via GitHub)" <gi...@apache.org> on 2023/10/19 13:54:17 UTC
[PR] Fixes #9913,Redis metadataReport implementation does not support regi… [dubbo]
Nortyr opened a new pull request, #13238:
URL: https://github.com/apache/dubbo/pull/13238
…sterServiceAppMapping() method
## What is the purpose of the change
see:#12627
## Brief changelog
## Verifying this change
<!-- Follow this checklist to help us incorporate your contribution quickly and easily: -->
## Checklist
- [x] Make sure there is a [GitHub_issue](https://github.com/apache/dubbo/issues) field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
- [ ] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in [dubbo samples](https://github.com/apache/dubbo-samples) project.
- [ ] Add some description to [dubbo-website](https://github.com/apache/dubbo-website) project if you are requesting to add a feature.
- [ ] GitHub Actions works fine on your own branch.
- [ ] If this contribution is large, please follow the [Software Donation Guide](https://github.com/apache/dubbo/wiki/Software-donation-guide).
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1382384517
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,261 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
Review Comment:
Close this one and track in #13303 in the future
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1368349014
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +245,242 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
+ }
+ String pathKey = buildMappingKey(defaultMappingGroup);
+
+ return storeMapping(pathKey, serviceInterface, newConfigContent,(String)ticket);
+ } catch (Exception e) {
+ logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "redis publishConfigCas failed.", e);
+ return false;
+ }
+ }
+
+
+ private boolean storeMapping(String key, String field, String value,String ticket) {
+ if (pool != null) {
+ return storeMappingStandalone(key, field, value, ticket);
+ } else {
+ return storeMappingInCluster(key, field, value, ticket);
+ }
+ }
+
+ private boolean storeMappingInCluster(String key, String field, String value,String ticket) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ Object result =jedisCluster.eval(luaScript, 1, key, field, ticket==null?"":ticket, value,buildPubSubKey(field));
+ return null==result;
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ }
+
+ private boolean storeMappingStandalone(String key, String field, String value,String ticket) {
+ try (Jedis jedis = pool.getResource()) {
+ Object result = jedis.eval(luaScript, 1, key, field, ticket==null?"":ticket, value,buildPubSubKey(field));
+ return null==result;
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ }
+
+ private String buildMappingKey(String defaultMappingGroup) {
+ return this.root + GROUP_CHAR_SEPARATOR + defaultMappingGroup;
+ }
+
+ private String buildPubSubKey(String serviceKey) {
+ return buildMappingKey(DEFAULT_MAPPING_GROUP) + GROUP_CHAR_SEPARATOR + serviceKey;
+ }
+
+ @Override
+ public ConfigItem getConfigItem(String serviceKey, String group) {
+ String key = buildMappingKey(group);
+ String content = getMappingData(key, serviceKey);
+
+ return new ConfigItem(content, content);
+ }
+
+ private String getMappingData(String key, String field) {
+ if (pool != null) {
+ return getMappingDataStandalone(key, field);
+ } else {
+ return getMappingDataInCluster(key, field);
+ }
+ }
+
+ private String getMappingDataInCluster(String key, String field) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ return jedisCluster.hget(key, field);
+ } catch (Throwable e) {
+ String msg = "Failed to get " + key + ":" + field + " from redis cluster , cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ }
+
+ private String getMappingDataStandalone(String key, String field) {
+ try (Jedis jedis = pool.getResource()) {
+ return jedis.hget(key, field);
+ } catch (Throwable e) {
+ String msg = "Failed to get " + key + ":" + field + " from redis , cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ }
+
+ @Override
+ public void removeServiceAppMappingListener(String serviceKey, MappingListener listener) {
+ if (null != listenerMap.get(serviceKey)) {
+ MappingDataListener mappingDataListener=listenerMap.get(serviceKey);
+ NotifySub notifySub=mappingDataListener.getNotifySub();
+ notifySub.removeListener(listener);
+ if(notifySub.isEmpty()){
+ mappingDataListener.shutdown();
+ listenerMap.remove(serviceKey,mappingDataListener);
+ }
+ }
+ }
+
+ @Override
+ public Set<String> getServiceAppMapping(String serviceKey, MappingListener listener, URL url) {
+ if (null == listenerMap.get(serviceKey)) {
+ NotifySub notifySub = new NotifySub(serviceKey);
+ notifySub.addListener(listener);
+ MappingDataListener dataListener = new MappingDataListener(buildPubSubKey(serviceKey), notifySub);
+ ConcurrentHashMapUtils.computeIfAbsent(listenerMap, serviceKey
+ , k -> dataListener);
+ dataListener.start();
+ }else{
+ listenerMap.get(serviceKey).getNotifySub().addListener(listener);
+ }
+ return this.getServiceAppMapping(serviceKey, url);
+ }
+
+ @Override
+ public Set<String> getServiceAppMapping(String serviceKey, URL url) {
+ String key = buildMappingKey(DEFAULT_MAPPING_GROUP);
+ return getAppNames(getMappingData(key, serviceKey));
+
+ }
+
+ @Override
+ public MetadataInfo getAppMetadata(SubscriberMetadataIdentifier identifier, Map<String, String> instanceMetadata) {
+ String content = this.getMetadata(identifier);
+ return JsonUtils.toJavaObject(content, MetadataInfo.class);
+ }
+
+ @Override
+ public void publishAppMetadata(SubscriberMetadataIdentifier identifier, MetadataInfo metadataInfo) {
+ this.storeMetadata(identifier, metadataInfo.getContent());
+ }
+
+ @Override
+ public void unPublishAppMetadata(SubscriberMetadataIdentifier identifier, MetadataInfo metadataInfo) {
+ this.deleteMetadata(identifier);
+ }
+
+ private static class NotifySub extends JedisPubSub {
+
+ private String serviceKey;
+ private Set<MappingListener> listeners = new HashSet<>();
+
+ public NotifySub(String serviceKey) {
+ this.serviceKey = serviceKey;
+ }
+
+ public void addListener(MappingListener listener) {
+ this.listeners.add(listener);
+ }
+
+ public void removeListener(MappingListener listener) {
+ this.listeners.remove(listener);
+ }
+
+ public Boolean isEmpty(){
+ return this.listeners.isEmpty();
+ }
+
+ @Override
+ public void onMessage(String key, String msg) {
+ logger.info("sub from redis " + key + " message:" + msg);
+ MappingChangedEvent mappingChangedEvent = new MappingChangedEvent(serviceKey, getAppNames(msg));
+ if(!listeners.isEmpty()){
+ listeners.forEach(listener -> listener.onEvent(mappingChangedEvent));
+ }
+
+ }
+
+ @Override
+ public void onPMessage(String pattern, String key, String msg) {
+ onMessage(key, msg);
+ }
+
+ @Override
+ public void onPSubscribe(String pattern, int subscribedChannels) {
+ super.onPSubscribe(pattern, subscribedChannels);
+ }
+ }
+
+ private class MappingDataListener extends Thread {
+
+ private String path;
+
+ private volatile Jedis currentClient;
Review Comment:
This variable is assigned but never accessed.
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Nortyr (via GitHub)" <gi...@apache.org>.
Nortyr commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1371601736
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,258 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
+ }
+ String pathKey = buildMappingKey(defaultMappingGroup);
+
+ return storeMapping(pathKey, serviceInterface, newConfigContent,(String)ticket);
+ } catch (Exception e) {
+ logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "redis publishConfigCas failed.", e);
+ return false;
+ }
+ }
+
+
+ private boolean storeMapping(String key, String field, String value,String ticket) {
+ if (pool != null) {
+ return storeMappingStandalone(key, field, value, ticket);
+ } else {
+ return storeMappingInCluster(key, field, value, ticket);
+ }
+ }
+
+ private boolean storeMappingInCluster(String key, String field, String value,String ticket) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ Jedis jedis=jedisCluster.getConnectionFromSlot(JedisClusterCRC16.getSlot(key));
+ jedis.watch(key);
+ String oldValue = jedis.get(key);
+ if (null==oldValue||null==ticket||oldValue.equals(ticket)) {
+ Transaction transaction = jedis.multi();
+ transaction.hset(key,field,value);
+ List<Object> result=transaction.exec();
+ if(null!=result){
+ jedisCluster.publish(buildPubSubKey(field),value);
+ return true;
+ }
+ }
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ return false;
+ }
+
+ private boolean storeMappingStandalone(String key, String field, String value,String ticket) {
+ try (Jedis jedis = pool.getResource()) {
+ jedis.watch(key);
Review Comment:
emmm,done
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627,Redis metadataReport implementation does not support regi… [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#issuecomment-1771145557
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=13238)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list)
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1368348587
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
Starting from version 2.2, redis provides `WATCH` to help implement [cas behavior](https://redis.io/docs/interact/transactions/#cas) in case race conditions may occur.
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1378721612
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,261 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
Review Comment:
Should be redis here
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#issuecomment-1779192653
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=13238)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list) [52.5% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list)
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627,Redis metadataReport implementation does not support regi… [dubbo]
Posted by "AlbumenJ (via GitHub)" <gi...@apache.org>.
AlbumenJ commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1366604081
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +207,8 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ throw new UnsupportedOperationException("Redis metadataReport implementation does not support registerServiceAppMapping() method.");
Review Comment:
To fix https://github.com/apache/dubbo/issues/12627, you should use jedis to set content here.
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#issuecomment-1778748836
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=13238)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list) [53.4% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list)
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Nortyr (via GitHub)" <gi...@apache.org>.
Nortyr commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1371584533
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
已改，之前完全没有注意过UNWATCH，十分感谢
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
已改，之前完全没有注意过UNWATCH，十分感谢
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#issuecomment-1777089006
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=13238)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list) [58.4% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list)
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Nortyr (via GitHub)" <gi...@apache.org>.
Nortyr commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1375815268
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +207,8 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ throw new UnsupportedOperationException("Redis metadataReport implementation does not support registerServiceAppMapping() method.");
Review Comment:
PTAL
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627,Redis metadataReport implementation does not support regi… [dubbo]
Posted by "cgyxx (via GitHub)" <gi...@apache.org>.
cgyxx commented on PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#issuecomment-1772027745
that's good
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Nortyr (via GitHub)" <gi...@apache.org>.
Nortyr commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1371364543
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
不错的建议，PTAL
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
不错的建议，PTAL
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1371554351
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,258 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
+ }
+ String pathKey = buildMappingKey(defaultMappingGroup);
+
+ return storeMapping(pathKey, serviceInterface, newConfigContent,(String)ticket);
+ } catch (Exception e) {
+ logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "redis publishConfigCas failed.", e);
+ return false;
+ }
+ }
+
+
+ private boolean storeMapping(String key, String field, String value,String ticket) {
+ if (pool != null) {
+ return storeMappingStandalone(key, field, value, ticket);
+ } else {
+ return storeMappingInCluster(key, field, value, ticket);
+ }
+ }
+
+ private boolean storeMappingInCluster(String key, String field, String value,String ticket) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ Jedis jedis=jedisCluster.getConnectionFromSlot(JedisClusterCRC16.getSlot(key));
+ jedis.watch(key);
+ String oldValue = jedis.get(key);
+ if (null==oldValue||null==ticket||oldValue.equals(ticket)) {
+ Transaction transaction = jedis.multi();
+ transaction.hset(key,field,value);
+ List<Object> result=transaction.exec();
+ if(null!=result){
+ jedisCluster.publish(buildPubSubKey(field),value);
+ return true;
+ }
+ }
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ return false;
+ }
+
+ private boolean storeMappingStandalone(String key, String field, String value,String ticket) {
+ try (Jedis jedis = pool.getResource()) {
+ jedis.watch(key);
Review Comment:
watch可以考虑移到transaction里。一是因为watch的目的是为了保证在transaction期间key不被更改，二是watch之后需要手动执行[unwatch](https://redis.io/commands/unwatch/)（除非执行过discard或exec）。
建议改成：
```suggestion
Transaction transaction = jedis.multi();
transaction.watch(key);
...
```
或：
```suggestion
jedis.watch(key);
...
List<Object> result=transaction.exec();
jedis.unwatch();
return null!=result;
```
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,258 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
+ }
+ String pathKey = buildMappingKey(defaultMappingGroup);
+
+ return storeMapping(pathKey, serviceInterface, newConfigContent,(String)ticket);
+ } catch (Exception e) {
+ logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "redis publishConfigCas failed.", e);
+ return false;
+ }
+ }
+
+
+ private boolean storeMapping(String key, String field, String value,String ticket) {
+ if (pool != null) {
+ return storeMappingStandalone(key, field, value, ticket);
+ } else {
+ return storeMappingInCluster(key, field, value, ticket);
+ }
+ }
+
+ private boolean storeMappingInCluster(String key, String field, String value,String ticket) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ Jedis jedis=jedisCluster.getConnectionFromSlot(JedisClusterCRC16.getSlot(key));
+ jedis.watch(key);
+ String oldValue = jedis.get(key);
+ if (null==oldValue||null==ticket||oldValue.equals(ticket)) {
+ Transaction transaction = jedis.multi();
+ transaction.hset(key,field,value);
+ List<Object> result=transaction.exec();
+ if(null!=result){
+ jedisCluster.publish(buildPubSubKey(field),value);
+ return true;
+ }
+ }
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ return false;
+ }
+
+ private boolean storeMappingStandalone(String key, String field, String value,String ticket) {
+ try (Jedis jedis = pool.getResource()) {
+ jedis.watch(key);
Review Comment:
watch可以考虑移到transaction里。一是因为watch的目的是为了保证在transaction期间key不被更改，二是watch之后需要手动执行[unwatch](https://redis.io/commands/unwatch/)（除非执行过discard或exec）。
建议改成：
```suggestion
Transaction transaction = jedis.multi();
transaction.watch(key);
...
```
或：
```suggestion
jedis.watch(key);
...
List<Object> result=transaction.exec();
jedis.unwatch();
return null!=result;
```
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1371556210
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
麻烦再改一下watch那里的逻辑，其他部分我觉得ok。
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
麻烦再改一下watch那里的逻辑，其他部分我觉得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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1368350537
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
Review Comment:
This is never used.
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Nortyr (via GitHub)" <gi...@apache.org>.
Nortyr commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1369995013
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
请问下JedisCluster中无法使用watch有什么好的建议呢？
如果不建议使用lua脚本的话，要不然使用setnx实现一个分布式锁？
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
请问下JedisCluster中无法使用watch有什么好的建议呢？
如果不建议使用lua脚本的话，要不然使用setnx实现一个分布式锁？
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1370026407
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
之前有人[讨论](https://groups.google.com/g/jedis_redis/c/Z4-0065UFkg)过在JedisCluster上如何做transaction，基本思路是从sluter里拿到对应key所在的Jedis，然后在上面开transaction，只不过这样做的话会稍微麻烦点。
我觉得用什么方式都无所谓，只要能实现目标就行。
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
之前有人[讨论](https://groups.google.com/g/jedis_redis/c/Z4-0065UFkg)过在JedisCluster上如何做transaction，基本思路是从sluter里拿到对应key所在的Jedis，然后在上面开transaction，只不过这样做的话会稍微麻烦点。
我觉得用什么方式都无所谓，只要能实现目标就行。
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1371554351
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,258 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
+ }
+ String pathKey = buildMappingKey(defaultMappingGroup);
+
+ return storeMapping(pathKey, serviceInterface, newConfigContent,(String)ticket);
+ } catch (Exception e) {
+ logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "redis publishConfigCas failed.", e);
+ return false;
+ }
+ }
+
+
+ private boolean storeMapping(String key, String field, String value,String ticket) {
+ if (pool != null) {
+ return storeMappingStandalone(key, field, value, ticket);
+ } else {
+ return storeMappingInCluster(key, field, value, ticket);
+ }
+ }
+
+ private boolean storeMappingInCluster(String key, String field, String value,String ticket) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ Jedis jedis=jedisCluster.getConnectionFromSlot(JedisClusterCRC16.getSlot(key));
+ jedis.watch(key);
+ String oldValue = jedis.get(key);
+ if (null==oldValue||null==ticket||oldValue.equals(ticket)) {
+ Transaction transaction = jedis.multi();
+ transaction.hset(key,field,value);
+ List<Object> result=transaction.exec();
+ if(null!=result){
+ jedisCluster.publish(buildPubSubKey(field),value);
+ return true;
+ }
+ }
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ return false;
+ }
+
+ private boolean storeMappingStandalone(String key, String field, String value,String ticket) {
+ try (Jedis jedis = pool.getResource()) {
+ jedis.watch(key);
Review Comment:
watch可以考虑移到transaction里。一是因为watch的目的是为了保证在transaction期间key不被更改，二是watch之后需要手动执行[unwatch](https://redis.io/commands/unwatch/)（除非执行过discard或exec）。
建议改成：
```suggestion
Transaction transaction = jedis.multi();
transaction.watch(key);
...
```
或：
```suggestion
jedis.watch(key);
...
List<Object> result=transaction.exec();
return null!=result;
}
jedis.unwatch();
```
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1371554351
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,258 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
+ }
+ String pathKey = buildMappingKey(defaultMappingGroup);
+
+ return storeMapping(pathKey, serviceInterface, newConfigContent,(String)ticket);
+ } catch (Exception e) {
+ logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "redis publishConfigCas failed.", e);
+ return false;
+ }
+ }
+
+
+ private boolean storeMapping(String key, String field, String value,String ticket) {
+ if (pool != null) {
+ return storeMappingStandalone(key, field, value, ticket);
+ } else {
+ return storeMappingInCluster(key, field, value, ticket);
+ }
+ }
+
+ private boolean storeMappingInCluster(String key, String field, String value,String ticket) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ Jedis jedis=jedisCluster.getConnectionFromSlot(JedisClusterCRC16.getSlot(key));
+ jedis.watch(key);
+ String oldValue = jedis.get(key);
+ if (null==oldValue||null==ticket||oldValue.equals(ticket)) {
+ Transaction transaction = jedis.multi();
+ transaction.hset(key,field,value);
+ List<Object> result=transaction.exec();
+ if(null!=result){
+ jedisCluster.publish(buildPubSubKey(field),value);
+ return true;
+ }
+ }
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ return false;
+ }
+
+ private boolean storeMappingStandalone(String key, String field, String value,String ticket) {
+ try (Jedis jedis = pool.getResource()) {
+ jedis.watch(key);
Review Comment:
watch之后需要手动执行[unwatch](https://redis.io/commands/unwatch/)（除非执行过discard或exec）。
建议在try的最后加一行：
```suggestion
jedis.watch(key);
...
List<Object> result=transaction.exec();
return null!=result;
}
jedis.unwatch();
```
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Nortyr (via GitHub)" <gi...@apache.org>.
Nortyr closed pull request #13238: Fixes#12627, Redis metadataReport implementation
URL: https://github.com/apache/dubbo/pull/13238
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#issuecomment-1791857809
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=13238)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list) [52.5% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list)
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1370026407
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
之前有人[讨论](https://groups.google.com/g/jedis_redis/c/Z4-0065UFkg)过在JedisCluster上如何做transaction，基本思路是从sluter里拿到对应key所在的Jedis，然后在上面开transaction，只不过这样做的话会稍微麻烦点。
我觉得用什么方式都无所谓，只要能实现目标就行。
从我的角度上看的话，毕竟官方提供了watch作为保险措施，能加最好还是加一下，免得为以后挖坑。
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Nortyr (via GitHub)" <gi...@apache.org>.
Nortyr commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1369995013
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -61,12 +77,34 @@ public class RedisMetadataReport extends AbstractMetadataReport {
private Set<HostAndPort> jedisClusterNodes;
private int timeout;
private String password;
+ private final String root;
+ private MD5Utils md5Utils = new MD5Utils();
+ protected ConcurrentMap<String, MappingDataListener> listenerMap = new ConcurrentHashMap<>();
+ private String luaScript="local key = KEYS[1];\n" +
+ "local field = ARGV[1];\n" +
+ "local oldValue = ARGV[2];\n" +
+ "local newValue = ARGV[3];\n" +
+ "local channel = ARGV[4];\n" +
+ "local valueExists = redis.call(\"HEXISTS\", key, field);\n" +
+ "if valueExists == 0 then\n" +
+ " redis.call(\"HSET\", key, field, newValue);\n" +
Review Comment:
请问下JedisCluster中无法使用watch有什么好的建议呢？
如果不建议使用lua脚本的话，要不然使用setnx实现一个简易的锁？
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627,Redis metadataReport implementation does not support regi… [dubbo]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#issuecomment-1771153193
## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/13238?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#13238](https://app.codecov.io/gh/apache/dubbo/pull/13238?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (7cad000) into [3.2](https://app.codecov.io/gh/apache/dubbo/commit/416374cd78f87fc20d293e4f2af6e6ef452b198d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (416374c) will **decrease** coverage by `0.01%`.
> Report is 3 commits behind head on 3.2.
> The diff coverage is `n/a`.
> :exclamation: Current head 7cad000 differs from pull request most recent head 847220a. Consider uploading reports for the commit 847220a to get more accurate results
```diff
@@ Coverage Diff @@
## 3.2 #13238 +/- ##
============================================
- Coverage 69.47% 69.46% -0.01%
Complexity 2 2
============================================
Files 1651 1653 +2
Lines 68736 68925 +189
Branches 10012 10041 +29
============================================
+ Hits 47753 47878 +125
- Misses 16385 16422 +37
- Partials 4598 4625 +27
```
[see 45 files with indirect coverage changes](https://app.codecov.io/gh/apache/dubbo/pull/13238/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627,Redis metadataReport implementation does not support regi… [dubbo]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#issuecomment-1771165849
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_dubbo&pullRequest=13238)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo&pullRequest=13238&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_dubbo&pullRequest=13238&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_dubbo&pullRequest=13238&metric=new_duplicated_lines_density&view=list)
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Linchen-Xu (via GitHub)" <gi...@apache.org>.
Linchen-Xu commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1371591292
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,258 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
+ }
+ String pathKey = buildMappingKey(defaultMappingGroup);
+
+ return storeMapping(pathKey, serviceInterface, newConfigContent,(String)ticket);
+ } catch (Exception e) {
+ logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "redis publishConfigCas failed.", e);
+ return false;
+ }
+ }
+
+
+ private boolean storeMapping(String key, String field, String value,String ticket) {
+ if (pool != null) {
+ return storeMappingStandalone(key, field, value, ticket);
+ } else {
+ return storeMappingInCluster(key, field, value, ticket);
+ }
+ }
+
+ private boolean storeMappingInCluster(String key, String field, String value,String ticket) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ Jedis jedis=jedisCluster.getConnectionFromSlot(JedisClusterCRC16.getSlot(key));
+ jedis.watch(key);
+ String oldValue = jedis.get(key);
+ if (null==oldValue||null==ticket||oldValue.equals(ticket)) {
+ Transaction transaction = jedis.multi();
+ transaction.hset(key,field,value);
+ List<Object> result=transaction.exec();
+ if(null!=result){
+ jedisCluster.publish(buildPubSubKey(field),value);
+ return true;
+ }
+ }
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ return false;
+ }
+
+ private boolean storeMappingStandalone(String key, String field, String value,String ticket) {
+ try (Jedis jedis = pool.getResource()) {
+ jedis.watch(key);
Review Comment:
> watch之后需要手动执行[unwatch](https://redis.io/commands/unwatch/)（除非执行过discard或exec）。 建议在try的最后加一行：
@Nortyr 麻烦再改下，想了下watch移到里面就没有意义了，还是应该写成watch+判断并执行+unwatch的形式
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,258 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
+ }
+ String pathKey = buildMappingKey(defaultMappingGroup);
+
+ return storeMapping(pathKey, serviceInterface, newConfigContent,(String)ticket);
+ } catch (Exception e) {
+ logger.warn(REGISTRY_ZOOKEEPER_EXCEPTION, "", "", "redis publishConfigCas failed.", e);
+ return false;
+ }
+ }
+
+
+ private boolean storeMapping(String key, String field, String value,String ticket) {
+ if (pool != null) {
+ return storeMappingStandalone(key, field, value, ticket);
+ } else {
+ return storeMappingInCluster(key, field, value, ticket);
+ }
+ }
+
+ private boolean storeMappingInCluster(String key, String field, String value,String ticket) {
+ try (JedisCluster jedisCluster = new JedisCluster(jedisClusterNodes, timeout, timeout, 2, password, new GenericObjectPoolConfig<>())) {
+ Jedis jedis=jedisCluster.getConnectionFromSlot(JedisClusterCRC16.getSlot(key));
+ jedis.watch(key);
+ String oldValue = jedis.get(key);
+ if (null==oldValue||null==ticket||oldValue.equals(ticket)) {
+ Transaction transaction = jedis.multi();
+ transaction.hset(key,field,value);
+ List<Object> result=transaction.exec();
+ if(null!=result){
+ jedisCluster.publish(buildPubSubKey(field),value);
+ return true;
+ }
+ }
+ } catch (Throwable e) {
+ String msg = "Failed to put " + key + ":" + field + " to redis " + value + ", cause: " + e.getMessage();
+ logger.error(TRANSPORT_FAILED_RESPONSE, "", "", msg, e);
+ throw new RpcException(msg, e);
+ }
+ return false;
+ }
+
+ private boolean storeMappingStandalone(String key, String field, String value,String ticket) {
+ try (Jedis jedis = pool.getResource()) {
+ jedis.watch(key);
Review Comment:
> watch之后需要手动执行[unwatch](https://redis.io/commands/unwatch/)（除非执行过discard或exec）。 建议在try的最后加一行：
@Nortyr 麻烦再改下，想了下watch移到里面就没有意义了，还是应该写成watch+判断并执行+unwatch的形式
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Fixes#12627, Redis metadataReport implementation [dubbo]
Posted by "Nortyr (via GitHub)" <gi...@apache.org>.
Nortyr commented on code in PR #13238:
URL: https://github.com/apache/dubbo/pull/13238#discussion_r1381278027
##########
dubbo-metadata/dubbo-metadata-report-redis/src/main/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReport.java:
##########
@@ -207,4 +226,261 @@ private String getMetadataStandalone(BaseMetadataIdentifier metadataIdentifier)
}
}
+ @Override
+ public boolean registerServiceAppMapping(String serviceInterface, String defaultMappingGroup, String newConfigContent, Object ticket) {
+ try {
+ if (null!= ticket && !(ticket instanceof String)) {
+ throw new IllegalArgumentException("zookeeper publishConfigCas requires stat type ticket");
Review Comment:
Okay, I will fix it in this link: https://github.com/apache/dubbo/pull/13303. Should I close this PR or that one?
--
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@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org