pkuwm commented on a change in pull request #819: Make RealmAwareZkClient
implementations use HttpRoutingDataReader for routing data
URL: https://github.com/apache/helix/pull/819#discussion_r387442475
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/DedicatedZkClient.java
##########
@@ -457,17 +470,17 @@ public PathBasedZkSerializer getZkSerializer() {
* @return
*/
private void checkIfPathContainsShardingKey(String path) {
- // TODO: replace with the singleton MetadataStoreRoutingData
try {
- String zkRealmForPath =
_metadataStoreRoutingData.getMetadataStoreRealm(path);
- if (!_zkRealmAddress.equals(zkRealmForPath)) {
- throw new IllegalArgumentException("Given path: " + path + "'s ZK
realm: " + zkRealmForPath
- + " does not match the ZK realm: " + _zkRealmAddress + " and
sharding key: "
- + _zkRealmShardingKey + " for this DedicatedZkClient!");
+ String targetShardingKey =
_metadataStoreRoutingData.getShardingKeyInPath(path);
+ if (!_zkRealmShardingKey.equals(targetShardingKey)) {
Review comment:
Nit, Shall we move this condition check out of try...catch block? I
understand it not may be unrelated to your change. But since you already touch
it and change the name :)
One more thing: This could throw NPE. `_zkRealmShardingKey` could be null if
it is not set in connectionConfig when the user expects it to be a multi-realm
mode by default.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
