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/05/06 22:00:32 UTC

[GitHub] [pulsar] merlimat opened a new pull request, #15478: PIP-117: Change Pulsar standalone defaults

merlimat opened a new pull request, #15478:
URL: https://github.com/apache/pulsar/pull/15478

   Master Issue: #13302
   
   ### Motivation
   
    * Change standalone metadata impl from ZooKeeper to RocksDB
   
    * Pulsar functions package backend -->  Change from using DistributedLog to using local filesystem, storing the
   jars directly in the data folder instead of uploading them into BK.
   
    * Pulsar functions state store implementation --> Change the state store to be backed by a MetadataStore based backed,
   with the RocksDB implementation.
   
    * Table Service --> Do not start BK table service by default


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


[GitHub] [pulsar] github-actions[bot] commented on pull request #15478: PIP-117: Change Pulsar standalone defaults

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15478:
URL: https://github.com/apache/pulsar/pull/15478#issuecomment-1120041222

   @merlimat:Thanks for your contribution. For this PR, do we need to update docs?
   (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? 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: commits-unsubscribe@pulsar.apache.org

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


[GitHub] [pulsar] tisonkun commented on a diff in pull request #15478: PIP-117: Change Pulsar standalone defaults

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #15478:
URL: https://github.com/apache/pulsar/pull/15478#discussion_r917236963


##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/MetadataStoreFactoryImpl.java:
##########
@@ -81,4 +81,12 @@ public static String removeIdentifierFromMetadataURL(String metadataURL) {
         }
         return metadataURL;
     }
+
+    public static boolean isBasedOnZookeeper(String metadataURL) {
+        if (!metadataURL.contains("://")) {

Review Comment:
   @merlimat it seems that metadata url doesn't contains `//` part?
   
   From the codebase a metadata url to zk is `zk:127.0.0.1:2181` and a metadata url to memory is `memory:local`.



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


[GitHub] [pulsar] eolivelli commented on a diff in pull request #15478: PIP-117: Change Pulsar standalone defaults

Posted by GitBox <gi...@apache.org>.
eolivelli commented on code in PR #15478:
URL: https://github.com/apache/pulsar/pull/15478#discussion_r867436696


##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarStandalone.java:
##########
@@ -246,7 +259,10 @@ public boolean isHelp() {
     @Parameter(names = { "-h", "--help" }, description = "Show this help message")
     private boolean help = false;
 
+    private boolean usingNew211Defaults;

Review Comment:
   Using version smells a little from my point ofbview.
   What about using PIP117 as reference?
   usingPIP117Defaults
   or we can invert the flag and name it 'useLegscyDefaults' and cite this PIP with a link



##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarStandalone.java:
##########
@@ -246,7 +259,10 @@ public boolean isHelp() {
     @Parameter(names = { "-h", "--help" }, description = "Show this help message")
     private boolean help = false;
 
+    private boolean usingNew211Defaults;
+
     public void start() throws Exception {
+        usingNew211Defaults = !Paths.get(zkDir).toFile().exists();

Review Comment:
   Os this is true maybe we could log a line explaining that we found  the directory and we are switching to the old defaults



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


[GitHub] [pulsar] merlimat commented on pull request #15478: PIP-117: Change Pulsar standalone defaults

Posted by GitBox <gi...@apache.org>.
merlimat commented on PR #15478:
URL: https://github.com/apache/pulsar/pull/15478#issuecomment-1124388722

   > Some thought (I voted the PIP but now that we are close to committing the patch some doibts camento my mind) Disabling zookeeper by default may have some nasty impact of integration tests of many applications and that run the same test against many versions of Pulsar.
   > 
   > I wonder if there is some easy way, especially for docker users, like passing a single ENV entry of telling Pulsar standalone to run with zookeeper. It will be quite difficult to support probably but I am now imagining the pain for making integration tests of many applications work with different versions of Pulsar
   
   I'm not sure I'm following here. The external interface of Pulsar does not change whether its metadata is stored in ZK or RocksDb. 
   
   I agree though that it's probably a good idea to leave a CLI flag to force the ZK mode, to leave the option open, but that wouldn't be the default.
   


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


[GitHub] [pulsar] Jason918 commented on a diff in pull request #15478: PIP-117: Change Pulsar standalone defaults

Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #15478:
URL: https://github.com/apache/pulsar/pull/15478#discussion_r867440862


##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarStandalone.java:
##########
@@ -246,7 +259,10 @@ public boolean isHelp() {
     @Parameter(names = { "-h", "--help" }, description = "Show this help message")
     private boolean help = false;
 
+    private boolean usingNew211Defaults;
+
     public void start() throws Exception {
+        usingNew211Defaults = !Paths.get(zkDir).toFile().exists();

Review Comment:
   A warning log would be great when both `--metadata-dir` is provided and `zkDir` exists.



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


[GitHub] [pulsar] codelipenghui commented on pull request #15478: PIP-117: Change Pulsar standalone defaults

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on PR #15478:
URL: https://github.com/apache/pulsar/pull/15478#issuecomment-1120330113

   /pulsarbot run-failure-checks


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


[GitHub] [pulsar] merlimat merged pull request #15478: PIP-117: Change Pulsar standalone defaults

Posted by GitBox <gi...@apache.org>.
merlimat merged PR #15478:
URL: https://github.com/apache/pulsar/pull/15478


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