-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
When the Replicator is enabled, no managedLedger is created when updating the number of partitions #10910
Conversation
…l not create managedLedger
Awaitility.await().untilAsserted(() -> assertNotNull(admin2.topics().getPartitionedTopicList(namespace))); | ||
Awaitility.await().untilAsserted(() -> assertEquals( | ||
admin2.topics().getPartitionedTopicList(namespace).get(0), persistentTopicName)); | ||
assertEquals(admin1.topics().getList(namespace).size(), 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@315157973 Could you please also add a check for checking we have 3 partitions in the cluster-2(using admin-1)
assertEquals(admin2.topics().getList(namespace).size(), 3);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok i will add this test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @codelipenghui
admin.topics().getList
reads the data under the /managed-ledgers node, which belongs to localZK, so the remote cluster cannot perceive it. This PR mainly fixes the remote replication problem of updatePartitions in issue#10673.
Now the CreatePartitionTopic interface does not have geo-related logic. If we want to add it, we need to add parameters to avoid circular replication. I will open another PR to support replication in CreatePartitionTopic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after addressing @codelipenghui 's comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ting the number of partitions (#10910) Fixes #10673 When updating the number of partitions, we need to update the data in two places in zk: ``` /admin/partitioned-topics /managed-ledgers/ ``` Now we only update the number of partitions in `/admin/partitioned-topics`, so if we do not create a Producer or Consumer, the data obtained in another cluster will be incorrect 1)Try to create managedLedger when updating the number of partitions 2)Ensure that the number of partitions in `/admin/partitioned-topics` is updated every time (cherry picked from commit 202da11)
…ting the number of partitions (apache#10910) Fixes apache#10673 ### Motivation When updating the number of partitions, we need to update the data in two places in zk: ``` /admin/partitioned-topics /managed-ledgers/ ``` Now we only update the number of partitions in `/admin/partitioned-topics`, so if we do not create a Producer or Consumer, the data obtained in another cluster will be incorrect ### Modifications 1)Try to create managedLedger when updating the number of partitions 2)Ensure that the number of partitions in `/admin/partitioned-topics` is updated every time
…ting the number of partitions (#10910) Fixes #10673 ### Motivation When updating the number of partitions, we need to update the data in two places in zk: ``` /admin/partitioned-topics /managed-ledgers/ ``` Now we only update the number of partitions in `/admin/partitioned-topics`, so if we do not create a Producer or Consumer, the data obtained in another cluster will be incorrect ### Modifications 1)Try to create managedLedger when updating the number of partitions 2)Ensure that the number of partitions in `/admin/partitioned-topics` is updated every time (cherry picked from commit 202da11)
…ting the number of partitions (apache#10910) Fixes apache#10673 ### Motivation When updating the number of partitions, we need to update the data in two places in zk: ``` /admin/partitioned-topics /managed-ledgers/ ``` Now we only update the number of partitions in `/admin/partitioned-topics`, so if we do not create a Producer or Consumer, the data obtained in another cluster will be incorrect ### Modifications 1)Try to create managedLedger when updating the number of partitions 2)Ensure that the number of partitions in `/admin/partitioned-topics` is updated every time
Fixes #10673
Motivation
When updating the number of partitions, we need to update the data in two places in zk:
Now we only update the number of partitions in
/admin/partitioned-topics
, so if we do not create a Producer or Consumer, the data obtained in another cluster will be incorrectModifications
1)Try to create managedLedger when updating the number of partitions
2)Ensure that the number of partitions in
/admin/partitioned-topics
is updated every time