You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/04/25 04:09:26 UTC
[GitHub] [pulsar] wuzhanpeng opened a new pull request, #15301: enhance validation when updating transaction topic
wuzhanpeng opened a new pull request, #15301:
URL: https://github.com/apache/pulsar/pull/15301
### Motivation
Currently executing the command `bin/pulsar initialize-transaction-coordinator-metadata` will directly update the number of partitions of `persistent://pulsar/system/transaction_coordinator_assign`.
When accidentally misoperation, the number of partitions is directly modified, and there is no way to roll back (the number of partitions cannot be reduced).
### Modifications
Simplely add an argument to determine whether forced update is allowed.
### Verifying this change
This change added tests and can be verified as follows:
*(example:)*
```shell
bin/pulsar initialize-transaction-coordinator-metadata -cs 127.0.0.1:2181 -c standalone -c 1
bin/pulsar initialize-transaction-coordinator-metadata -cs 127.0.0.1:2181 -c standalone -c 2
# Check for changes of partitions
bin/pulsar-admin topics get-partitioned-topic-metadata persistent://pulsar/system/transaction_coordinator_assign
```
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- **The admin cli options: (yes)**
- Anything that affects deployment: (no)
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [x] `no-need-doc`
I think command description is sufficient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15301:
URL: https://github.com/apache/pulsar/pull/15301#discussion_r862566239
##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java:
##########
@@ -370,7 +373,7 @@ static void createPartitionedTopic(MetadataStore configStore, TopicName topicNam
PartitionedTopicMetadata existsMeta = getResult.get();
// Only update metadata if the partitions should be modified
- if (existsMeta.partitions < numPartitions) {
+ if (existsMeta.partitions < numPartitions && force) {
Review Comment:
My idea, wait for others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15301:
URL: https://github.com/apache/pulsar/pull/15301#discussion_r858230344
##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java:
##########
@@ -370,7 +373,7 @@ static void createPartitionedTopic(MetadataStore configStore, TopicName topicNam
PartitionedTopicMetadata existsMeta = getResult.get();
// Only update metadata if the partitions should be modified
- if (existsMeta.partitions < numPartitions) {
+ if (existsMeta.partitions < numPartitions && force) {
Review Comment:
Seems `force` should not check the partition numbers, update directly...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #15301:
URL: https://github.com/apache/pulsar/pull/15301#issuecomment-1229148781
> It's not able to reduce the partitions of transaction coordinators, it will lost data of transaction log.
@wuzhanpeng PTAL, and confirm if we can close this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] liangyepianzhou commented on pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on PR #15301:
URL: https://github.com/apache/pulsar/pull/15301#issuecomment-1240140655
The goal of the command of `bin/pulsar initialize-transaction-coordinator-metadata` is to create the partition topic <ersistent://pulsar/system/transaction_coordinator_assign> if absent.
That is not an optional item, so the motivation of this PR is confused.
I will close this PR and you can reopen it when you have more valid feedback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] liangyepianzhou closed pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
liangyepianzhou closed pull request #15301: [improve][tx] enhance validation when updating transaction topic
URL: https://github.com/apache/pulsar/pull/15301
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15301:
URL: https://github.com/apache/pulsar/pull/15301#issuecomment-1144344043
The pr had no activity for 30 days, mark with Stale label.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] liangyepianzhou commented on a diff in pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #15301:
URL: https://github.com/apache/pulsar/pull/15301#discussion_r893082267
##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java:
##########
@@ -370,7 +373,7 @@ static void createPartitionedTopic(MetadataStore configStore, TopicName topicNam
PartitionedTopicMetadata existsMeta = getResult.get();
// Only update metadata if the partitions should be modified
- if (existsMeta.partitions < numPartitions) {
+ if (existsMeta.partitions < numPartitions && force) {
Review Comment:
We do not support reducing the number of TC.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wuzhanpeng commented on a diff in pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
wuzhanpeng commented on code in PR #15301:
URL: https://github.com/apache/pulsar/pull/15301#discussion_r858327786
##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java:
##########
@@ -370,7 +373,7 @@ static void createPartitionedTopic(MetadataStore configStore, TopicName topicNam
PartitionedTopicMetadata existsMeta = getResult.get();
// Only update metadata if the partitions should be modified
- if (existsMeta.partitions < numPartitions) {
+ if (existsMeta.partitions < numPartitions && force) {
Review Comment:
I don't quite understand what you mean 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15301: [improve][tx] enhance validation when updating transaction topic
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15301:
URL: https://github.com/apache/pulsar/pull/15301#discussion_r862566175
##########
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java:
##########
@@ -370,7 +373,7 @@ static void createPartitionedTopic(MetadataStore configStore, TopicName topicNam
PartitionedTopicMetadata existsMeta = getResult.get();
// Only update metadata if the partitions should be modified
- if (existsMeta.partitions < numPartitions) {
+ if (existsMeta.partitions < numPartitions && force) {
Review Comment:
if (force) {
....
} else if (existsMeta.partitions < numPartitions) {
...
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org