You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/02/08 01:52:01 UTC

[GitHub] [pulsar] RobertIndie commented on a change in pull request #14147: PIP 45: Deprecate zookeeper settings

RobertIndie commented on a change in pull request #14147:
URL: https://github.com/apache/pulsar/pull/14147#discussion_r801203494



##########
File path: pulsar-websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java
##########
@@ -72,12 +72,20 @@
     @FieldContext(doc = "Connection string of configuration metadata store servers")
     private String configurationMetadataStoreUrl;
 
-    @FieldContext(doc = "ZooKeeper session timeout in milliseconds")
+    @FieldContext(doc = "ZooKeeper session timeout in milliseconds. "
+            + "@deprecated - Use metadataStoreSessionTimeoutMillis instead.")
     private long zooKeeperSessionTimeoutMillis = 30000;

Review comment:
       ```suggestion
       @Deprecated
       @FieldContext(doc = "ZooKeeper session timeout in milliseconds. "
               + "@deprecated - Use metadataStoreSessionTimeoutMillis instead.",
               deprecated = true)
       private long zooKeeperSessionTimeoutMillis = 30000;
   ```
   It's better to add `deprecated` here.

##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1609,9 +1609,6 @@ public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfigu
         workerConfig.setAuthorizationEnabled(brokerConfig.isAuthorizationEnabled());
         workerConfig.setAuthorizationProvider(brokerConfig.getAuthorizationProvider());
         workerConfig.setConfigurationMetadataStoreUrl(brokerConfig.getConfigurationMetadataStoreUrl());
-        workerConfig.setZooKeeperSessionTimeoutMillis(brokerConfig.getZooKeeperSessionTimeoutMillis());
-        workerConfig.setZooKeeperOperationTimeoutSeconds(brokerConfig.getZooKeeperOperationTimeoutSeconds());
-

Review comment:
       Seems that we need to set metadataStoreSessionTimeoutMillis etc. for the workerConfig here.

##########
File path: pulsar-websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java
##########
@@ -72,12 +72,20 @@
     @FieldContext(doc = "Connection string of configuration metadata store servers")
     private String configurationMetadataStoreUrl;
 
-    @FieldContext(doc = "ZooKeeper session timeout in milliseconds")
+    @FieldContext(doc = "ZooKeeper session timeout in milliseconds. "
+            + "@deprecated - Use metadataStoreSessionTimeoutMillis instead.")
     private long zooKeeperSessionTimeoutMillis = 30000;
 
-    @FieldContext(doc = "ZooKeeper cache expiry time in seconds")
+    @FieldContext(doc = "ZooKeeper cache expiry time in seconds. "
+            + "@deprecated - Use metadataStoreCacheExpirySeconds instead.")
     private int zooKeeperCacheExpirySeconds = 300;

Review comment:
       ```suggestion
       @Deprecated
       @FieldContext(doc = "ZooKeeper cache expiry time in seconds. "
               + "@deprecated - Use metadataStoreCacheExpirySeconds instead.",
               deprecated = true)
       private int zooKeeperCacheExpirySeconds = 300;
   ```
   It's better to add `deprecated` here.




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org