junkaixue commented on code in PR #2249: URL: https://github.com/apache/helix/pull/2249#discussion_r1023123650
########## meta-client/src/main/java/org/apache/helix/metaclient/api/ConnectStateChangeListener.java: ########## @@ -0,0 +1,55 @@ +package org.apache.helix.metaclient.api; + +/* + * 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 interface ConnectStateChangeListener { + + enum ConnectState { + // Client is connected to server + CONNECTED, + + // Authentication failed. + AUTH_FAILED, + + // Server has expired this connection. + EXPIRED, + + // When client failed to connect server. + INIT_FAILED, + + // When client explicitly call disconnect. + CLOSED_BY_CLIENT + } + + + /** + * Called when the connection state has changed. + */ + void handleConnectStateChanged(ConnectState currentState) throws Exception; + /** + * Called when new connection is established + */ + void handleNewConnection(final String sessionId) throws Exception; Review Comment: Are we allowing session ID passin? This is confusing... ########## meta-client/src/main/java/org/apache/helix/metaclient/api/DirectSubEntryChangeListener.java: ########## @@ -0,0 +1,30 @@ +package org.apache.helix.metaclient.api; + +/* + * 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. + */ + +/** + * Listener interface for direct subentry change event on a particular key. It includes new + * subentry creation or subentry deletion. The callback won't differentiate these types. + * For hierarchy key spaces like zookeeper, it refers to an entry's direct subentry nodes. + * For flat key spaces, it refers to keys that matches `prefix*separator`. + */ +public interface DirectSubEntryChangeListener { Review Comment: Subentry? Do you mean children？ ########## meta-client/src/main/java/org/apache/helix/metaclient/api/MetaClientInterface.java: ########## @@ -20,139 +20,382 @@ */ import java.util.List; +import java.util.concurrent.TimeUnit; public interface MetaClientInterface<T> { enum EntryMode { - //The node will be removed automatically when the session associated with the creation + // The node will be removed automatically when the session associated with the creation // of the node expires. EPHEMERAL, - //The node will not be automatically deleted upon client's disconnect. - PERSISTENT + + // The node will not be automatically deleted upon client's disconnect. + // An ephemeral node cannot have sub entry. + PERSISTENT, + + // The node will not be automatically deleted when the last sub-entry of the node is deleted. + // The node is an ephemeral node. + CONTAINER } /** * Interface representing the metadata of an entry. It contains entry type and version number. + * TODO: we will add session ID to entry stats in the future */ class Stat { private int _version; private EntryMode _entryMode; - public EntryMode getEntryType() {return _entryMode;} - public int getVersion() {return _version;} + public EntryMode getEntryType() { + return _entryMode; + } + + public int getVersion() { + return _version; + } } //synced CRUD API - void create(final String key, T data, final EntryMode mode); - void create(final String key, T data, final EntryMode mode, long ttl); + /** + * Create an persistent entry with given key and data. The entry will not be created if there is + * an existing entry with the same key. + * @param key key to identify the entry + * @param data value of the entry + */ + void create(final String key, T data); + + /** + * Create an entry of given EntryMode with given key and data. The entry will not be created if + * there is an existing entry with ethe same key. + * @param key key to identify the entry + * @param data value of the entry + * @param mode EntryMode identifying if the entry will be deleted upon client disconnect + */ + void create(final String key, final T data, final EntryMode mode); + + // TODO: add TTL create and renew API /** * Set the data for the entry of the given key if it exists and the given version matches the * version of the node (if the given version is -1, it matches any node's versions). + * @param key key to identify the entry + * @param data new data of the entry + * @param version expected version of the entry. -1 matched any version. */ - void set(final String key, T data, int version); + void set(final String key, final T data, int version); /** * Update existing data of a given key using an updater. This method will issue a read to get * current data and apply updater upon the current data. - * @param updater : An updater that modifies the entry value. + * @param key key to identify the entry + * @param updater An updater that modifies the entry value. * @return: the updated value. */ - T update(String key, DataUpdater<T> updater); + T update(final String key, DataUpdater<T> updater); /** * Check if there is an entry for the given key. - * @param key + * @param key key to identify the entry * @return return a Stat object if the entry exists. Return null otherwise. */ Stat exists(final String key); /** * Fetch the data for a given key. * TODO: define exception type when key does not exist + * @param key key to identify the entry + * @return Return data of the entry */ - T get(String key); + T get(final String key); /** * API for transaction. The list of operation will be executed as an atomic operation. * @param ops a list of operations. These operations will all be executed or non of them. - * @return + * @return Return a list of OpResult. */ List<OpResult> transactionOP(final Iterable<Op> ops); /** - * Return a list of sub entries for the given keys - * @param path: For metadata storage that has hierarchical key space (e.g. ZK), the path would be - * a parent path, - * For metadata storage that has non-hierarchical key space (e.g. etcd), the path would - * be a prefix path. + * Return a list of sub entries for the given keys. + * @param key For metadata storage that has hierarchical key space (e.g. ZK), the key would be + * a parent key, + * For metadata storage that has non-hierarchical key space (e.g. etcd), the key would + * be a prefix key. + * @eturn Return a list of sub entry keys. Return direct child name only for hierarchical key + * space, return the whole sub key for non-hierarchical key space. */ - List<String> getSubEntryKeys(final String path); + List<String> getSubEntryKeys(final String key); /** - * Return the number of sub entries for the given keys - * @param path: For metadata storage that has hierarchical key space (e.g. ZK), the path would be - * a parent path, - * For metadata storage that has non-hierarchical key space (e.g. etcd), the path would - * be a prefix path. + * Return the number of sub entries for the given keys. + * @param key For metadata storage that has hierarchical key space (e.g. ZK), the key would be + * a parent key, + * For metadata storage that has non-hierarchical key space (e.g. etcd), the key would + * be a prefix key. */ - int countSubEntries(final String path); + int countSubEntries(final String key); /** * Remove the entry associated with the given key. * For metadata storage that has hierarchical key space, the entry can only be deleted if the key * has no child entry. - * TODO: throws - * @param path - * @return + * TODO: define exception to throw + * @param key key to identify the entry to delete + * @return Return true if the deletion is completed */ - boolean delete(String path); + boolean delete(final String key); /** * Remove the entry associated with the given key. * For metadata storage that has hierarchical key space, remove all its child entries as well * For metadata storage that has non-hierarchical key space, this API is the same as delete() - * @param path - * @return + * @param key key to identify the entry to delete + * @return Return true if the deletion is completed */ - boolean recursiveDelete(String path); + boolean recursiveDelete(final String key); /* Asynchronous methods return immediately. * They take a callback object that will be executed either on successful execution of the request * or on error with an appropriate return code indicating the error. */ - void asyncCreate(final String key, T data, int version, long ttl, + + /** + * The asynchronous version of create. + * @param key key to identify the entry + * @param data value of the entry + * @param mode EntryMode identifying if the entry will be deleted upon client disconnect + * @param cb An user defined VoidCallback implementation that will be invoked when async create return. + */ + void asyncCreate(final String key, final T data, final EntryMode mode, Review Comment: All these async APIs, shall we return some results? Or we rely on callbacks? How about exceptions happen? ########## meta-client/src/main/java/org/apache/helix/metaclient/api/MetaClientInterface.java: ########## @@ -20,139 +20,382 @@ */ import java.util.List; +import java.util.concurrent.TimeUnit; public interface MetaClientInterface<T> { enum EntryMode { - //The node will be removed automatically when the session associated with the creation + // The node will be removed automatically when the session associated with the creation // of the node expires. EPHEMERAL, - //The node will not be automatically deleted upon client's disconnect. - PERSISTENT + + // The node will not be automatically deleted upon client's disconnect. + // An ephemeral node cannot have sub entry. + PERSISTENT, + + // The node will not be automatically deleted when the last sub-entry of the node is deleted. + // The node is an ephemeral node. + CONTAINER } /** * Interface representing the metadata of an entry. It contains entry type and version number. + * TODO: we will add session ID to entry stats in the future */ class Stat { private int _version; private EntryMode _entryMode; - public EntryMode getEntryType() {return _entryMode;} - public int getVersion() {return _version;} + public EntryMode getEntryType() { + return _entryMode; + } + + public int getVersion() { + return _version; + } } //synced CRUD API - void create(final String key, T data, final EntryMode mode); - void create(final String key, T data, final EntryMode mode, long ttl); + /** + * Create an persistent entry with given key and data. The entry will not be created if there is + * an existing entry with the same key. + * @param key key to identify the entry + * @param data value of the entry + */ + void create(final String key, T data); + + /** + * Create an entry of given EntryMode with given key and data. The entry will not be created if + * there is an existing entry with ethe same key. + * @param key key to identify the entry + * @param data value of the entry + * @param mode EntryMode identifying if the entry will be deleted upon client disconnect + */ + void create(final String key, final T data, final EntryMode mode); + + // TODO: add TTL create and renew API /** * Set the data for the entry of the given key if it exists and the given version matches the * version of the node (if the given version is -1, it matches any node's versions). + * @param key key to identify the entry + * @param data new data of the entry + * @param version expected version of the entry. -1 matched any version. */ - void set(final String key, T data, int version); + void set(final String key, final T data, int version); /** * Update existing data of a given key using an updater. This method will issue a read to get * current data and apply updater upon the current data. - * @param updater : An updater that modifies the entry value. + * @param key key to identify the entry + * @param updater An updater that modifies the entry value. * @return: the updated value. */ - T update(String key, DataUpdater<T> updater); + T update(final String key, DataUpdater<T> updater); /** * Check if there is an entry for the given key. - * @param key + * @param key key to identify the entry * @return return a Stat object if the entry exists. Return null otherwise. */ Stat exists(final String key); /** * Fetch the data for a given key. * TODO: define exception type when key does not exist + * @param key key to identify the entry + * @return Return data of the entry */ - T get(String key); + T get(final String key); /** * API for transaction. The list of operation will be executed as an atomic operation. * @param ops a list of operations. These operations will all be executed or non of them. - * @return + * @return Return a list of OpResult. */ List<OpResult> transactionOP(final Iterable<Op> ops); /** - * Return a list of sub entries for the given keys - * @param path: For metadata storage that has hierarchical key space (e.g. ZK), the path would be - * a parent path, - * For metadata storage that has non-hierarchical key space (e.g. etcd), the path would - * be a prefix path. + * Return a list of sub entries for the given keys. + * @param key For metadata storage that has hierarchical key space (e.g. ZK), the key would be + * a parent key, + * For metadata storage that has non-hierarchical key space (e.g. etcd), the key would + * be a prefix key. + * @eturn Return a list of sub entry keys. Return direct child name only for hierarchical key + * space, return the whole sub key for non-hierarchical key space. */ - List<String> getSubEntryKeys(final String path); + List<String> getSubEntryKeys(final String key); /** - * Return the number of sub entries for the given keys - * @param path: For metadata storage that has hierarchical key space (e.g. ZK), the path would be - * a parent path, - * For metadata storage that has non-hierarchical key space (e.g. etcd), the path would - * be a prefix path. + * Return the number of sub entries for the given keys. + * @param key For metadata storage that has hierarchical key space (e.g. ZK), the key would be + * a parent key, + * For metadata storage that has non-hierarchical key space (e.g. etcd), the key would + * be a prefix key. */ - int countSubEntries(final String path); + int countSubEntries(final String key); /** * Remove the entry associated with the given key. * For metadata storage that has hierarchical key space, the entry can only be deleted if the key * has no child entry. - * TODO: throws - * @param path - * @return + * TODO: define exception to throw + * @param key key to identify the entry to delete + * @return Return true if the deletion is completed */ - boolean delete(String path); + boolean delete(final String key); /** * Remove the entry associated with the given key. * For metadata storage that has hierarchical key space, remove all its child entries as well * For metadata storage that has non-hierarchical key space, this API is the same as delete() - * @param path - * @return + * @param key key to identify the entry to delete + * @return Return true if the deletion is completed */ - boolean recursiveDelete(String path); + boolean recursiveDelete(final String key); /* Asynchronous methods return immediately. * They take a callback object that will be executed either on successful execution of the request * or on error with an appropriate return code indicating the error. */ - void asyncCreate(final String key, T data, int version, long ttl, + + /** + * The asynchronous version of create. + * @param key key to identify the entry + * @param data value of the entry + * @param mode EntryMode identifying if the entry will be deleted upon client disconnect + * @param cb An user defined VoidCallback implementation that will be invoked when async create return. + */ + void asyncCreate(final String key, final T data, final EntryMode mode, AsyncCallback.VoidCallback cb); - void asyncSet(final String key, T data, int version, AsyncCallback.VoidCallback cb); + /** + * The asynchronous version of set. + * @param key key to identify the entry + * @param data new data of the entry + * @param version expected version if the entry. -1 matched any version + * @param cb An user defined VoidCallback implementation that will be invoked when async create return. + */ + void asyncSet(final String key, final T data, final int version, AsyncCallback.VoidCallback cb); - void asyncUpdate(final String key, DataUpdater<T> updater, AsyncCallback.VoidCallback cb); + /** + * The asynchronous version of update. + * @param key key to identify the entry + * @param updater An updater that modifies the entry value. + * @param cb An user defined VoidCallback implementation that will be invoked when async create return. + * It will contain the newly updated data if update succeeded. + */ + void asyncUpdate(final String key, DataUpdater<T> updater, AsyncCallback.DataCallback cb); + /** + * The asynchronous version of get. + * @param key key to identify the entry + * @param cb An user defined VoidCallback implementation that will be invoked when async create return. + * It will contain the entry data if get succeeded. + */ void asyncGet(final String key, AsyncCallback.DataCallback cb); - void asyncCountSubEntries(final String path, AsyncCallback.DataCallback cb); + /** + * The asynchronous version of get sub entries. + * @param key key to identify the entry + * @param cb An user defined VoidCallback implementation that will be invoked when async create return. + * It will contain the list of sub entry keys if succeeded. + */ + void asyncCountSubEntries(final String key, AsyncCallback.DataCallback cb); + /** + * The asynchronous version of get sub entries. + * @param key key to identify the entry + * @param cb An user defined VoidCallback implementation that will be invoked when async create return. + * It will contain the stats of the entry if succeeded. + */ void asyncExist(final String key, AsyncCallback.StatCallback cb); - void asyncDelete(final String keys, AsyncCallback.VoidCallback cb); + /** + * The asynchronous version of delete. + * @param key key to identify the entry + * @param cb An user defined VoidCallback implementation that will be invoked when async delete return. + */ + void asyncDelete(final String key, AsyncCallback.VoidCallback cb); - void asyncTransaction(final String keys, AsyncCallback.TransactionCallback cb); + /** + * The asynchronous version of transaction operations. + * @param ops A list of operations + * @param cb An user defined TransactionCallback implementation that will be invoked when transaction operations return. + */ + void asyncTransaction(final Iterable<Op> ops, AsyncCallback.TransactionCallback cb); Review Comment: Especially for transaction, if any rollback operations required, do you expect the callback is the one user providing rollback operations? Or this callback is just designed for user return. ########## meta-client/src/main/java/org/apache/helix/metaclient/api/SubEntryChangeListener.java: ########## @@ -0,0 +1,40 @@ +package org.apache.helix.metaclient.api; + +/* + * 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. + */ + +/** + * Listener interface for children change events on a particular key. It includes new subentry + * creation, subentry deletion, subentry data change and when the listener is removed. + * This listener type can only be registered as a persist listener. + * For hierarchy key spaces like zookeeper, it refers to an entry's entire subtree. + * For flat key spaces, it refers to keys that matches `prefix*`. + */ +public interface SubEntryChangeListener { Review Comment: Same as commented before. I would suggest to use children instead of subentry. Because if this is not hierarchy data structure, then there is no sub entry concept. But if it is hierarchy structure, then children makes more sense as the world. Even for prefix match, subentry is confusing word as "sub" -- 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]
