You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by "junkaixue (via GitHub)" <gi...@apache.org> on 2023/01/29 22:09:45 UTC
[GitHub] [helix] junkaixue commented on a diff in pull request #2351: Add code and code translator
junkaixue commented on code in PR #2351:
URL: https://github.com/apache/helix/pull/2351#discussion_r1090059483
##########
meta-client/src/main/java/org/apache/helix/metaclient/impl/zk/util/ZkMetaClientUtil.java:
##########
@@ -210,4 +222,87 @@ public static MetaClientException translateZkExceptionToMetaclientException(ZkEx
return new MetaClientException(e);
}
}
+
+ /**
+ * This function translate and group Zk exception code to metaclient code.
+ * It currently includes all ZK code on 3.6.3.
+ */
+ public static MetaClientException.Code translateKeeperCodeToMetaClientCode(
Review Comment:
NIT: that's too much as keepercode. Most of zookeeper user even dont know what is keepercode. Better we call it fromZookeeperCodeToMetaClientCode()
##########
meta-client/src/main/java/org/apache/helix/metaclient/exception/MetaClientException.java:
##########
@@ -35,4 +35,114 @@ public MetaClientException(String message) {
public MetaClientException(Throwable cause) {
super(cause);
}
+
+ public enum Code {
+ /** Everything is OK. */
+ OK,
+
+ /** Indicates a system and server-side errors not defined by following codes.
+ * It also indicate a range. Any value larger than this and less than DBUSERERROR
+ * indicating error from server side.*/
+ DBSYSTEMERROR,
+
+ /** Connection to the server has been lost. */
+ CONNECTIONLOSS,
+
+ /** Operation is unimplemented. */
+ UNIMPLEMENTED,
+
+ /** Operation timeout. */
+ OPERATIONTIMEOUT,
+
+ /** Either a runetime or data inconsistency was found. */
+ CONSISTENCYERROR,
Review Comment:
Same here. Better to have one word and use underscore connecting them. This is Helix tradition. I know ZK may have a different style. But let's follow how Helix defines.
##########
meta-client/src/main/java/org/apache/helix/metaclient/impl/zk/util/ZkMetaClientUtil.java:
##########
@@ -74,22 +78,24 @@ private static Map<org.apache.helix.metaclient.api.Op.Type, Function<org.apache.
opmap.put(org.apache.helix.metaclient.api.Op.Type.CREATE, op -> {
try {
- CreateMode mode = convertMetaClientMode(((org.apache.helix.metaclient.api.Op.Create) op).getEntryMode());
- return Op.create(op.getPath(), ((org.apache.helix.metaclient.api.Op.Create) op).getData(), DEFAULT_ACL, mode);
+ CreateMode mode = convertMetaClientMode(
+ ((org.apache.helix.metaclient.api.Op.Create) op).getEntryMode());
+ return Op.create(op.getPath(), ((org.apache.helix.metaclient.api.Op.Create) op).getData(),
+ DEFAULT_ACL, mode);
} catch (KeeperException e) {
throw translateZkExceptionToMetaclientException(ZkException.create(e));
}
});
- opmap.put(org.apache.helix.metaclient.api.Op.Type.DELETE,
- op -> Op.delete(op.getPath(), ((org.apache.helix.metaclient.api.Op.Delete) op).getVersion()));
+ opmap.put(org.apache.helix.metaclient.api.Op.Type.DELETE, op -> Op
+ .delete(op.getPath(), ((org.apache.helix.metaclient.api.Op.Delete) op).getVersion()));
Review Comment:
One suggestion for you. If this is just style change or reformat, better do it with refactor PR. Otherwise, we need to take a very deep understand what's changed but actually just format change.
##########
meta-client/src/main/java/org/apache/helix/metaclient/impl/zk/util/ZkMetaClientUtil.java:
##########
@@ -210,4 +222,87 @@ public static MetaClientException translateZkExceptionToMetaclientException(ZkEx
return new MetaClientException(e);
}
}
+
+ /**
+ * This function translate and group Zk exception code to metaclient code.
+ * It currently includes all ZK code on 3.6.3.
+ */
+ public static MetaClientException.Code translateKeeperCodeToMetaClientCode(
+ KeeperException.Code zkCode) {
+ // TODO: add log to track ZK origional code.
+ switch (zkCode) {
+ case AUTHFAILED:
+ case SESSIONCLOSEDREQUIRESASLAUTH:
+ case INVALIDACL:
+ return MetaClientException.Code.AUTHFAILED;
+
+ case CONNECTIONLOSS:
+ return MetaClientException.Code.CONNECTIONLOSS;
+
+ case BADARGUMENTS:
+ return MetaClientException.Code.INVALIDARGUMENTS;
+
+ case BADVERSION:
+ return MetaClientException.Code.BADVERSION;
+
+ case NOAUTH:
+ return MetaClientException.Code.NOAUTH;
+
+ case NOWATCHER:
+ return MetaClientException.Code.INVALIDLISTENER;
+
+ case NOTEMPTY:
+ return MetaClientException.Code.NOTLEAFENTRY;
+
+ case NODEEXISTS:
+ return MetaClientException.Code.ENTRYEXISTS;
+
+ case SESSIONEXPIRED:
+ case SESSIONMOVED:
+ case UNKNOWNSESSION:
+ return MetaClientException.Code.SESSIONERROR;
+
+ case NONODE:
+ return MetaClientException.Code.NOSUCHENTRY;
+
+ case OPERATIONTIMEOUT:
+ return MetaClientException.Code.OPERATIONTIMEOUT;
+
+ case OK:
+ return MetaClientException.Code.OK;
+
+ case UNIMPLEMENTED:
+ return MetaClientException.Code.UNIMPLEMENTED;
+
+ case RUNTIMEINCONSISTENCY:
+ case DATAINCONSISTENCY:
+ return MetaClientException.Code.CONSISTENCYERROR;
+
+ case SYSTEMERROR:
+ case MARSHALLINGERROR:
+ case NEWCONFIGNOQUORUM:
+ case RECONFIGINPROGRESS:
+ return MetaClientException.Code.DBSYSTEMERROR;
+
+ case NOCHILDRENFOREPHEMERALS:
+ case INVALIDCALLBACK:
+ case NOTREADONLY:
+ case EPHEMERALONLOCALSESSION:
+ case RECONFIGDISABLED:
+ return MetaClientException.Code.DBUSERERROR;
+
+ /*
+ * APIERROR is ZooKeeper Code value separator. It is never thrown by ZK server,
+ * ZK error codes greater than its value are user or client errors and values less than
+ * this indicate a ZK server.
+ */
+ default:
+ if (zkCode.intValue() < KeeperException.Code.APIERROR.intValue()
+ & zkCode.intValue() >= KeeperException.Code.SYSTEMERROR.intValue()) {
+ return MetaClientException.Code.DBSYSTEMERROR;
+ } else {
+ return MetaClientException.Code.DBUSERERROR;
Review Comment:
A better practice when you have if with some return. You can remove "else clause":
if (zkCode.intValue() < KeeperException.Code.APIERROR.intValue()
& zkCode.intValue() >= KeeperException.Code.SYSTEMERROR.intValue()) {
return MetaClientException.Code.DBSYSTEMERROR;
}
return MetaClientException.Code.DBUSERERROR;
--
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: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org