hangc0276 commented on code in PR #4035:
URL: https://github.com/apache/bookkeeper/pull/4035#discussion_r1268158517


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java:
##########
@@ -279,6 +279,13 @@ public class ServerConfiguration extends 
AbstractConfiguration<ServerConfigurati
     protected static final String HTTP_SERVER_TRUST_STORE_PATH = 
"httpServerTrustStorePath";
     protected static final String HTTP_SERVER_TRUST_STORE_PASSWORD = 
"httpServerTrustStorePassword";
 
+    // Table Service parameters
+    protected static final String TABLE_NUM_BOOKIES = "tableNumBookies";

Review Comment:
   The table service run within the Bookie, why do we need to set 
tableNumBookies?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java:
##########
@@ -4102,4 +4109,113 @@ public ServerConfiguration 
setLedgerMetadataRocksdbConf(String ledgerMetadataRoc
         this.setProperty(LEDGER_METADATA_ROCKSDB_CONF, 
ledgerMetadataRocksdbConf);
         return this;
     }
+
+    /**
+     * Returns the number of bookies to run in this cluster.
+     *
+     * @return the number of bookies to run in this cluster.
+     */
+    public int getNumBookies() {
+        return getInt(TABLE_NUM_BOOKIES, 1);
+    }
+
+    /**
+     * Set the number of bookies to run in this cluster.
+     *
+     * @param numBookies the number of bookies
+     * @return ServerConfiguration
+     */
+    public ServerConfiguration setNumBookies(int numBookies) {
+        setProperty(TABLE_NUM_BOOKIES, numBookies);
+        return this;
+    }
+
+    /**
+     * Returns if should start zookeeper.
+     *
+     * @return true if should start zookeeper, otherwise false.
+     */
+    public boolean getShouldStartZooKeeper() throws ConfigurationException {
+        return getMetadataServiceUri() == null;
+    }
+
+    /**
+     * Returns the zookeeper server port used in this cluster.
+     *
+     * @return the zookeeper server port used in this cluster.
+     */
+    public int getZKPort() {

Review Comment:
   The table service shares the same Zookeeper with the Bookie, why do we set 
the zookeeper port?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java:
##########
@@ -4102,4 +4109,113 @@ public ServerConfiguration 
setLedgerMetadataRocksdbConf(String ledgerMetadataRoc
         this.setProperty(LEDGER_METADATA_ROCKSDB_CONF, 
ledgerMetadataRocksdbConf);
         return this;
     }
+
+    /**
+     * Returns the number of bookies to run in this cluster.
+     *
+     * @return the number of bookies to run in this cluster.
+     */
+    public int getNumBookies() {
+        return getInt(TABLE_NUM_BOOKIES, 1);
+    }
+
+    /**
+     * Set the number of bookies to run in this cluster.
+     *
+     * @param numBookies the number of bookies
+     * @return ServerConfiguration
+     */
+    public ServerConfiguration setNumBookies(int numBookies) {
+        setProperty(TABLE_NUM_BOOKIES, numBookies);
+        return this;
+    }
+
+    /**
+     * Returns if should start zookeeper.
+     *
+     * @return true if should start zookeeper, otherwise false.
+     */
+    public boolean getShouldStartZooKeeper() throws ConfigurationException {

Review Comment:
   Why do we need this one?



##########
conf/standalone.conf:
##########
@@ -23,15 +23,68 @@
 #bookieId=
 #allowLoopback=false
 
+############################################## Bookie Storage 
##############################################

Review Comment:
   Please remove the un-related configurations



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java:
##########
@@ -279,6 +279,13 @@ public class ServerConfiguration extends 
AbstractConfiguration<ServerConfigurati
     protected static final String HTTP_SERVER_TRUST_STORE_PATH = 
"httpServerTrustStorePath";
     protected static final String HTTP_SERVER_TRUST_STORE_PASSWORD = 
"httpServerTrustStorePassword";
 
+    // Table Service parameters
+    protected static final String TABLE_NUM_BOOKIES = "tableNumBookies";
+    protected static final String TABLE_ZK_PORT = "tableZKPort";
+    protected static final String TABLE_START_BOOKIE_AND_START_PROVIDER = 
"startBookieAndStartProvider";

Review Comment:
   Can we simplify the parameter to `enableTableService`?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java:
##########
@@ -4102,4 +4109,113 @@ public ServerConfiguration 
setLedgerMetadataRocksdbConf(String ledgerMetadataRoc
         this.setProperty(LEDGER_METADATA_ROCKSDB_CONF, 
ledgerMetadataRocksdbConf);
         return this;
     }
+
+    /**
+     * Returns the number of bookies to run in this cluster.
+     *
+     * @return the number of bookies to run in this cluster.
+     */
+    public int getNumBookies() {
+        return getInt(TABLE_NUM_BOOKIES, 1);
+    }
+
+    /**
+     * Set the number of bookies to run in this cluster.
+     *
+     * @param numBookies the number of bookies
+     * @return ServerConfiguration
+     */
+    public ServerConfiguration setNumBookies(int numBookies) {
+        setProperty(TABLE_NUM_BOOKIES, numBookies);
+        return this;
+    }
+
+    /**
+     * Returns if should start zookeeper.
+     *
+     * @return true if should start zookeeper, otherwise false.
+     */
+    public boolean getShouldStartZooKeeper() throws ConfigurationException {
+        return getMetadataServiceUri() == null;
+    }
+
+    /**
+     * Returns the zookeeper server port used in this cluster.
+     *
+     * @return the zookeeper server port used in this cluster.
+     */
+    public int getZKPort() {
+        return getInt(TABLE_ZK_PORT, 2181);
+    }
+
+    /**
+     * Set the zookeeper server port used in this cluster.
+     *
+     * @param zkPort zookeeper server port
+     * @return ServerConfiguration
+     */
+    public ServerConfiguration setZKPort(int zkPort) {
+        setProperty(TABLE_ZK_PORT, zkPort);
+        return this;
+    }
+
+    /**
+     * Returns the gRPC server port used in this cluster.
+     *
+     * @return the gRPC server port used in this cluster.
+     */
+    public int getInitialBookieGrpcPort() {
+        return getInt(TABLE_INITIAL_GRPC_PORT, 4181);
+    }
+
+    /**
+     * Set the gRPC server port used in this cluster.
+     *
+     * @param initialBookieGrpcPort gRPC server port
+     * @return ServerConfiguration
+     */
+    public ServerConfiguration setInitialBookieGrpcPort(int 
initialBookieGrpcPort) {
+        setProperty(TABLE_INITIAL_GRPC_PORT, initialBookieGrpcPort);
+        return this;
+    }
+
+    /**
+     * The local storage directories for storing table ranges data (e.g. 
rocksdb sst files).
+     *
+     * @return local storage directories for storing table ranges data.
+     */
+    public String getStorageRangeStoreDirs() {

Review Comment:
   Can we share the same directory with ledgers?



-- 
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]

Reply via email to