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/06/09 15:53:49 UTC

[GitHub] [pulsar] lgxbslgx opened a new pull request, #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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

   Fixes #15830
   
   ### Motivation
   
   Currently, when the users use JDK17 and run the command `./bin/pulsar sql-worker run` according to the document [Query data with Pulsar SQL](https://pulsar.apache.org/docs/next/sql-getting-started), they will get an error. We need to direct the user to add the options `--add-opens java.base/java.lang=ALL-UNNAMED` to config file so that the SQL worker can be started successfully.
   
   ### Modifications
   
   Add a step about JDK17 configuration to the Pulsar SQL document.
   I only revise the document of version `2.10.0` and `next` version now.
   
   ### Verifying this change
   
   - [x] Make sure that the change passes the CI checks.
   
   ### Documentation
   
   - [x] `doc` 
   


-- 
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] lgxbslgx commented on a diff in pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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


##########
site2/docs/sql-getting-started.md:
##########
@@ -21,23 +21,34 @@ To query data in Pulsar with Pulsar SQL, complete the following steps.
 
 ```
 
-2. Start a Pulsar SQL worker.
+2. Adjust configuration. (JDK17)

Review Comment:
   Fixed.



-- 
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] nicoloboschi commented on a diff in pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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


##########
site2/docs/sql-getting-started.md:
##########
@@ -21,23 +21,33 @@ To query data in Pulsar with Pulsar SQL, complete the following steps.
 
 ```
 
-2. Start a Pulsar SQL worker.
+2. Adjust configuration.
+
+If you use JDK17 and Pulsar version is less or equal than 2.10, you need to add the following options to the configuration file `conf/presto/jvm.config`.

Review Comment:
   In Pulsar 2.10.1 we fixed the issue, see: https://github.com/apache/pulsar/pull/14300
   I'm not sure it's even worth to add this document paragraph since it will be deprecated very soon



-- 
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 closed pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

Posted by GitBox <gi...@apache.org>.
tisonkun closed pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document
URL: https://github.com/apache/pulsar/pull/15998


-- 
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] lgxbslgx commented on a diff in pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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


##########
site2/docs/sql-getting-started.md:
##########
@@ -21,23 +21,34 @@ To query data in Pulsar with Pulsar SQL, complete the following steps.
 
 ```
 
-2. Start a Pulsar SQL worker.
+2. Adjust configuration. (JDK17)
+
+If you use JDK17, you need to add the following options to the configuration file `conf/presto/jvm.config`. Please note that these two lines can not be put into one line.
+
+```bash
+
+--add-opens

Review Comment:
   The config parser of the `lib/presto/bin/launcher.py` will add the quotation to each line, if we put them in one line, the option will be changed to `"--add-opens java.base/java.lang=ALL-UNNAMED"` which can not be identified by `java` command.
   
   You can write a simple class `Test.java`, compile it and run it by using option `"--add-opens java.base/java.lang=ALL-UNNAMED"`.
   
   ```shell
   $ javac Test.java
   $ java "--add-opens java.base/java.lang=ALL-UNNAMED" Test
   ```
   
   The `java` command will reply:
   ```
   Unrecognized option: --add-opens java.base/java.lang=ALL-UNNAMED
   Error: Could not create the Java Virtual Machine.
   Error: A fatal exception has occurred. Program will exit.
   ```
   
   The right usages are shown below (note the quatation):
   ```
   java --add-opens java.base/java.lang=ALL-UNNAMED FinalMethod
   java "--add-opens" "java.base/java.lang=ALL-UNNAMED" FinalMethod
   ```
   
   Or you can use the one line config and run the command of [current document](https://pulsar.apache.org/docs/sql-getting-started), the same error message `Unrecognized option: --add-opens java.base/java.lang=ALL-UNNAMED` also occurs.



-- 
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] lgxbslgx commented on pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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

   It seems the code in master branch (2.11.0-SNAPSHOT) uses the guice 5.1.0 instead of guice 4.2.3 so that https://github.com/apache/pulsar/issues/15830 doesn't occur at current master branch. I added the condition `Pulsar version is less or equal than 2.10` just now.


-- 
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] gaoran10 commented on a diff in pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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


##########
site2/docs/sql-getting-started.md:
##########
@@ -21,23 +21,34 @@ To query data in Pulsar with Pulsar SQL, complete the following steps.
 
 ```
 
-2. Start a Pulsar SQL worker.
+2. Adjust configuration. (JDK17)
+
+If you use JDK17, you need to add the following options to the configuration file `conf/presto/jvm.config`. Please note that these two lines can not be put into one line.
+
+```bash
+
+--add-opens

Review Comment:
   Do we need to separate this config into two lines? refer to https://trino.io/docs/current/installation/deployment.html#jvm-config, maybe one line is enough?



-- 
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] gaoran10 commented on a diff in pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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


##########
site2/docs/sql-getting-started.md:
##########
@@ -21,23 +21,34 @@ To query data in Pulsar with Pulsar SQL, complete the following steps.
 
 ```
 
-2. Start a Pulsar SQL worker.
+2. Adjust configuration. (JDK17)
+
+If you use JDK17, you need to add the following options to the configuration file `conf/presto/jvm.config`. Please note that these two lines can not be put into one line.
+
+```bash
+
+--add-opens

Review Comment:
   Could you try to use this `--add-opens=java.base/java.lang=ALL-UNNAMED`?



-- 
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 pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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

   Closed as conflicts and the issue has been fixed at 2.10.1.
   
   If you'd like to improve the doc, you can modify only the 2.10.x version and add a troubleshooting section below.


-- 
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 #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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

   The pr had no activity for 30 days, mark with Stale label.


-- 
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] lgxbslgx commented on a diff in pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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


##########
site2/docs/sql-getting-started.md:
##########
@@ -21,23 +21,34 @@ To query data in Pulsar with Pulsar SQL, complete the following steps.
 
 ```
 
-2. Start a Pulsar SQL worker.
+2. Adjust configuration. (JDK17)
+
+If you use JDK17, you need to add the following options to the configuration file `conf/presto/jvm.config`. Please note that these two lines can not be put into one line.
+
+```bash
+
+--add-opens

Review Comment:
   > Could you try to use this `--add-opens=java.base/java.lang=ALL-UNNAMED`?
   
   `--add-opens=java.base/java.lang=ALL-UNNAMED` works well. I missed such usage. The document was updated just now.



-- 
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] Anonymitaet commented on a diff in pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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


##########
site2/docs/sql-getting-started.md:
##########
@@ -21,23 +21,34 @@ To query data in Pulsar with Pulsar SQL, complete the following steps.
 
 ```
 
-2. Start a Pulsar SQL worker.
+2. Adjust configuration. (JDK17)

Review Comment:
   ```suggestion
   2. Adjust configuration. 
   ```
   remove unnecessary info  



-- 
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] lgxbslgx commented on a diff in pull request #15998: [doc] Add a step about JDK17 configuration to the Pulsar SQL document

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


##########
site2/docs/sql-getting-started.md:
##########
@@ -21,23 +21,33 @@ To query data in Pulsar with Pulsar SQL, complete the following steps.
 
 ```
 
-2. Start a Pulsar SQL worker.
+2. Adjust configuration.
+
+If you use JDK17 and Pulsar version is less or equal than 2.10, you need to add the following options to the configuration file `conf/presto/jvm.config`.

Review Comment:
   The concrete content and description of this patch may need to adjusted. Considering the users may use old version and also meet the bug like https://github.com/apache/pulsar/issues/15830, I suggest adding this document paragraph even it was solved at new version.



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