[improve][misc] Change PIP issue template

[asafm]

Motivation & Modifications

As discussed in the mailing list, the PIP issue template has several drawbacks. I'm now quoting the opening mail in the thread.

I would like to suggest two changes I'd like to make to the PIP design template:

1. Remove the form - just have a markdown template fill the issue body as it is created.
2. Change the PIP template structure

== Removing the form

Today, when you want to submit a PIP, you are required to fill out a form with boxes composed of 3-4 lines length.
It's not good because:

• It broadcasts to the author: we want a very small PIP, something that fits those small boxes.
• It makes the PIP look like a bug, where you fill out fields.
• It doesn't allow having H2 headings, only H1 headings, thus limiting the structure.

A PIP is a design, essentially, something 1–3 pages long. Thus, people take the time to write it down. Preferably, they copy and paste the body of the PIP issue, and use it to fill in sections.

My suggestion is to define an issue template using only Markdown, without a form.

== Changing PIP Structure

Today the structure of the PIP doc (pasted below), is missing a section and generally aims to jump directly into API changes / code / implementation. This results in lots of back and forth emails in an attempt to get the following essentials:

• All required background knowledge to understand the proposal
• A high level overview of the proposed solution
• Understanding how this proposal will be monitored
• What steps exactly I need to take if I revert to the previous version.

The structure I propose in this PR aims to reduce that friction and get all PIP aligned to provide that information.

Verifying this change

• Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

[github-actions]

@asafm Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

[michaeljmarshall]

This is a great initiative. Would you consider adding a section on multi-tenancy? I wrote a PR #19846 and then noticed this one. If we add it here, we can close my PR.

[asafm]

This is a great initiative. Would you consider adding a section on multi-tenancy? I wrote a PR #19846 and then noticed this one. If we add it here, we can close my PR.

@michaeljmarshall I want to, but I didn't understand the explanation provided in the PR, so I'm probably missing some context. Can you give some concrete examples?

[michaeljmarshall]

This is a great initiative. Would you consider adding a section on multi-tenancy? I wrote a PR #19846 and then noticed this one. If we add it here, we can close my PR.

@michaeljmarshall I want to, but I didn't understand the explanation provided in the PR, so I'm probably missing some context. Can you give some concrete examples?

I agree that the language could be improved. My main motivation is in response of features like stateful functions, which are not properly multi-tenant. This is clear when the stateful functions rely on direct access to the bookkeeper and/or zookeeper cluster(s). Further, the only implementations of the StateStore appear to be created without any thought to isolating tenants in such a way that one tenant cannot get/modify another tenant's function state.

In my mind, we should not accept any features into Pulsar that are not designed to be multi-tenant unless there is a specific reason we cannot make it so and the community explicitly accepts that reasoning.

Does that provide sufficient motivation? I am definitely open to different wording on the PIP template.

[asafm]

It's better.
Can you provide an example of a feature in pulsar which is dealing correctly with multi-tenancy?

Once I have a good example and a bad example, it will be easier to understand for the reader.

[asafm]

Regarding stateStore, you're basically saying they didn't take into consideration the security aspects of multi-tenancy, since that interface allows consuming any state by any tenant.

So permissions across tenants.
Any other aspect relarted to multi-tenancy with examples?

[michaeljmarshall]

Regarding stateStore, you're basically saying they didn't take into consideration the security aspects of multi-tenancy, since that interface allows consuming any state by any tenant.

So permissions across tenants. Any other aspect relarted to multi-tenancy with examples?

This touches on a deep question about what is multi-tenancy. I've been thinking about this the past week, and I think the most tangible aspect of multi-tenancy is the (in)ability to "impact" another tenant where impact is quite general.

The most obvious impact is directly operating on another tenant's resources. Permissions/authorization prevent one tenant from performing actions on another tenant.

The next impact is resource utilization, or the noisy neighbor problem. We generally have rate limits to solve this problem, though we only really have rate limits at the topic level. I'm not sure this problem goes much beyond rate limits in the pulsar ecosystem, though.

Perhaps multi-tenancy is just a subpoint under the security concerns section?

[asafm]

@michaeljmarshall

Perhaps multi-tenancy is just a subpoint under the security concerns section?

I believe so.
I've added a commit - take a look?

[michaeljmarshall]

LGTM. Thanks for working on our processes @asafm!

[tisonkun]

Can this section be optional?

[asafm]

I think it forces you to think from the operator perspective. My guess, it will trigger you to change your metrics once you stop and think about it.
In my opinion, it's important.

[tisonkun]

Agree. My comment is that may we add an (optional) tail to indicate this is not always applied. But that can be the default and we just omit it.

[asafm]

Ok to resolve?

[asafm]

@tisonkun

[tisonkun]

Thanks for taking care of this!

+1 for the direction and generally looks good.

Comments inline.

[liangyepianzhou]

Maybe there should be Admin API & CLI

[asafm]

Admin API != CLI, no?

[asafm]

Admin API == REST API?

[asafm]

@liangyepianzhou ping

[liangyepianzhou]

Admin API and CLI (Command Line Interface) are not the same. Admin API typically refers to a set of APIs provided by a service for managing and configuring its features, while CLI is a command line tool that allows users to interact with the service and manage it using text-based commands. However, CLI often uses the Admin API under the hood to communicate with the service and perform the requested operations.

[liangyepianzhou]

I apologize, I overlooked the email notification about this PR.

[liangyepianzhou]

For example:

Admin API Endpoints
`GET /admin/v2/transactions/transaction-keys`: Retrieve a list of all transaction keys in the system.
`GET /admin/v2/transactions/transaction-keys/{transaction_key}`: Retrieve information about a specific transaction key, such as the current epoch and associated transaction IDs.
`DELETE /admin/v2/transactions/transaction-keys/{transaction_key}`: Remove a transaction key from the system, effectively aborting any associated transactions.
CLI Commands
`pulsar-admin transactions list-transaction-keys`: List all transaction keys in the system.
`pulsar-admin transactions get-transaction-key <transaction_key>`: Retrieve information about a specific transaction key, such as the current epoch and associated transaction IDs.
`pulsar-admin transactions delete-transaction-key <transaction_key>`: Remove a transaction key from the system, effectively aborting any associated transactions.

[tisonkun]

Thank you!

Merging...