Skip to content

Speed up the process to start standalone for tests #236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
BewareMyPower merged 1 commit into from
Apr 10, 2023
Merged

Speed up the process to start standalone for tests #236

BewareMyPower merged 1 commit into from
Apr 10, 2023

Conversation

@BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Mar 29, 2023

Motivation

It took too much time to start Pulsar standalone for tests because of the slow JVM startup when running pulsar-admin commands.

Modifications

Use curl to send REST requests directly. After this change, the startup time reduced from 1m39.597s to 32.451s in my local env.

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@BewareMyPower
Speed up the process to start standalone for tests
d6f5323 
### Motivation

It took too much time to start Pulsar standalone for tests because of
the slow JVM startup when running `pulsar-admin` commands.

### Modifications

Use `curl` to send REST requests directly. After this change, the
startup time reduced from 1m39.597s to 32.451s in my local env.
@BewareMyPower BewareMyPower added the enhancement New feature or request label Mar 29, 2023
@BewareMyPower BewareMyPower added this to the 3.2.0 milestone Mar 29, 2023
@BewareMyPower BewareMyPower self-assigned this Mar 29, 2023
RobertIndie
RobertIndie reviewed Mar 29, 2023
View reviewed changes
Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to use pulsar-shell instead of using raw HTTP calls?
We can use the no-interactive mode of the pulsar shell: https://pulsar.apache.org/docs/next/administration-pulsar-shell/#run-commands-sequentially

@BewareMyPower
Copy link
Contributor Author

BewareMyPower commented Mar 29, 2023

Is it better to use pulsar-shell instead of using raw HTTP calls?

If we're going to use pulsar-shell, we need to download it first and uncompress it to a specific path, then add separated config files for each cluster. But curl is already installed in most of the Linux/Unix systems by default.

For this case, using curl is simple and enough and I didn't see any advantage of pulsar-shell. pulsar-shell could have pretty output for JSON, but here we only need to create the namespaces and configure the policies.

@RobertIndie
Copy link
Member

RobertIndie commented Mar 30, 2023

@BewareMyPower Why do we need to download it separately? This script is running inside the docker. The pulsar after 2.11 already contains the pulsar-shell. We just need to use it like the pulsar-admin.

Using the curl would reduce the readability. It makes the script more complicate.

@BewareMyPower
Copy link
Contributor Author

BewareMyPower commented Mar 30, 2023

The pulsar after 2.11 already contains the pulsar-shell.

Okay. I will replace it with the pulsar-shell command.

@BewareMyPower
Copy link
Contributor Author

BewareMyPower commented Apr 6, 2023

When I tried to write shell scripts for pulsar-shell, I found it's hard to split it into two lines. For example, I met the following error:

[5/29] ? admin namespaces create public/default -c standalone
[6/29] Executing admin namespaces grant-permission
The following options are required: [--actions], [--role]

while my script is:

admin namespaces create public/default -c standalone
admin namespaces grant-permission \
    --actions produce,consume --role anonymous \
    public/default

The backslash seems not recognized, which is bad from my view.

BTW, I think the readability is not bad even with the raw REST APIs.

  1. I wrote the comments for each REST call.
  2. The curl command is encapsulated into a shell function put <path> <body>.

In addition, it can also be easily used to verify an older Pulsar image (< 2.11.0) to test the backward compatibility.

@RobertIndie

RobertIndie
RobertIndie approved these changes Apr 10, 2023
View reviewed changes
Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. Make sense to me.

@BewareMyPower BewareMyPower merged commit 2f61c62 into apache:main Apr 10, 2023
@BewareMyPower BewareMyPower deleted the bewaremypower/speedup-tests branch April 10, 2023 03:28
@BewareMyPower BewareMyPower mentioned this pull request Apr 14, 2023
Merged
4 tasks
BewareMyPower added a commit to BewareMyPower/pulsar-client-python that referenced this pull request Nov 16, 2023
@BewareMyPower
Improve the developer experience when running tests
f54c831 
Migrate the PRs from the pulsar-client-cpp repo:
- apache/pulsar-client-cpp#297
- apache/pulsar-client-cpp#236
- apache/pulsar-client-cpp#340
@BewareMyPower BewareMyPower mentioned this pull request Nov 16, 2023
Merged
BewareMyPower added a commit to apache/pulsar-client-python that referenced this pull request Nov 16, 2023
@BewareMyPower
Improve the developer experience when running tests (#163)
d38c156 
Migrate the PRs from the pulsar-client-cpp repo:
- apache/pulsar-client-cpp#297
- apache/pulsar-client-cpp#236
- apache/pulsar-client-cpp#340
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RobertIndie RobertIndie RobertIndie approved these changes

@shibd shibd shibd approved these changes

Assignees

@BewareMyPower BewareMyPower

Labels

enhancement New feature or request

Projects

None yet

Milestone

3.2.0

Development

Successfully merging this pull request may close these issues.

3 participants

@BewareMyPower @RobertIndie @shibd