This is an automated email from the ASF dual-hosted git repository. hulee pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/helix.git
commit e3f751cae70aea05213df508cc6c9395f0e125de Author: Hunter Lee <[email protected]> AuthorDate: Fri Mar 20 21:28:38 2020 -0700 Use Java Generics and inheritance to reduce duplicate code in Helix API Builders (#899) This PR removes duplicate logic and refactors the ZK helix API Builder logic into one single public abstract class so that other Builders can inherit from it. It makes use of Builder inheritance and Java Generics. This PR promotes code reuse and better craftsmanship. --- .../main/java/org/apache/helix/ConfigAccessor.java | 96 +--------- .../manager/zk/GenericBaseDataAccessorBuilder.java | 145 +++++++++++++++ .../helix/manager/zk/GenericZkHelixApiBuilder.java | 139 ++++++++++++++ .../org/apache/helix/manager/zk/ZKHelixAdmin.java | 94 +--------- .../helix/manager/zk/ZkBaseDataAccessor.java | 173 +++-------------- .../helix/manager/zk/ZkCacheBaseDataAccessor.java | 204 ++------------------- .../java/org/apache/helix/tools/ClusterSetup.java | 88 +-------- .../BestPossibleExternalViewVerifier.java | 39 ++-- .../ClusterVerifiers/ClusterLiveNodesVerifier.java | 30 +-- .../StrictMatchExternalViewVerifier.java | 37 +--- .../ClusterVerifiers/ZkHelixClusterVerifier.java | 103 +++++------ .../zookeeper/api/client/RealmAwareZkClient.java | 6 + .../helix/zookeeper/api/client/ZkClientType.java | 42 +++++ 13 files changed, 469 insertions(+), 727 deletions(-) diff --git a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java index 896f936..41f7365 100644 --- a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java +++ b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import org.apache.helix.manager.zk.GenericZkHelixApiBuilder; import org.apache.helix.manager.zk.ZKUtil; import org.apache.helix.model.ClusterConfig; import org.apache.helix.model.CloudConfig; @@ -83,32 +84,9 @@ public class ConfigAccessor { // This is used for close() to determine how ConfigAccessor should close the underlying ZkClient private final boolean _usesExternalZkClient; - /** - * Constructor that creates a realm-aware ConfigAccessor using a builder. - * @param builder - */ - private ConfigAccessor(Builder builder) { - switch (builder._realmMode) { - case MULTI_REALM: - try { - _zkClient = new FederatedZkClient(builder._realmAwareZkConnectionConfig, - builder._realmAwareZkClientConfig.setZkSerializer(new ZNRecordSerializer())); - } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { - throw new HelixException("Failed to create ConfigAccessor!", e); - } - break; - case SINGLE_REALM: - // Create a HelixZkClient: Use a SharedZkClient because ConfigAccessor does not need to do - // ephemeral operations - _zkClient = SharedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(builder._zkAddress), - builder._realmAwareZkClientConfig.createHelixZkClientConfig() - .setZkSerializer(new ZNRecordSerializer())); - break; - default: - throw new HelixException("Invalid RealmMode given: " + builder._realmMode); - } - _usesExternalZkClient = false; + private ConfigAccessor(RealmAwareZkClient zkClient, boolean usesExternalZkClient) { + _zkClient = zkClient; + _usesExternalZkClient = usesExternalZkClient; } /** @@ -993,73 +971,15 @@ public class ConfigAccessor { } } - public static class Builder { - private String _zkAddress; - private RealmAwareZkClient.RealmMode _realmMode; - private RealmAwareZkClient.RealmAwareZkConnectionConfig _realmAwareZkConnectionConfig; - private RealmAwareZkClient.RealmAwareZkClientConfig _realmAwareZkClientConfig; - + public static class Builder extends GenericZkHelixApiBuilder<Builder> { public Builder() { } - public Builder setZkAddress(String zkAddress) { - _zkAddress = zkAddress; - return this; - } - - public Builder setRealmMode(RealmAwareZkClient.RealmMode realmMode) { - _realmMode = realmMode; - return this; - } - - public Builder setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - _realmAwareZkConnectionConfig = realmAwareZkConnectionConfig; - return this; - } - - public Builder setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - _realmAwareZkClientConfig = realmAwareZkClientConfig; - return this; - } - public ConfigAccessor build() { validate(); - return new ConfigAccessor(this); - } - - /** - * Validate the given parameters before creating an instance of ConfigAccessor. - */ - private void validate() { - // Resolve RealmMode based on other parameters - boolean isZkAddressSet = _zkAddress != null && !_zkAddress.isEmpty(); - if (_realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM && !isZkAddressSet) { - throw new HelixException( - "ConfigAccessor: RealmMode cannot be single-realm without a valid ZkAddress set!"); - } - if (_realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM && isZkAddressSet) { - throw new HelixException( - "ConfigAccessor: You cannot set the ZkAddress on multi-realm mode!"); - } - - if (_realmMode == null) { - _realmMode = isZkAddressSet ? RealmAwareZkClient.RealmMode.SINGLE_REALM - : RealmAwareZkClient.RealmMode.MULTI_REALM; - } - - // Resolve RealmAwareZkClientConfig - if (_realmAwareZkClientConfig == null) { - _realmAwareZkClientConfig = new RealmAwareZkClient.RealmAwareZkClientConfig(); - } - - // Resolve RealmAwareZkConnectionConfig - if (_realmAwareZkConnectionConfig == null) { - // If not set, create a default one - _realmAwareZkConnectionConfig = - new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build(); - } + return new ConfigAccessor( + createZkClient(_realmMode, _realmAwareZkConnectionConfig, _realmAwareZkClientConfig, + _zkAddress), false); } } } diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/GenericBaseDataAccessorBuilder.java b/helix-core/src/main/java/org/apache/helix/manager/zk/GenericBaseDataAccessorBuilder.java new file mode 100644 index 0000000..d19ebb1 --- /dev/null +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/GenericBaseDataAccessorBuilder.java @@ -0,0 +1,145 @@ +package org.apache.helix.manager.zk; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import java.io.IOException; +import java.util.concurrent.TimeUnit; + +import org.apache.helix.HelixException; +import org.apache.helix.msdcommon.exception.InvalidRoutingDataException; +import org.apache.helix.zookeeper.api.client.HelixZkClient; +import org.apache.helix.zookeeper.api.client.RealmAwareZkClient; +import org.apache.helix.zookeeper.api.client.ZkClientType; +import org.apache.helix.zookeeper.impl.client.FederatedZkClient; +import org.apache.helix.zookeeper.impl.factory.DedicatedZkClientFactory; +import org.apache.helix.zookeeper.impl.factory.SharedZkClientFactory; + + +/** + * GenericBaseDataAccessorBuilder serves as the abstract parent class for Builders used by + * BaseDataAccessor APIs that create ZK connections. By having this class, we promote code-reuse. + * @param <B> + */ +public class GenericBaseDataAccessorBuilder<B extends GenericBaseDataAccessorBuilder<B>> extends GenericZkHelixApiBuilder<B> { + /** ZK-based BaseDataAccessor-specific parameter **/ + private ZkClientType _zkClientType; + + /** + * Sets the ZkClientType. + * If this is set to either DEDICATED or SHARED, this accessor will be created on + * single-realm mode. + * If this is set to FEDERATED, multi-realm mode will be used. + * @param zkClientType + * @return + */ + public B setZkClientType(ZkBaseDataAccessor.ZkClientType zkClientType) { + return setZkClientType(Enum.valueOf(ZkClientType.class, zkClientType.name())); + } + + public B setZkClientType(ZkClientType zkClientType) { + _zkClientType = zkClientType; + return self(); + } + + /** + * Validates the given parameters before building an instance of ZkBaseDataAccessor. + */ + @Override + protected void validate() { + super.validate(); + validateZkClientType(_zkClientType, _realmMode); + } + + /** + * This method contains construction logic for ZK-based BaseDataAccessor + * implementations. + * It uses an implementation of GenericBaseDataAccessorBuilder to construct the right + * RealmAwareZkClient based on a host of configs provided in the Builder. + * @return RealmAwareZkClient + */ + @Override + protected RealmAwareZkClient createZkClient(RealmAwareZkClient.RealmMode realmMode, + RealmAwareZkClient.RealmAwareZkConnectionConfig connectionConfig, + RealmAwareZkClient.RealmAwareZkClientConfig clientConfig, String zkAddress) { + RealmAwareZkClient zkClient; + switch (realmMode) { + case MULTI_REALM: + try { + if (_zkClientType == ZkClientType.DEDICATED) { + // Use a realm-aware dedicated zk client + zkClient = DedicatedZkClientFactory.getInstance() + .buildZkClient(connectionConfig, clientConfig); + } else if (_zkClientType == ZkClientType.SHARED) { + // Use a realm-aware shared zk client + zkClient = + SharedZkClientFactory.getInstance().buildZkClient(connectionConfig, clientConfig); + } else { + zkClient = new FederatedZkClient(connectionConfig, clientConfig); + } + } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { + throw new HelixException("Not able to connect on multi-realm mode.", e); + } + break; + + case SINGLE_REALM: + // Create a HelixZkClient: Use a SharedZkClient because ZkBaseDataAccessor does not need to + // do ephemeral operations. + if (_zkClientType == ZkClientType.DEDICATED) { + // If DEDICATED, then we use a dedicated HelixZkClient because we must support ephemeral + // operations + zkClient = DedicatedZkClientFactory.getInstance() + .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress), + clientConfig.createHelixZkClientConfig()); + } else { + zkClient = SharedZkClientFactory.getInstance() + .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress), + clientConfig.createHelixZkClientConfig()); + zkClient + .waitUntilConnected(HelixZkClient.DEFAULT_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS); + } + break; + default: + throw new HelixException("Invalid RealmMode given: " + realmMode); + } + return zkClient; + } + + /** + * Validate ZkClientType based on RealmMode. + * @param zkClientType + * @param realmMode + */ + private void validateZkClientType(ZkClientType zkClientType, + RealmAwareZkClient.RealmMode realmMode) { + boolean isZkClientTypeSet = zkClientType != null; + // If ZkClientType is set, RealmMode must either be single-realm or not set. + if (isZkClientTypeSet && realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM) { + throw new HelixException("ZkClientType cannot be set on multi-realm mode!"); + } + // If ZkClientType is not set and realmMode is single-realm, default to SHARED + if (!isZkClientTypeSet && realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM) { + zkClientType = ZkClientType.SHARED; + } + if (realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM + && zkClientType == ZkClientType.FEDERATED) { + throw new HelixException("FederatedZkClient cannot be set on single-realm mode!"); + } + } +} diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/GenericZkHelixApiBuilder.java b/helix-core/src/main/java/org/apache/helix/manager/zk/GenericZkHelixApiBuilder.java new file mode 100644 index 0000000..f6015c4 --- /dev/null +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/GenericZkHelixApiBuilder.java @@ -0,0 +1,139 @@ +package org.apache.helix.manager.zk; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import java.io.IOException; + +import org.apache.helix.HelixException; +import org.apache.helix.msdcommon.exception.InvalidRoutingDataException; +import org.apache.helix.zookeeper.api.client.HelixZkClient; +import org.apache.helix.zookeeper.api.client.RealmAwareZkClient; +import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer; +import org.apache.helix.zookeeper.impl.client.FederatedZkClient; +import org.apache.helix.zookeeper.impl.factory.SharedZkClientFactory; + + +/** + * GenericZkHelixApiBuilder serves as the abstract parent class for Builders used by Helix Java APIs + * that create ZK connections. By having this class, we reduce duplicate code as much as possible. + * @param <B> + */ +public abstract class GenericZkHelixApiBuilder<B extends GenericZkHelixApiBuilder<B>> { + protected String _zkAddress; + protected RealmAwareZkClient.RealmMode _realmMode; + protected RealmAwareZkClient.RealmAwareZkConnectionConfig _realmAwareZkConnectionConfig; + protected RealmAwareZkClient.RealmAwareZkClientConfig _realmAwareZkClientConfig; + + public B setZkAddress(String zkAddress) { + _zkAddress = zkAddress; + return self(); + } + + public B setRealmMode(RealmAwareZkClient.RealmMode realmMode) { + _realmMode = realmMode; + return self(); + } + + public B setRealmAwareZkConnectionConfig( + RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { + _realmAwareZkConnectionConfig = realmAwareZkConnectionConfig; + return self(); + } + + public B setRealmAwareZkClientConfig( + RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { + _realmAwareZkClientConfig = realmAwareZkClientConfig; + return self(); + } + + /** + * Validates the given Builder parameters using a generic validation logic. + */ + protected void validate() { + // Resolve RealmMode based on whether ZK address has been set + boolean isZkAddressSet = _zkAddress != null && !_zkAddress.isEmpty(); + if (_realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM && !isZkAddressSet) { + throw new HelixException("RealmMode cannot be single-realm without a valid ZkAddress set!"); + } + if (_realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM && isZkAddressSet) { + throw new HelixException("ZkAddress cannot be set on multi-realm mode!"); + } + if (_realmMode == null) { + _realmMode = isZkAddressSet ? RealmAwareZkClient.RealmMode.SINGLE_REALM + : RealmAwareZkClient.RealmMode.MULTI_REALM; + } + + initializeConfigsIfNull(); + } + + /** + * Initializes Realm-aware ZkConnection and ZkClient configs if they haven't been set. + */ + protected void initializeConfigsIfNull() { + // Resolve all default values + if (_realmAwareZkConnectionConfig == null) { + _realmAwareZkConnectionConfig = + new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build(); + } + + // For Helix APIs, ZNRecord should be the default data model + if (_realmAwareZkClientConfig == null) { + _realmAwareZkClientConfig = new RealmAwareZkClient.RealmAwareZkClientConfig() + .setZkSerializer(new ZNRecordSerializer()); + } + } + + /** + * Creates a RealmAwareZkClient based on the parameters set. + * To be used in Helix ZK APIs' constructors: ConfigAccessor, ClusterSetup, ZKHelixAdmin + * @return + */ + protected RealmAwareZkClient createZkClient(RealmAwareZkClient.RealmMode realmMode, + RealmAwareZkClient.RealmAwareZkConnectionConfig connectionConfig, + RealmAwareZkClient.RealmAwareZkClientConfig clientConfig, String zkAddress) { + switch (realmMode) { + case MULTI_REALM: + try { + return new FederatedZkClient(connectionConfig, + clientConfig.setZkSerializer(new ZNRecordSerializer())); + } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { + throw new HelixException("Failed to create FederatedZkClient!", e); + } + case SINGLE_REALM: + // Create a HelixZkClient: Use a SharedZkClient because ClusterSetup does not need to do + // ephemeral operations + return SharedZkClientFactory.getInstance() + .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress), + clientConfig.createHelixZkClientConfig().setZkSerializer(new ZNRecordSerializer())); + default: + throw new HelixException("Invalid RealmMode given: " + realmMode); + } + } + + /** + * Returns an instance of a subclass-Builder in order to reduce duplicate code. + * SuppressWarnings is used to rid of IDE warnings. + * @return an instance of a subclass-Builder. E.g.) ConfigAccessor.Builder + */ + @SuppressWarnings("unchecked") + final B self() { + return (B) this; + } +} diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java index 689d3c9..4f5b67e 100644 --- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java @@ -161,31 +161,10 @@ public class ZKHelixAdmin implements HelixAdmin { _usesExternalZkClient = false; } - private ZKHelixAdmin(Builder builder) { - RealmAwareZkClient zkClient; - switch (builder.realmMode) { - case MULTI_REALM: - try { - zkClient = new FederatedZkClient(builder.realmAwareZkConnectionConfig, - builder.realmAwareZkClientConfig); - } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { - throw new HelixException("Not able to connect on multi-realm mode.", e); - } - break; - case SINGLE_REALM: - // Create a HelixZkClient: Use a SharedZkClient because ZKHelixAdmin does not need to do - // ephemeral operations - zkClient = SharedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(builder.zkAddress), - builder.realmAwareZkClientConfig.createHelixZkClientConfig()); - break; - default: - throw new HelixException("Invalid RealmMode given: " + builder.realmMode); - } - + private ZKHelixAdmin(RealmAwareZkClient zkClient, boolean usesExternalZkClient) { _zkClient = zkClient; _configAccessor = new ConfigAccessor(_zkClient); - _usesExternalZkClient = false; + _usesExternalZkClient = usesExternalZkClient; } @Override @@ -1901,76 +1880,15 @@ public class ZKHelixAdmin implements HelixAdmin { return true; } - // TODO: refactor builder to reduce duplicate code with other Helix Java APIs - public static class Builder { - private String zkAddress; - private RealmAwareZkClient.RealmMode realmMode; - private RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig; - private RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig; - + public static class Builder extends GenericZkHelixApiBuilder<Builder> { public Builder() { } - public ZKHelixAdmin.Builder setZkAddress(String zkAddress) { - this.zkAddress = zkAddress; - return this; - } - - public ZKHelixAdmin.Builder setRealmMode(RealmAwareZkClient.RealmMode realmMode) { - this.realmMode = realmMode; - return this; - } - - public ZKHelixAdmin.Builder setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - realmAwareZkConnectionConfig = realmAwareZkConnectionConfig; - return this; - } - - public ZKHelixAdmin.Builder setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - realmAwareZkClientConfig = realmAwareZkClientConfig; - return this; - } - public ZKHelixAdmin build() { validate(); - return new ZKHelixAdmin(this); - } - - /* - * Validates the given parameters before creating an instance of ZKHelixAdmin. - */ - private void validate() { - // Resolve RealmMode based on other parameters - boolean isZkAddressSet = zkAddress != null && !zkAddress.isEmpty(); - if (realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM && !isZkAddressSet) { - throw new HelixException( - "RealmMode cannot be single-realm without a valid ZkAddress set!"); - } - if (realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM && isZkAddressSet) { - throw new HelixException( - "ZkAddress cannot be set on multi-realm mode!"); - } - if (realmMode == null) { - realmMode = isZkAddressSet ? RealmAwareZkClient.RealmMode.SINGLE_REALM - : RealmAwareZkClient.RealmMode.MULTI_REALM; - } - - // Resolve RealmAwareZkClientConfig - if (realmAwareZkClientConfig == null) { - // ZkHelixAdmin should have ZNRecordSerializer set by default - realmAwareZkClientConfig = new RealmAwareZkClient.RealmAwareZkClientConfig() - .setZkSerializer( - new org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer()); - } - - // Resolve RealmAwareZkConnectionConfig - if (realmAwareZkConnectionConfig == null) { - // If not set, create a default one - realmAwareZkConnectionConfig = - new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build(); - } + return new ZKHelixAdmin( + createZkClient(_realmMode, _realmAwareZkConnectionConfig, _realmAwareZkClientConfig, + _zkAddress), false); } } } diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java index 32f33f8..a7596f4 100644 --- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java @@ -65,7 +65,6 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { // Designates which mode ZkBaseDataAccessor should be created in. If not specified, it will be // created on SHARED mode. - // TODO: move this to RealmAwareZkClient public enum ZkClientType { /** * When ZkBaseDataAccessor is created with the DEDICATED type, it supports ephemeral node @@ -141,51 +140,9 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { _usesExternalZkClient = true; } - private ZkBaseDataAccessor(Builder<T> builder) { - switch (builder.realmMode) { - case MULTI_REALM: - try { - if (builder.zkClientType == ZkClientType.DEDICATED) { - // Use a realm-aware dedicated zk client - _zkClient = DedicatedZkClientFactory.getInstance() - .buildZkClient(builder.realmAwareZkConnectionConfig, - builder.realmAwareZkClientConfig); - } else if (builder.zkClientType == ZkClientType.SHARED) { - // Use a realm-aware shared zk client - _zkClient = SharedZkClientFactory.getInstance() - .buildZkClient(builder.realmAwareZkConnectionConfig, - builder.realmAwareZkClientConfig); - } else { - _zkClient = new FederatedZkClient(builder.realmAwareZkConnectionConfig, - builder.realmAwareZkClientConfig); - } - } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { - throw new HelixException("Not able to connect on multi-realm mode.", e); - } - break; - - case SINGLE_REALM: - // Create a HelixZkClient: Use a SharedZkClient because ZkBaseDataAccessor does not need to - // do ephemeral operations. - if (builder.zkClientType == ZkClientType.DEDICATED) { - // If DEDICATED, then we use a dedicated HelixZkClient because we must support ephemeral - // operations - _zkClient = DedicatedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(builder.zkAddress), - builder.realmAwareZkClientConfig.createHelixZkClientConfig()); - } else { - _zkClient = SharedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(builder.zkAddress), - builder.realmAwareZkClientConfig.createHelixZkClientConfig()); - _zkClient - .waitUntilConnected(HelixZkClient.DEFAULT_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS); - } - break; - default: - throw new HelixException("Invalid RealmMode given: " + builder.realmMode); - } - - _usesExternalZkClient = false; + private ZkBaseDataAccessor(RealmAwareZkClient zkClient, boolean usesExternalZkClient) { + _zkClient = zkClient; + _usesExternalZkClient = usesExternalZkClient; } /** @@ -260,10 +217,9 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { @Deprecated public ZkBaseDataAccessor(String zkAddress, ZkSerializer zkSerializer, ZkClientType zkClientType) { - RealmAwareZkClient.RealmAwareZkClientConfig clientConfig = - new RealmAwareZkClient.RealmAwareZkClientConfig().setZkSerializer(zkSerializer); - - _zkClient = buildRealmAwareZkClient(clientConfig, zkAddress, zkClientType); + _zkClient = buildRealmAwareZkClientWithDefaultConfigs( + new RealmAwareZkClient.RealmAwareZkClientConfig().setZkSerializer(zkSerializer), zkAddress, + zkClientType); _usesExternalZkClient = false; } @@ -282,10 +238,9 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { @Deprecated public ZkBaseDataAccessor(String zkAddress, PathBasedZkSerializer pathBasedZkSerializer, ZkClientType zkClientType) { - RealmAwareZkClient.RealmAwareZkClientConfig clientConfig = - new RealmAwareZkClient.RealmAwareZkClientConfig().setZkSerializer(pathBasedZkSerializer); - - _zkClient = buildRealmAwareZkClient(clientConfig, zkAddress, zkClientType); + _zkClient = buildRealmAwareZkClientWithDefaultConfigs( + new RealmAwareZkClient.RealmAwareZkClientConfig().setZkSerializer(pathBasedZkSerializer), + zkAddress, zkClientType); _usesExternalZkClient = false; } @@ -1308,7 +1263,7 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { } /** - * Subscrie to zookeeper data changes + * Subscribe to zookeeper data changes */ @Override public List<String> subscribeChildChanges(String path, IZkChildListener listener) { @@ -1316,7 +1271,7 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { } /** - * Unsubscrie to zookeeper data changes + * Unsubscribe to zookeeper data changes */ @Override public void unsubscribeChildChanges(String path, IZkChildListener childListener) { @@ -1341,44 +1296,10 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { } } - // TODO: refactor Builder class to remove duplicate code with other Helix Java APIs - public static class Builder<T> { - private String zkAddress; - private ZkBaseDataAccessor.ZkClientType zkClientType; - private RealmAwareZkClient.RealmMode realmMode; - private RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig; - private RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig; - + public static class Builder<T> extends GenericBaseDataAccessorBuilder<Builder<T>> { public Builder() { } - public Builder<T> setZkAddress(String zkAddress) { - this.zkAddress = zkAddress; - return this; - } - - public Builder<T> setRealmMode(RealmAwareZkClient.RealmMode realmMode) { - this.realmMode = realmMode; - return this; - } - - public Builder<T> setZkClientType(ZkClientType zkClientType) { - this.zkClientType = zkClientType; - return this; - } - - public Builder<T> setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - this.realmAwareZkConnectionConfig = realmAwareZkConnectionConfig; - return this; - } - - public Builder<T> setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - this.realmAwareZkClientConfig = realmAwareZkClientConfig; - return this; - } - /** * Returns a <code>ZkBaseDataAccessor</code> instance. * <p> @@ -1388,67 +1309,27 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { */ public ZkBaseDataAccessor<T> build() { validate(); - return new ZkBaseDataAccessor<>(this); - } - - /* - * Validates the given parameters before building an instance of ZkBaseDataAccessor. - */ - private void validate() { - // Resolve RealmMode based on other parameters - boolean isZkAddressSet = zkAddress != null && !zkAddress.isEmpty(); - boolean isZkClientTypeSet = zkClientType != null; - - // If ZkClientType is set, RealmMode must either be single-realm or not set. - if (isZkClientTypeSet && realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM) { - throw new HelixException("ZkClientType cannot be set on multi-realm mode!"); - } - // If ZkClientType is not set and realmMode is single-realm, default to SHARED - if (!isZkClientTypeSet && realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM) { - zkClientType = ZkClientType.SHARED; - } - - if (realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM && !isZkAddressSet) { - throw new HelixException("RealmMode cannot be single-realm without a valid ZkAddress set!"); - } - - if (realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM && isZkAddressSet) { - throw new HelixException("ZkAddress cannot be set on multi-realm mode!"); - } - - if (realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM - && zkClientType == ZkClientType.FEDERATED) { - throw new HelixException("FederatedZkClient cannot be set on single-realm mode!"); - } - - if (realmMode == null) { - realmMode = isZkAddressSet ? RealmAwareZkClient.RealmMode.SINGLE_REALM - : RealmAwareZkClient.RealmMode.MULTI_REALM; - } - - // Resolve RealmAwareZkClientConfig - if (realmAwareZkClientConfig == null) { - realmAwareZkClientConfig = new RealmAwareZkClient.RealmAwareZkClientConfig() - .setZkSerializer(new ZNRecordSerializer()); - } - - // Resolve RealmAwareZkConnectionConfig - if (realmAwareZkConnectionConfig == null) { - // If not set, create a default one - realmAwareZkConnectionConfig = - new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build(); - } + return new ZkBaseDataAccessor<>( + createZkClient(_realmMode, _realmAwareZkConnectionConfig, _realmAwareZkClientConfig, + _zkAddress)); } } - /* - * This is used for constructors that do not take a Builder in as a parameter because of - * keeping backward-compatibility. + /** + * This method is used for constructors that are not based on the Builder for + * backward-compatibility. + * It checks if there is a System Property config set for Multi-ZK mode and determines if a + * FederatedZkClient should be created. + * @param clientConfig default RealmAwareZkClientConfig with ZK serializer set + * @param zkAddress + * @param zkClientType + * @return */ - private RealmAwareZkClient buildRealmAwareZkClient( + static RealmAwareZkClient buildRealmAwareZkClientWithDefaultConfigs( RealmAwareZkClient.RealmAwareZkClientConfig clientConfig, String zkAddress, ZkClientType zkClientType) { if (Boolean.getBoolean(SystemPropertyKeys.MULTI_ZK_ENABLED)) { + // If the multi ZK config is enabled, use multi-realm mode with FederatedZkClient try { return new FederatedZkClient( new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build(), clientConfig); @@ -1458,7 +1339,6 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { } RealmAwareZkClient zkClient; - switch (zkClientType) { case DEDICATED: zkClient = DedicatedZkClientFactory.getInstance() @@ -1475,7 +1355,6 @@ public class ZkBaseDataAccessor<T> implements BaseDataAccessor<T> { .waitUntilConnected(HelixZkClient.DEFAULT_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS); break; } - return zkClient; } } diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ZkCacheBaseDataAccessor.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ZkCacheBaseDataAccessor.java index 72d5601..fdae5dc 100644 --- a/helix-core/src/main/java/org/apache/helix/manager/zk/ZkCacheBaseDataAccessor.java +++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ZkCacheBaseDataAccessor.java @@ -19,31 +19,22 @@ package org.apache.helix.manager.zk; * under the License. */ -import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.TreeMap; -import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import org.apache.helix.AccessOption; import org.apache.helix.HelixException; -import org.apache.helix.SystemPropertyKeys; import org.apache.helix.manager.zk.ZkBaseDataAccessor.RetCode; -import org.apache.helix.msdcommon.exception.InvalidRoutingDataException; import org.apache.helix.store.HelixPropertyListener; import org.apache.helix.store.HelixPropertyStore; import org.apache.helix.store.zk.ZNode; import org.apache.helix.util.PathUtils; -import org.apache.helix.zookeeper.api.client.HelixZkClient; import org.apache.helix.zookeeper.api.client.RealmAwareZkClient; -import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer; -import org.apache.helix.zookeeper.impl.client.FederatedZkClient; -import org.apache.helix.zookeeper.impl.factory.DedicatedZkClientFactory; -import org.apache.helix.zookeeper.impl.factory.SharedZkClientFactory; import org.apache.helix.zookeeper.zkclient.DataUpdater; import org.apache.helix.zookeeper.zkclient.IZkChildListener; import org.apache.helix.zookeeper.zkclient.IZkDataListener; @@ -126,41 +117,9 @@ public class ZkCacheBaseDataAccessor<T> implements HelixPropertyStore<T> { public ZkCacheBaseDataAccessor(String zkAddress, ZkSerializer serializer, String chrootPath, List<String> wtCachePaths, List<String> zkCachePaths, String monitorType, String monitorkey, ZkBaseDataAccessor.ZkClientType zkClientType) { - - // If the multi ZK config is enabled, use multi-realm mode with FederatedZkClient - if (Boolean.parseBoolean(System.getProperty(SystemPropertyKeys.MULTI_ZK_ENABLED))) { - try { - RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder connectionConfigBuilder = - new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder(); - RealmAwareZkClient.RealmAwareZkClientConfig clientConfig = - new RealmAwareZkClient.RealmAwareZkClientConfig(); - clientConfig.setZkSerializer(serializer).setMonitorType(monitorType) - .setMonitorKey(monitorkey); - // Use a federated zk client - _zkClient = new FederatedZkClient(connectionConfigBuilder.build(), clientConfig); - } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { - // Note: IllegalStateException is for HttpRoutingDataReader if MSDS endpoint cannot be - // found - throw new HelixException("Failed to create ZkCacheBaseDataAccessor!", e); - } - } else { - HelixZkClient.ZkClientConfig clientConfig = new HelixZkClient.ZkClientConfig(); - clientConfig.setZkSerializer(serializer).setMonitorType(monitorType) - .setMonitorKey(monitorkey); - switch (zkClientType) { - case DEDICATED: - _zkClient = DedicatedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress), - new HelixZkClient.ZkClientConfig().setZkSerializer(serializer)); - break; - case SHARED: - default: - _zkClient = SharedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress), clientConfig); - } - _zkClient.waitUntilConnected(HelixZkClient.DEFAULT_CONNECTION_TIMEOUT, TimeUnit.MILLISECONDS); - } - + _zkClient = ZkBaseDataAccessor.buildRealmAwareZkClientWithDefaultConfigs( + new RealmAwareZkClient.RealmAwareZkClientConfig().setZkSerializer(serializer) + .setMonitorType(monitorType).setMonitorKey(monitorkey), zkAddress, zkClientType); _baseAccessor = new ZkBaseDataAccessor<>(_zkClient); if (chrootPath == null || chrootPath.equals("/")) { @@ -176,65 +135,15 @@ public class ZkCacheBaseDataAccessor<T> implements HelixPropertyStore<T> { start(); } - /** - * Constructor using a Builder that allows users to set connection and client configs. - * @param builder - */ - private ZkCacheBaseDataAccessor(Builder builder) { - _chrootPath = builder._chrootPath; - _wtCachePaths = builder._wtCachePaths; - _zkCachePaths = builder._zkCachePaths; - - RealmAwareZkClient zkClient; - switch (builder._realmMode) { - case MULTI_REALM: - try { - if (builder._zkClientType == ZkBaseDataAccessor.ZkClientType.DEDICATED) { - // Use a realm-aware dedicated zk client - zkClient = DedicatedZkClientFactory.getInstance() - .buildZkClient(builder._realmAwareZkConnectionConfig, - builder._realmAwareZkClientConfig); - } else if (builder._zkClientType == ZkBaseDataAccessor.ZkClientType.SHARED) { - // Use a realm-aware shared zk client - zkClient = SharedZkClientFactory.getInstance() - .buildZkClient(builder._realmAwareZkConnectionConfig, - builder._realmAwareZkClientConfig); - } else { - // Use a federated zk client - zkClient = new FederatedZkClient(builder._realmAwareZkConnectionConfig, - builder._realmAwareZkClientConfig); - } - break; // Must break out of the switch statement here since zkClient has been created - } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { - // Note: IllegalStateException is for HttpRoutingDataReader if MSDS endpoint cannot be - // found - throw new HelixException("Failed to create ZkCacheBaseDataAccessor!", e); - } - case SINGLE_REALM: - switch (builder._zkClientType) { - case DEDICATED: - // If DEDICATED, then we use a dedicated HelixZkClient because we must support ephemeral - // operations - zkClient = DedicatedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(builder._zkAddress), - builder._realmAwareZkClientConfig.createHelixZkClientConfig()); - break; - case SHARED: - default: - zkClient = SharedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(builder._zkAddress), - builder._realmAwareZkClientConfig.createHelixZkClientConfig()); - zkClient.waitUntilConnected(HelixZkClient.DEFAULT_CONNECTION_TIMEOUT, - TimeUnit.MILLISECONDS); - break; - } - default: - throw new HelixException("Invalid RealmMode given: " + builder._realmMode); - } - + private ZkCacheBaseDataAccessor(RealmAwareZkClient zkClient, String chrootPath, + List<String> wtCachePaths, List<String> zkCachePaths) { _zkClient = zkClient; _baseAccessor = new ZkBaseDataAccessor<>(_zkClient); + _chrootPath = chrootPath; + _wtCachePaths = wtCachePaths; + _zkCachePaths = zkCachePaths; + start(); } @@ -920,43 +829,15 @@ public class ZkCacheBaseDataAccessor<T> implements HelixPropertyStore<T> { } } - public static class Builder<T> { - private String _zkAddress; - private RealmAwareZkClient.RealmMode _realmMode; - private RealmAwareZkClient.RealmAwareZkConnectionConfig _realmAwareZkConnectionConfig; - private RealmAwareZkClient.RealmAwareZkClientConfig _realmAwareZkClientConfig; - + public static class Builder<T> extends GenericBaseDataAccessorBuilder<Builder<T>> { /** ZkCacheBaseDataAccessor-specific parameters */ private String _chrootPath; private List<String> _wtCachePaths; private List<String> _zkCachePaths; - private ZkBaseDataAccessor.ZkClientType _zkClientType; public Builder() { } - public Builder<T> setZkAddress(String zkAddress) { - _zkAddress = zkAddress; - return this; - } - - public Builder<T> setRealmMode(RealmAwareZkClient.RealmMode realmMode) { - _realmMode = realmMode; - return this; - } - - public Builder<T> setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - _realmAwareZkConnectionConfig = realmAwareZkConnectionConfig; - return this; - } - - public Builder<T> setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - _realmAwareZkClientConfig = realmAwareZkClientConfig; - return this; - } - public Builder<T> setChrootPath(String chrootPath) { _chrootPath = chrootPath; return this; @@ -972,70 +853,11 @@ public class ZkCacheBaseDataAccessor<T> implements HelixPropertyStore<T> { return this; } - /** - * Sets the ZkClientType. If this is set, ZkCacheBaseDataAccessor will be created on - * single-realm mode. - * @param zkClientType - * @return - */ - public Builder<T> setZkClientType(ZkBaseDataAccessor.ZkClientType zkClientType) { - _zkClientType = zkClientType; - return this; - } - public ZkCacheBaseDataAccessor<T> build() { validate(); - return new ZkCacheBaseDataAccessor<>(this); - } - - private void validate() { - // Resolve RealmMode based on other parameters - boolean isZkAddressSet = _zkAddress != null && !_zkAddress.isEmpty(); - boolean isZkClientTypeSet = _zkClientType != null; - - // If ZkClientType is set, RealmMode must either be single-realm or not set. - if (isZkClientTypeSet && _realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM) { - throw new HelixException( - "ZkCacheBaseDataAccessor: you cannot set ZkClientType on multi-realm mode!"); - } - // If ZkClientType is not set and realmMode is single-realm, default to SHARED - if (!isZkClientTypeSet && _realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM) { - _zkClientType = ZkBaseDataAccessor.ZkClientType.SHARED; - } - - if (_realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM && !isZkAddressSet) { - throw new HelixException( - "ZkCacheBaseDataAccessor: RealmMode cannot be single-realm without a valid ZkAddress set!"); - } - - if (_realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM && isZkAddressSet) { - throw new HelixException( - "ZkCacheBaseDataAccessor: You cannot set the ZkAddress on multi-realm mode!"); - } - - if (_realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM - && _zkClientType == ZkBaseDataAccessor.ZkClientType.FEDERATED) { - throw new HelixException( - "ZkCacheBaseDataAccessor: You cannot use FederatedZkClient on single-realm mode!"); - } - - if (_realmMode == null) { - _realmMode = isZkAddressSet ? RealmAwareZkClient.RealmMode.SINGLE_REALM - : RealmAwareZkClient.RealmMode.MULTI_REALM; - } - - // Resolve RealmAwareZkClientConfig - if (_realmAwareZkClientConfig == null) { - _realmAwareZkClientConfig = new RealmAwareZkClient.RealmAwareZkClientConfig() - .setZkSerializer(new ZNRecordSerializer()); - } - - // Resolve RealmAwareZkConnectionConfig - if (_realmAwareZkConnectionConfig == null) { - // If not set, create a default one - _realmAwareZkConnectionConfig = - new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build(); - } + return new ZkCacheBaseDataAccessor<>( + createZkClient(_realmMode, _realmAwareZkConnectionConfig, _realmAwareZkClientConfig, + _zkAddress), _chrootPath, _wtCachePaths, _zkCachePaths); } } } diff --git a/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java b/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java index 453a2f1..166a7a8 100644 --- a/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java +++ b/helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java @@ -46,6 +46,7 @@ import org.apache.helix.cloud.azure.AzureConstants; import org.apache.helix.cloud.constants.CloudProvider; import org.apache.helix.PropertyKey; import org.apache.helix.SystemPropertyKeys; +import org.apache.helix.manager.zk.GenericZkHelixApiBuilder; import org.apache.helix.manager.zk.ZKHelixAdmin; import org.apache.helix.manager.zk.ZKHelixDataAccessor; import org.apache.helix.manager.zk.ZkBaseDataAccessor; @@ -188,29 +189,10 @@ public class ClusterSetup { _usesExternalZkClient = true; } - private ClusterSetup(Builder builder) { - switch (builder._realmMode) { - case MULTI_REALM: - try { - _zkClient = new FederatedZkClient(builder._realmAwareZkConnectionConfig, - builder._realmAwareZkClientConfig.setZkSerializer(new ZNRecordSerializer())); - break; - } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { - throw new HelixException("Failed to create ClusterSetup!", e); - } - case SINGLE_REALM: - // Create a HelixZkClient: Use a SharedZkClient because ClusterSetup does not need to do - // ephemeral operations - _zkClient = SharedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(builder._zkAddress), - builder._realmAwareZkClientConfig.createHelixZkClientConfig() - .setZkSerializer(new ZNRecordSerializer())); - break; - default: - throw new HelixException("Invalid RealmMode given: " + builder._realmMode); - } + private ClusterSetup(RealmAwareZkClient zkClient, boolean usesExternalZkClient) { + _zkClient = zkClient; _admin = new ZKHelixAdmin(_zkClient); - _usesExternalZkClient = false; + _usesExternalZkClient = usesExternalZkClient; } /** @@ -1636,69 +1618,15 @@ public class ClusterSetup { System.exit(ret); } - public static class Builder { - private String _zkAddress; - private RealmAwareZkClient.RealmMode _realmMode; - private RealmAwareZkClient.RealmAwareZkConnectionConfig _realmAwareZkConnectionConfig; - private RealmAwareZkClient.RealmAwareZkClientConfig _realmAwareZkClientConfig; - + public static class Builder extends GenericZkHelixApiBuilder<Builder> { public Builder() { } - public Builder setZkAddress(String zkAddress) { - _zkAddress = zkAddress; - return this; - } - - public Builder setRealmMode(RealmAwareZkClient.RealmMode realmMode) { - _realmMode = realmMode; - return this; - } - - public Builder setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - _realmAwareZkConnectionConfig = realmAwareZkConnectionConfig; - return this; - } - - public Builder setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - _realmAwareZkClientConfig = realmAwareZkClientConfig; - return this; - } - public ClusterSetup build() { validate(); - return new ClusterSetup(this); - } - - private void validate() { - // Resolve RealmMode based on other parameters - boolean isZkAddressSet = _zkAddress != null && !_zkAddress.isEmpty(); - if (_realmMode == RealmAwareZkClient.RealmMode.SINGLE_REALM && !isZkAddressSet) { - throw new HelixException( - "ClusterSetup: RealmMode cannot be single-realm without a valid ZkAddress set!"); - } - if (_realmMode == RealmAwareZkClient.RealmMode.MULTI_REALM && isZkAddressSet) { - throw new HelixException( - "ClusterSetup: You cannot set the ZkAddress on multi-realm mode!"); - } - if (_realmMode == null) { - _realmMode = isZkAddressSet ? RealmAwareZkClient.RealmMode.SINGLE_REALM - : RealmAwareZkClient.RealmMode.MULTI_REALM; - } - - // Resolve RealmAwareZkClientConfig - if (_realmAwareZkClientConfig == null) { - _realmAwareZkClientConfig = new RealmAwareZkClient.RealmAwareZkClientConfig(); - } - - // Resolve RealmAwareZkConnectionConfig - if (_realmAwareZkConnectionConfig == null) { - // If not set, create a default one - _realmAwareZkConnectionConfig = - new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build(); - } + return new ClusterSetup( + createZkClient(_realmMode, _realmAwareZkConnectionConfig, _realmAwareZkClientConfig, + _zkAddress), false); } } } diff --git a/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/BestPossibleExternalViewVerifier.java b/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/BestPossibleExternalViewVerifier.java index 6d601c4..12cdad4 100644 --- a/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/BestPossibleExternalViewVerifier.java +++ b/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/BestPossibleExternalViewVerifier.java @@ -111,19 +111,22 @@ public class BestPossibleExternalViewVerifier extends ZkHelixClusterVerifier { _dataProvider = new ResourceControllerDataProvider(); } - private BestPossibleExternalViewVerifier(Builder builder) { - super(builder); + private BestPossibleExternalViewVerifier(RealmAwareZkClient zkClient, String clusterName, + Map<String, Map<String, String>> errStates, Set<String> resources, + Set<String> expectLiveInstances) { + super(zkClient, clusterName); // Deep copy data from Builder _errStates = new HashMap<>(); - if (builder._errStates != null) { - builder._errStates.forEach((k, v) -> _errStates.put(k, new HashMap<>(v))); + if (errStates != null) { + errStates.forEach((k, v) -> _errStates.put(k, new HashMap<>(v))); } - _resources = new HashSet<>(builder._resources); - _expectLiveInstances = new HashSet<>(builder._expectLiveInstances); + _resources = resources == null ? new HashSet<>() : new HashSet<>(resources); + _expectLiveInstances = + expectLiveInstances == null ? new HashSet<>() : new HashSet<>(expectLiveInstances); _dataProvider = new ResourceControllerDataProvider(); } - public static class Builder extends ZkHelixClusterVerifier.Builder { + public static class Builder extends ZkHelixClusterVerifier.Builder<Builder> { private final String _clusterName; private Map<String, Map<String, String>> _errStates; private Set<String> _resources; @@ -151,7 +154,10 @@ public class BestPossibleExternalViewVerifier extends ZkHelixClusterVerifier { } validate(); - return new BestPossibleExternalViewVerifier(this); + return new BestPossibleExternalViewVerifier( + createZkClient(RealmAwareZkClient.RealmMode.SINGLE_REALM, _realmAwareZkConnectionConfig, + _realmAwareZkClientConfig, _zkAddress), _clusterName, _errStates, _resources, + _expectLiveInstances); } public String getClusterName() { @@ -193,23 +199,6 @@ public class BestPossibleExternalViewVerifier extends ZkHelixClusterVerifier { _zkClient = zkClient; return this; } - - @Override - public Builder setZkAddr(String zkAddress) { - return (Builder) super.setZkAddr(zkAddress); - } - - @Override - public Builder setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - return (Builder) super.setRealmAwareZkConnectionConfig(realmAwareZkConnectionConfig); - } - - @Override - public Builder setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - return (Builder) super.setRealmAwareZkClientConfig(realmAwareZkClientConfig); - } } @Override diff --git a/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ClusterLiveNodesVerifier.java b/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ClusterLiveNodesVerifier.java index 8aeb64a..4a46f18 100644 --- a/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ClusterLiveNodesVerifier.java +++ b/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ClusterLiveNodesVerifier.java @@ -38,9 +38,10 @@ public class ClusterLiveNodesVerifier extends ZkHelixClusterVerifier { _expectLiveNodes = new HashSet<>(expectLiveNodes); } - private ClusterLiveNodesVerifier(Builder builder) { - super(builder); - _expectLiveNodes = new HashSet<>(builder._expectLiveNodes); + private ClusterLiveNodesVerifier(RealmAwareZkClient zkClient, String clusterName, + Set<String> expectLiveNodes) { + super(zkClient, clusterName); + _expectLiveNodes = expectLiveNodes == null ? new HashSet<>() : new HashSet<>(expectLiveNodes); } @Override @@ -58,7 +59,7 @@ public class ClusterLiveNodesVerifier extends ZkHelixClusterVerifier { return _expectLiveNodes.equals(actualLiveNodes); } - public static class Builder extends ZkHelixClusterVerifier.Builder { + public static class Builder extends ZkHelixClusterVerifier.Builder<Builder> { private final String _clusterName; // This is the ZK path sharding key private final Set<String> _expectLiveNodes; @@ -73,24 +74,9 @@ public class ClusterLiveNodesVerifier extends ZkHelixClusterVerifier { } validate(); - return new ClusterLiveNodesVerifier(this); - } - - @Override - public Builder setZkAddr(String zkAddress) { - return (Builder) super.setZkAddr(zkAddress); - } - - @Override - public Builder setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - return (Builder) super.setRealmAwareZkConnectionConfig(realmAwareZkConnectionConfig); - } - - @Override - public Builder setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - return (Builder) super.setRealmAwareZkClientConfig(realmAwareZkClientConfig); + return new ClusterLiveNodesVerifier( + createZkClient(RealmAwareZkClient.RealmMode.SINGLE_REALM, _realmAwareZkConnectionConfig, + _realmAwareZkClientConfig, _zkAddress), _clusterName, _expectLiveNodes); } } } diff --git a/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/StrictMatchExternalViewVerifier.java b/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/StrictMatchExternalViewVerifier.java index 76f0d09..987599e 100644 --- a/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/StrictMatchExternalViewVerifier.java +++ b/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/StrictMatchExternalViewVerifier.java @@ -78,23 +78,16 @@ public class StrictMatchExternalViewVerifier extends ZkHelixClusterVerifier { _isDeactivatedNodeAware = isDeactivatedNodeAware; } - @Deprecated private StrictMatchExternalViewVerifier(RealmAwareZkClient zkClient, String clusterName, Set<String> resources, Set<String> expectLiveInstances, boolean isDeactivatedNodeAware) { super(zkClient, clusterName); - _resources = resources; - _expectLiveInstances = expectLiveInstances; + _resources = resources == null ? new HashSet<>() : new HashSet<>(resources); + _expectLiveInstances = + expectLiveInstances == null ? new HashSet<>() : new HashSet<>(expectLiveInstances); _isDeactivatedNodeAware = isDeactivatedNodeAware; } - private StrictMatchExternalViewVerifier(Builder builder) { - super(builder); - _resources = new HashSet<>(builder._resources); - _expectLiveInstances = new HashSet<>(builder._expectLiveInstances); - _isDeactivatedNodeAware = builder._isDeactivatedNodeAware; - } - - public static class Builder extends ZkHelixClusterVerifier.Builder { + public static class Builder extends ZkHelixClusterVerifier.Builder<Builder> { private final String _clusterName; // This is the ZK path sharding key private Set<String> _resources; private Set<String> _expectLiveInstances; @@ -119,7 +112,10 @@ public class StrictMatchExternalViewVerifier extends ZkHelixClusterVerifier { } validate(); - return new StrictMatchExternalViewVerifier(this); + return new StrictMatchExternalViewVerifier( + createZkClient(RealmAwareZkClient.RealmMode.SINGLE_REALM, _realmAwareZkConnectionConfig, + _realmAwareZkClientConfig, _zkAddress), _clusterName, _resources, + _expectLiveInstances, _isDeactivatedNodeAware); } public Builder(String clusterName) { @@ -167,23 +163,6 @@ public class StrictMatchExternalViewVerifier extends ZkHelixClusterVerifier { return this; } - @Override - public Builder setZkAddr(String zkAddress) { - return (Builder) super.setZkAddr(zkAddress); - } - - @Override - public Builder setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - return (Builder) super.setRealmAwareZkConnectionConfig(realmAwareZkConnectionConfig); - } - - @Override - public Builder setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - return (Builder) super.setRealmAwareZkClientConfig(realmAwareZkClientConfig); - } - protected void validate() { super.validate(); if (!_clusterName.equals(_realmAwareZkConnectionConfig.getZkRealmShardingKey())) { diff --git a/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ZkHelixClusterVerifier.java b/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ZkHelixClusterVerifier.java index 449b659..e2022e7 100644 --- a/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ZkHelixClusterVerifier.java +++ b/helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/ZkHelixClusterVerifier.java @@ -31,6 +31,7 @@ import org.apache.helix.HelixException; import org.apache.helix.PropertyKey; import org.apache.helix.SystemPropertyKeys; import org.apache.helix.api.listeners.PreFetch; +import org.apache.helix.manager.zk.GenericZkHelixApiBuilder; import org.apache.helix.manager.zk.ZKHelixDataAccessor; import org.apache.helix.manager.zk.ZkBaseDataAccessor; import org.apache.helix.msdcommon.exception.InvalidRoutingDataException; @@ -93,13 +94,7 @@ public abstract class ZkHelixClusterVerifier } } - /** - * Deprecated because we discourage passing in a ZkClient directly. - * @param zkClient - * @param clusterName - */ - @Deprecated - public ZkHelixClusterVerifier(RealmAwareZkClient zkClient, String clusterName) { + protected ZkHelixClusterVerifier(RealmAwareZkClient zkClient, String clusterName) { if (zkClient == null || clusterName == null) { throw new IllegalArgumentException("requires zkClient|clusterName"); } @@ -144,27 +139,6 @@ public abstract class ZkHelixClusterVerifier _keyBuilder = _accessor.keyBuilder(); } - protected ZkHelixClusterVerifier(Builder builder) { - if (Boolean.parseBoolean(System.getProperty(SystemPropertyKeys.MULTI_ZK_ENABLED))) { - try { - // First, try to create a RealmAwareZkClient that's a DedicatedZkClient - _zkClient = DedicatedZkClientFactory.getInstance() - .buildZkClient(builder._realmAwareZkConnectionConfig, - builder._realmAwareZkClientConfig); - } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { - throw new HelixException("ZkHelixClusterVerifier: failed to create ZkClient!", e); - } - } else { - _zkClient = DedicatedZkClientFactory.getInstance() - .buildZkClient(new HelixZkClient.ZkConnectionConfig(builder._zkAddress)); - } - _usesExternalZkClient = false; - _zkClient.setZkSerializer(new ZNRecordSerializer()); - _clusterName = builder._realmAwareZkConnectionConfig.getZkRealmShardingKey(); - _accessor = new ZKHelixDataAccessor(_clusterName, new ZkBaseDataAccessor<>(_zkClient)); - _keyBuilder = _accessor.keyBuilder(); - } - /** * Verify the cluster. * The method will be blocked at most {@code timeout}. @@ -345,36 +319,36 @@ public abstract class ZkHelixClusterVerifier return _clusterName; } - // TODO: refactor Builders for Java APIs - protected abstract static class Builder { - // Note: ZkHelixClusterVerifier is a single-realm API, so RealmMode is assumed to be - // SINGLE-REALM - protected String _zkAddress; - protected RealmAwareZkClient.RealmAwareZkConnectionConfig _realmAwareZkConnectionConfig; - protected RealmAwareZkClient.RealmAwareZkClientConfig _realmAwareZkClientConfig; - + protected abstract static class Builder<B extends Builder<B>> extends GenericZkHelixApiBuilder<B> { public Builder() { + // Note: ZkHelixClusterVerifier is a single-realm API, so RealmMode is assumed to be + // SINGLE-REALM + setRealmMode(RealmAwareZkClient.RealmMode.SINGLE_REALM); } - public Builder setZkAddr(String zkAddress) { - _zkAddress = zkAddress; - return this; - } - - public Builder setRealmAwareZkConnectionConfig( - RealmAwareZkClient.RealmAwareZkConnectionConfig realmAwareZkConnectionConfig) { - _realmAwareZkConnectionConfig = realmAwareZkConnectionConfig; - return this; + /** + * Use setZkAddress() instead. Deprecated but left here for backward-compatibility. + * @param zkAddress + * @return + */ + @Deprecated + public B setZkAddr(String zkAddress) { + return setZkAddress(zkAddress); } - public Builder setRealmAwareZkClientConfig( - RealmAwareZkClient.RealmAwareZkClientConfig realmAwareZkClientConfig) { - _realmAwareZkClientConfig = realmAwareZkClientConfig; - return this; + public String getClusterName() { + if (_realmAwareZkConnectionConfig != null && ( + _realmAwareZkConnectionConfig.getZkRealmShardingKey() != null + && !_realmAwareZkConnectionConfig.getZkRealmShardingKey().isEmpty())) { + // Need to remove the first "/" from sharding key given + return _realmAwareZkConnectionConfig.getZkRealmShardingKey().substring(1); + } + throw new HelixException( + "Failed to get the cluster name! Either RealmAwareZkConnectionConfig is null or its sharding key is null or empty!"); } protected void validate() { - // Validate that either ZkAddress or ZkRealmShardingKey is set. + // Validate that either ZkAddress or ZkRealmShardingKey is set if (_zkAddress == null || _zkAddress.isEmpty()) { if (_realmAwareZkConnectionConfig == null || _realmAwareZkConnectionConfig.getZkRealmShardingKey() == null @@ -384,14 +358,29 @@ public abstract class ZkHelixClusterVerifier + _zkAddress + " RealmAwareZkConnectionConfig: " + _realmAwareZkConnectionConfig); } } + initializeConfigsIfNull(); + } - // Resolve all default values - if (_realmAwareZkConnectionConfig == null) { - _realmAwareZkConnectionConfig = - new RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder().build(); - } - if (_realmAwareZkClientConfig == null) { - _realmAwareZkClientConfig = new RealmAwareZkClient.RealmAwareZkClientConfig(); + /** + * Creates a RealmAwareZkClient for ZkHelixClusterVerifiers. + * Note that DedicatedZkClient is used whether it's multi-realm or single-realm. + * @return + */ + @Override + protected RealmAwareZkClient createZkClient(RealmAwareZkClient.RealmMode realmMode, + RealmAwareZkClient.RealmAwareZkConnectionConfig connectionConfig, + RealmAwareZkClient.RealmAwareZkClientConfig clientConfig, String zkAddress) { + if (Boolean.parseBoolean(System.getProperty(SystemPropertyKeys.MULTI_ZK_ENABLED))) { + try { + // First, try to create a RealmAwareZkClient that's a DedicatedZkClient + return DedicatedZkClientFactory.getInstance() + .buildZkClient(connectionConfig, clientConfig); + } catch (IOException | InvalidRoutingDataException | IllegalStateException e) { + throw new HelixException("ZkHelixClusterVerifier: failed to create ZkClient!", e); + } + } else { + return DedicatedZkClientFactory.getInstance() + .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress)); } } } diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/RealmAwareZkClient.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/RealmAwareZkClient.java index 40b6c54..0e461b7 100644 --- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/RealmAwareZkClient.java +++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/RealmAwareZkClient.java @@ -419,6 +419,12 @@ public interface RealmAwareZkClient { return this; } + /** + * Note: this must be a valid ZK path. For example, if you are trying to create a sharding key + * out of a cluster name "CLUSTER", then your sharding key should be "/CLUSTER". + * @param shardingKey + * @return + */ public Builder setZkRealmShardingKey(String shardingKey) { _zkRealmShardingKey = shardingKey; return this; diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/ZkClientType.java b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/ZkClientType.java new file mode 100644 index 0000000..4db06a8 --- /dev/null +++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/ZkClientType.java @@ -0,0 +1,42 @@ +package org.apache.helix.zookeeper.api.client; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +public enum ZkClientType { + /** + * If a Helix API is created with the DEDICATED type, it supports ephemeral node + * creation, callback functionality, and session management. But note that this is more + * resource-heavy since it creates a dedicated ZK connection so should be used sparingly only + * when the aforementioned features are needed. + */ + DEDICATED, + + /** + * If a Helix API is created with the SHARED type, it only supports CRUD + * functionalities. This will be the default mode of creation. + */ + SHARED, + + /** + * Uses FederatedZkClient (applicable on multi-realm mode only) that queries Metadata Store + * Directory Service for routing data. + */ + FEDERATED +}
