qqu0127 commented on code in PR #2239:
URL: https://github.com/apache/helix/pull/2239#discussion_r1024620760
##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java:
##########
@@ -477,10 +477,31 @@ public long getCreationTime(String path) {
return getZkClient(path).getCreationTime(path);
}
+ /**
+ * Executes ZkMulti on operations that are connected to the same Zk server.
+ * Will throw exception if any operation's server connection is different.
+ * @param ops
+ * @return
+ * @throws IllegalArgumentException
+ */
@Override
public List<OpResult> multi(Iterable<Op> ops) {
- throwUnsupportedOperationException();
- return null;
+ if (ops == null) {
+ throw new NullPointerException("ops must not be null.");
+ }
+ String opPath = null;
+ String opPathRealm = null;
+ for (Op op : ops) {
+ if (opPath == null) {
+ opPath = op.getPath();
+ opPathRealm = getZkRealm(op.getPath());
+ } else {
+ if (!opPathRealm.equals(getZkRealm(op.getPath()))){
+ throw new IllegalArgumentException();
Review Comment:
Some exception message will be great, to indicate not all operations are
using the same realm.
##########
helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiInMultiZk.java:
##########
@@ -0,0 +1,74 @@
+package org.apache.helix.integration.multizk;
+import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
+import org.apache.helix.zookeeper.constant.RoutingDataReaderType;
+import org.apache.helix.zookeeper.datamodel.serializer.ZNRecordSerializer;
+import org.apache.helix.zookeeper.impl.client.FederatedZkClient;
+import org.apache.helix.zookeeper.routing.RoutingDataManager;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.Op;
+import org.apache.zookeeper.ZooDefs;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+import java.util.*;
+
+/**
+ * This class test multi implementation in FederatedZkClient. Extends
MultiZkTestBase as the test require a multi zk
+ * server setup.
+ */
+public class TestMultiInMultiZk extends MultiZkTestBase {
+
+ @BeforeClass
+ public void beforeClass() throws Exception {
+ super.beforeClass();
+ // Routing data may be set by other tests using the same endpoint;
reset() for good measure
+ RoutingDataManager.getInstance().reset();
+ // Create a FederatedZkClient for admin work
+
+ try {
+ _zkClient =
+ new FederatedZkClient(new
RealmAwareZkClient.RealmAwareZkConnectionConfig.Builder()
+ .setRoutingDataSourceEndpoint(_msdsEndpoint + ","
+ ZK_PREFIX + ZK_START_PORT)
+
.setRoutingDataSourceType(RoutingDataReaderType.HTTP_ZK_FALLBACK.name()).build(),
+ new RealmAwareZkClient.RealmAwareZkClientConfig());
+ _zkClient.setZkSerializer(new ZNRecordSerializer());
+ } catch (Exception ex) {
+ for (StackTraceElement elm : ex.getStackTrace()) {
+ System.out.println(elm);
+ }
+ }
+ System.out.println("end start");
+ }
+
+ @AfterClass
+ public void afterClass() throws Exception {
+ super.afterClass();
+ }
Review Comment:
is this necessary?
##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/impl/client/FederatedZkClient.java:
##########
@@ -477,10 +477,31 @@ public long getCreationTime(String path) {
return getZkClient(path).getCreationTime(path);
}
+ /**
+ * Executes ZkMulti on operations that are connected to the same Zk server.
+ * Will throw exception if any operation's server connection is different.
+ * @param ops
+ * @return
+ * @throws IllegalArgumentException
+ */
@Override
public List<OpResult> multi(Iterable<Op> ops) {
- throwUnsupportedOperationException();
- return null;
+ if (ops == null) {
+ throw new NullPointerException("ops must not be null.");
+ }
+ String opPath = null;
+ String opPathRealm = null;
+ for (Op op : ops) {
+ if (opPath == null) {
+ opPath = op.getPath();
+ opPathRealm = getZkRealm(op.getPath());
+ } else {
+ if (!opPathRealm.equals(getZkRealm(op.getPath()))){
Review Comment:
else if can be combined
##########
helix-core/src/test/java/org/apache/helix/integration/multizk/MultiZkTestBase.java:
##########
@@ -0,0 +1,150 @@
+package org.apache.helix.integration.multizk;
Review Comment:
please add apache license, thanks
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
