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

Penghui/verify 2.11 #21

Closed
wants to merge 429 commits into from
Closed

Penghui/verify 2.11 #21

wants to merge 429 commits into from

Conversation

codelipenghui
Copy link
Owner

No description provided.

@github-actions
Copy link

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

@github-actions github-actions bot added the Stale label Jan 25, 2023
merlimat and others added 28 commits March 15, 2023 18:37
…ct if schema future not complete (apache#19242)

(cherry picked from commit 3ba6fa8)
apache#17423) (apache#19834)

Co-authored-by: congbo <39078850+congbobo184@users.noreply.github.com>
When apache#19519 was
cherry-picked to branch-2.11, it did not implement the
authenticate method in the MockMutableAuthenticationState,
which led to several test failures in the ServerCnxTest class.
This commit fixes those tests.

Note that the issue is only in the test code.
The ServerCnxTest class is run as part of the flaky tests,
so apache#19830 was merged
without noticing that the changes didn't actually pass
tests. This commit fixes the test by changing the assertions
to ensure that "correct" combinations of roles and original
principals are verified.
### Motivation

Upgrade bookkeeper to 4.15.4.
apache#19861)

Co-authored-by: Andrey Yegorov <andrey.yegorov@datastax.com>
(cherry picked from commit 03f8b80)
…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)
poorbarcode and others added 28 commits September 14, 2023 10:13
…n per broker (apache#21144)

Motivation: Pulsar has two mechanisms to guarantee that a producer connects to the broker multiple times the result is still correct.

- In a connection, the second connection waits for the first connection to complete.
- In a topic, the second connection will override the previous one.

However, if a producer can use different connections to connect to the broker, these two mechanisms will not work.

When the config `connectionsPerBroker` of `PulsarClient` is larger than `1`, a producer could use more than one connection, leading to the error above. You can reproduce this issue by the test `testSelectConnectionForSameProducer.`

Modifications: Make the same producer/consumer usage the same connection
(cherry picked from commit f2b9a3f)
apache#21155)

The client assumed the connection was inactive, but the Broker assumed the connection was fine. The Client tried to  use a new connection to reconnect a producer, then got an error `Producer with name 'st-0-5' is already connected to topic`.

- In a connection, the second connection waits for the first connection to complete\. But there is a bug that causes this mechanism to fail\.
- If a producer uses a default name, the second registration will override the first one. But it can not override the first one if it uses a specified producer name\. I think this mechanism is to prevent a client from creating two producers with the same name. However, method `Producer.isSuccessorTo` has checked the `producer-id`, and the `producer-id` of multiple producers created by the same client are different. So this mechanism can be deleted.

- For `issue 1`: If a producer with the same name tries to use a new connection, async checks the old connection is available. The producers related to the connection that is not available are automatically cleaned up.

- For `issue 2`:
  -  Fix the bug that causes a complete producer future will be removed from `ServerCnx`.
  - Remove the mechanism that prevents a producer with a specified name from overriding the previous producer.

(cherry picked from commit bda16b6)
… many requests (apache#21216)

Motivation: The Pulsar client will close the socket if it receives a `ServiceNotReady` error when doing a lookup. The Broker will respond to the client with a `TooManyRequests` error if there are too many lookup requests in progress, but the Pulsar Proxy responds to the client with a `ServiceNotReady` error in the same scenario.

Modifications: Make Pulsar Proxy respond to the client with a `TooManyRequests` error if there are too many lookup requests in progress.
(cherry picked from commit d6c3fa4)
…1232)

Motivation: Multiple consumers and producers can be maintained by the same Pulsar Client. In some cases, multiple consumers or producers might attempt to connect to the same topic. To optimize the process, it is recommended to perform the topic lookup only once for each topic.

Modifications:
- Merge lookup requests for the same topic.
- Merge get partitioned metadata request for the same partitioned topic.

(cherry picked from commit be4ab66)
…er id when switch ledger (apache#21201)

### Modifications
- Print a warning log if the SSL handshake error
- Print ledger ID when switching ledger

(cherry picked from commit 8485d68)
…ing queue is not empty (apache#21259)

Reproduce steps:
- Create a reader.
- Reader pulls messages into `incoming queue`, do not call `reader.readNext` now.
- Trim ledger task will delete the ledgers, then there is no in the topic.
- Now, you can get messages if you call `reader.readNext`, but the method `reader.hasMessageAvailable` return `false`

Note: the similar issue of `MultiTopicsConsumerImpl` has been fixed by apache#13332, current PR only trying to fix the issue of `ConsumerImpl`.

Make `reader.hasMessageAvailable` return `true` when `incoming queue` is not empty.

(cherry picked from commit 6d82b09)
…ndle unloading or metadata ex (apache#21211)

### Motivation

**Background**: The Pulsar client will close the socket if it receives a ServiceNotReady error when doing a lookup.
Closing the socket causes the other consumer or producer to reconnect and does not make the lookup more efficient.

There are two cases that should be improved:
- If the broker gets a metadata read/write error, the broker responds with a `ServiceNotReady` error, but it should respond with a `MetadataError`
- If the topic is unloading, the broker responds with a `ServiceNotReady` error.

### Modifications
- Respond to the client with a `MetadataError` if the broker gets a metadata read/write error.
- Respond to the client with a `MetadataError` if the topic is unloading

(cherry picked from commit 09a1720)
… BookkeeperInternalCallbacks.Processor (apache#21315)

### Motivation
The Auditor will start AuditorCheckAllLedgersTask to checkAllLedgers. It will iterate each ledger using `ledgerManager.asyncProcessLedgers`, then callback checkLedgersProcessor in the metadata store thread.

https://github.com/apache/bookkeeper/blob/e8da8eb6cb7c8ef52336a51d4f7348ae07986c26/bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AuditorCheckAllLedgersTask.java#L158-L248

In the checkLedgersProcessor, it will invoke `ledgerUnderreplicationManager.isLedgerReplicationEnabled()`, it's a metadata store sync operation, so deadlock.

### Modifications
Using scheduler to trigger the processor.
(cherry picked from commit e76a86e)

# Conflicts:
#	pulsar-common/src/test/java/org/apache/pulsar/common/nar/NarUnpackerTest.java
After trimming ledgers, the variable `lastConfirmedEntry` of the managed ledger might rely on a deleted ledger(the latest ledger which contains data).

There is a bug that makes pulsar allow users to set the start read position to an unexisting ledger or a deleted ledger when creating a subscription. This makes the `backlog` and `markDeletedPosition` wrong.

Fix the bug.

(cherry picked from commit 4ee5cd7)
…gers (apache#21250)

### Background
- But after trimming ledgers, `ml.lastConfirmedPosition` relies on a deleted ledger when the current ledger of ML is empty.
- Cursor prevents setting `markDeletedPosition` to a value larger than `ml.lastConfirmedPosition`, but there are no entries to read<sup>[1]</sup>.
- The code description in the method `advanceCursors` said: do not make `cursor.markDeletedPosition` larger than `ml.lastConfirmedPosition`<sup>[2]</sup>

### Issue
If there is no durable cursor, the `markDeletedPosition` might be set to `{current_ledger, -1}`, and `async mark delete` will be prevented by the `rule-2` above. So he `backlog`, `readPosition`, and `markDeletedPosition` of the cursor will be in an incorrect position after trimming the ledger. You can reproduce it by the test `testTrimLedgerIfNoDurableCursor`

### Modifications
Do not make `cursor.markDeletedPosition` larger than `ml.lastConfirmedPosition` when advancing non-durable cursors.

(cherry picked from commit ca77982)
@codelipenghui codelipenghui deleted the penghui/verify_2.11 branch March 26, 2024 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.