This repository was archived by the owner on Jan 24, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 142
[Tests] Configure a different metadata namespace with the default namespace #1274
Merged
BewareMyPower
merged 2 commits into
streamnative:master
from
BewareMyPower:bewaremypower/separate-metadata-ns
May 7, 2022
Merged
[Tests] Configure a different metadata namespace with the default namespace #1274
BewareMyPower
merged 2 commits into
streamnative:master
from
BewareMyPower:bewaremypower/separate-metadata-ns
May 7, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…espace ### Motivation Currently in KoP tests, the metadata namespace configured by `kafkaMetadataNamespace` is "default", which is same with the default namespace of Pulsar and KoP (`kafkaNamespace`). https://github.com/streamnative/kop/blob/64001937c92d80ed4f652becd8d6b9768918b875/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java#L176-L177 It's a terrible config because some topic policies might be changed with the metadata namespace and the offset topic is created during starting. We should avoid the namespace for normal topics being affected by these implicit operations. ### Modifications Remove the configs for `kafkaMetadataTenant` and `kafkaMetadataNamespace`, then fix the following tests: - `MultiLedgerTest`: set the infinite retention to avoid rollover ledgers being removed automatically. - `KafkaTopicConsumerManagerTest`: trigger the topic lookup before creating `PersistentTopic` objects in `BrokerService` so that broker can acquire the ownership of namespace bundles. - `CacheInvalidatorTest`: unload the metadata namespace instead of the default namespace.
65ba3a1 to
b52a489
Compare
3 tasks
Demogorgon314
approved these changes
May 7, 2022
wenbingshen
reviewed
May 7, 2022
tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java
Show resolved
Hide resolved
BewareMyPower
added a commit
that referenced
this pull request
May 7, 2022
…espace (#1274) ### Motivation Currently in KoP tests, the metadata namespace configured by `kafkaMetadataNamespace` is "default", which is same with the default namespace of Pulsar and KoP (`kafkaNamespace`). https://github.com/streamnative/kop/blob/64001937c92d80ed4f652becd8d6b9768918b875/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java#L176-L177 It's a terrible config because some topic policies might be changed with the metadata namespace and the offset topic is created during starting. We should avoid the namespace for normal topics being affected by these implicit operations. ### Modifications Remove the configs for `kafkaMetadataTenant` and `kafkaMetadataNamespace`, then fix the following tests: - `MultiLedgerTest`: set the infinite retention to avoid rollover ledgers being removed automatically. - `KafkaTopicConsumerManagerTest`: trigger the topic lookup before creating `PersistentTopic` objects in `BrokerService` so that broker can acquire the ownership of namespace bundles. - `CacheInvalidatorTest`: unload the metadata namespace instead of the default namespace. (cherry picked from commit 9e402f8)
BewareMyPower
added a commit
that referenced
this pull request
May 7, 2022
…espace (#1274) ### Motivation Currently in KoP tests, the metadata namespace configured by `kafkaMetadataNamespace` is "default", which is same with the default namespace of Pulsar and KoP (`kafkaNamespace`). https://github.com/streamnative/kop/blob/64001937c92d80ed4f652becd8d6b9768918b875/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java#L176-L177 It's a terrible config because some topic policies might be changed with the metadata namespace and the offset topic is created during starting. We should avoid the namespace for normal topics being affected by these implicit operations. ### Modifications Remove the configs for `kafkaMetadataTenant` and `kafkaMetadataNamespace`, then fix the following tests: - `MultiLedgerTest`: set the infinite retention to avoid rollover ledgers being removed automatically. - `KafkaTopicConsumerManagerTest`: trigger the topic lookup before creating `PersistentTopic` objects in `BrokerService` so that broker can acquire the ownership of namespace bundles. - `CacheInvalidatorTest`: unload the metadata namespace instead of the default namespace. (cherry picked from commit 9e402f8)
BewareMyPower
added a commit
that referenced
this pull request
May 7, 2022
…espace (#1274) ### Motivation Currently in KoP tests, the metadata namespace configured by `kafkaMetadataNamespace` is "default", which is same with the default namespace of Pulsar and KoP (`kafkaNamespace`). https://github.com/streamnative/kop/blob/64001937c92d80ed4f652becd8d6b9768918b875/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java#L176-L177 It's a terrible config because some topic policies might be changed with the metadata namespace and the offset topic is created during starting. We should avoid the namespace for normal topics being affected by these implicit operations. ### Modifications Remove the configs for `kafkaMetadataTenant` and `kafkaMetadataNamespace`, then fix the following tests: - `MultiLedgerTest`: set the infinite retention to avoid rollover ledgers being removed automatically. - `KafkaTopicConsumerManagerTest`: trigger the topic lookup before creating `PersistentTopic` objects in `BrokerService` so that broker can acquire the ownership of namespace bundles. - `CacheInvalidatorTest`: unload the metadata namespace instead of the default namespace. (cherry picked from commit 9e402f8) Resolve the conflicts by printing the KOP_ADDRESS_CACHE, which was removed in branch-2.9.x and higher since #864.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
area/test
cherry-picked/branch-2.8.3
cherry-picked/branch-2.9.2
cherry-picked/branch-2.10.0
no-need-doc
This pr does not need any document
release/2.8.3
release/2.9.2
release/2.10.0
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Currently in KoP tests, the metadata namespace configured by
kafkaMetadataNamespaceis "default", which is same with the defaultnamespace of Pulsar and KoP (
kafkaNamespace).kop/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java
Lines 176 to 177 in 6400193
It's a terrible config because some topic policies might be changed with
the metadata namespace and the offset topic is created during starting.
We should avoid the namespace for normal topics being affected by these
implicit operations.
Modifications
Remove the configs for
kafkaMetadataTenantandkafkaMetadataNamespace, then fix the following tests:MultiLedgerTest: set the infinite retention to avoid rolloverledgers being removed automatically.
KafkaTopicConsumerManagerTest: trigger the topic lookup beforecreating
PersistentTopicobjects inBrokerServiceso that brokercan acquire the ownership of namespace bundles.
CacheInvalidatorTest: unload the metadata namespace instead of thedefault namespace.
Documentation
Check the box below.
Need to update docs?
doc-required(If you need help on updating docs, create a doc issue)
no-need-doc(Please explain why)
doc(If this PR contains doc changes)