Skip to content
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.

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

Ci 2.11.1 rc2 #13

Closed
wants to merge 362 commits into from
Closed

Ci 2.11.1 rc2 #13

wants to merge 362 commits into from

Conversation

Jason918
Copy link
Owner

Fixes #xyz

Master Issue: #xyz

PIP: #xyz

Motivation

Modifications

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

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

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

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

Matching PR in forked repository

PR in forked repository:

Nicklee007 and others added 30 commits December 8, 2022 10:07
…yListener when use ByteBufAllocator.DEFAULT.heapBuffer in PrometheusMetricsGeneratorUtils (apache#18747)

Co-authored-by: nicklixinyang <nicklixinyang@didiglobal.com>
…pache#18603)

This PR is a supplement to apache#18369.
- `AbstractTopic.isSameAddressProducersExceeded()`
- `AbstractBaseDispatcher.isConsumersExceededOnSubscription()`
…e by force (apache#18307) (apache#18826)

### Motivation
Cherry-pick (apache#18307) to release 2.11.1.
### Modifications

Cherry-pick (apache#18307) to release 2.11.1.
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
apache#18943)

when `MLPendingAckStoreProvider` init PendingAckStore gets the ManagedLedger config throw exception, we don't handle the exception. and the `pendingAckStoreFeture` can't be complete, topic unload will use this future to close the pendingAck.
https://github.com/apache/pulsar/blob/3011946a5c3b64ed7c08b6bfb1f6492f8aaaca9c/pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/pendingack/impl/MLPendingAckStoreProvider.java#L114-L115
when getting managedledger config to fail, `pendingAckStoreFeture` will `completeExceptionally()`;

when pendingAckStore init fail, close pendingAckHandle success directly

mock get managedLeger config throw exception, then unload can success

(cherry picked from commit 1d9956c)
…xt (apache#18969)

### Motivation
Since PR apache#18833 can not cherry-pick to `branch-2.11`, create a separate PR.


#### Context for Transaction Buffer
- If turn on `transactionCoordinatorEnabled`,  then `TransactionBuffer` will be initialized when a user topic create.
- The `TransactionBuffer` reads the aborted logs of transactions from topic `__transaction_buffer_snapshot`  -- this process is called `recovery`.
- During recovery, the reading from that snapshot ledger is done via a `Reader`; the reader works like this:
```
while (reader.hasMessageAvailable()){
    reader.readNext();
}
``` 

#### Context for Compaction
- After [pip-14](https://github.com/apache/pulsar/wiki/PIP-14:-Topic-compaction), the consumer that enabled feature read-compacted will read messages from the compacted topic instead of the original topic if the task-compaction is done, and read messages from the original topic if task-compaction is not done.
- If the data of the last message with key k sent to a topic is null, the compactor will mark all messages for that key as deleted.

#### Issue
There is a race condition: after executing `reader.hasMessageAvailable`,  the following messages have been deleted by compaction-task, so read next will be blocked because there have no messages to read.

----

### Modifications

- If hits this issue, do recover again.

----

#### Why not just let the client try to load the topic again to retry the recover?
If the topic load is failed, the client will receive an error response. This is a behavior that we can handle, so should not be perceived by the users.
poorbarcode and others added 27 commits March 22, 2023 17:46
…pic fails due to enabled geo-replication (apache#19879)

Motivation: As expected, If geo-replication is enabled, a topic cannot be deleted. However deleting that topic returns a 500, and no further info.
Modifications: Make response code to 400 instead of 500 when delete topic fails due to enabled geo-replication
(cherry picked from commit a903733)
Signed-off-by: nodece <nodeces@gmail.com>
(cherry picked from commit 32ad906)
…-client-all (apache#19937)

(cherry picked from commit d14e43e)

Ref - apache#19484

Signed-off-by: tison <wander4096@gmail.com>
…ioned-topic stat (apache#19942)

### Motivation

Pulsar will merge the variable `PartitionedTopicStatsImpl.replication[x].connected` by the way below when we call `pulsar-admin topics partitioned-stats`

``` java
this.connected = this.connected & other.connected
```

But the variable `connected` of `PartitionedTopicStatsImpl.replication` is initialized `false`, so the expression `this.connected & other.connected` will always be `false`.

Then we will always get the value `false` if we call `pulsar-admin topics partitioned-stats`.

### Modifications

make the variable `` of `PartitionedTopicStatsImpl` is initialized `true`

(cherry picked from commit 9fc0b5e)
…9951)

Motivation
Kafka's schema has "Optional" flag that used there to validate data/allow nulls.
Pulsar's schema does not have such info which makes conversion to kafka schema lossy.

Modifications
Added a config parameter that lets one force primitive schemas into optional ones.
KV schema is always optional.

Default is false, to match existing behavior.

(cherry picked from commit 55523ac)
…eleted (apache#19825)

When deleting the zk node of the cursor, if the exception `MetadataStoreException.NotFoundException`  occurs, the deletion is considered successful.
…#19989)

### Motivation

In apache#19455, I added a requirement that only the proxy role could supply an original principal. That check is only supposed to apply when the broker has authorization enabled. However, in one case, that was not the case. This PR does a check and returns early when authorization is not enabled in the broker.

See apache#19830 (comment) for additional motivation.

### Modifications

* Update the `PulsarWebResource#validateSuperUserAccessAsync` to only validate when authentication and authorization are enabled in the configuration.

### Verifying this change

This is a trivial change. It'd be good to add tests, but I didn't include them here because this is a somewhat urgent fix. There was one test that broke because of this change, so there is at least some existing coverage.

### Documentation

- [x] `doc-not-needed`

### Matching PR in forked repository

PR in forked repository: michaeljmarshall#39

(cherry picked from commit 1a6c28d)
This PR builds on apache#19467. When we modify/abort transactions, we need to make sure that authorization is checked for both the proxy and the client.

* Add a second authorization check when `originalPrincipal` is set in the `ServerCnx`.
* Fix a bug where we were not doing a deep copy of the `SubscriptionsList` object. (Tests caught this bug!)

Added a new test to cover some of the changes.

This is an internal change.

- [x] `doc-not-needed`

PR in forked repository: michaeljmarshall#38

(cherry picked from commit f76beda)
…pache#19975)

The current function worker interfaces introduced by apache#8560 are hard to extend. It is better to pass an object that we can add fields to as needed.

* Introduce a new `Authentication` class to wrap all of the relevant authentication "data". This class is somewhat unfortunate in that we already have many classes that hold authentication information. However, my goal with this class is to wrap all of the relevant auth info. This class is only currently used in the Function Worker API, but I plan to add it to the rest of the Admin HTTP endpoints.
* Deprecate all of the methods that are no longer needed. Add a default implementation to call the new methods that supersede those methods.
* Add `proxyRoles` setting to function worker.

There are tests to cover the changes.

This PR changes several interfaces in completely backwards compatible ways. It's possible we'll want to improve the abstraction for the `Authentication` class by making it an interface.

- [x] `doc`

This change has Javadoc updates to document the changes.

PR in forked repository: michaeljmarshall#37

(cherry picked from commit 55acbe6)
…ache#20046)

In apache#19975, we introduced a wrapper for all authentication parameters. This PR adds that wrapper to the Rest Producer.

* Use `AuthenticationParameters` to simplify parameter management in Rest Producer.
* Add method to the `AuthorizationService` that takes the `AuthenticationParameters`.
* Update annotations on Rest Producer to indicate that a 401 is an expected response.

This change is covered by the `TopicsAuthTest`.

- [x] `doc-not-needed`

This is an internal change that does not need to be documented.

PR in forked repository: skipping PR since the relevant tests pass locally

(cherry picked from commit 7990948)
…ls (apache#20031)

### Motivation
After apache#15868, we allow `PULSAR_MEM` & `PULSAR_GC` to be overridden in `pulsar_tool_env.sh`.

Many users set `-Xms` to `2G` or larger in `PULSAR_MEM`, this will make the tools(such as `pulsar-admin`) cost a lot of memory, and if users execute `pulsar-admin` or another tool on the machine where the Broker is deployed, the current device will not have enough memory to allocate, resulting in a broker crash.

### Modifications

When `PULSAR_MEM` is overridden  in `pulsar_tool_env.sh`, delete parameter `-Xms`

(cherry picked from commit 4f503fd)
@Jason918 Jason918 closed this Apr 15, 2023
@Jason918 Jason918 reopened this Apr 15, 2023
@Jason918 Jason918 closed this Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.