yangl commented on a change in pull request #10902:
URL: https://github.com/apache/pulsar/pull/10902#discussion_r650315155
##########
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
##########
@@ -272,7 +272,17 @@ public static int getPartitionIndex(String topic) {
int partitionIndex = -1;
if (topic.contains(PARTITIONED_TOPIC_SUFFIX)) {
try {
- partitionIndex =
Integer.parseInt(topic.substring(topic.lastIndexOf('-') + 1));
+ String idx = StringUtils.substringAfterLast(topic,
PARTITIONED_TOPIC_SUFFIX);
+ partitionIndex = Integer.parseInt(idx);
+ if (partitionIndex < 0) {
+ // for the "topic-partition--1"
+ partitionIndex = -1;
+ log.warn("Partition index should >=0!");
Review comment:
The constructor method depends on `private TopicName(String
completeTopicName)` and it depends on this method to set the `partitionIndex`,
and the `isPartitioned()` method depends on the `partitionIndex`.
##########
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
##########
@@ -272,7 +272,17 @@ public static int getPartitionIndex(String topic) {
int partitionIndex = -1;
if (topic.contains(PARTITIONED_TOPIC_SUFFIX)) {
try {
- partitionIndex =
Integer.parseInt(topic.substring(topic.lastIndexOf('-') + 1));
+ String idx = StringUtils.substringAfterLast(topic,
PARTITIONED_TOPIC_SUFFIX);
+ partitionIndex = Integer.parseInt(idx);
+ if (partitionIndex < 0) {
+ // for the "topic-partition--1"
+ partitionIndex = -1;
+ log.warn("Partition index should >=0!");
Review comment:
Hmmm, maybe debug level is more appropriate? What do you think
##########
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
##########
@@ -272,7 +272,17 @@ public static int getPartitionIndex(String topic) {
int partitionIndex = -1;
if (topic.contains(PARTITIONED_TOPIC_SUFFIX)) {
try {
- partitionIndex =
Integer.parseInt(topic.substring(topic.lastIndexOf('-') + 1));
+ String idx = StringUtils.substringAfterLast(topic,
PARTITIONED_TOPIC_SUFFIX);
+ partitionIndex = Integer.parseInt(idx);
+ if (partitionIndex < 0) {
+ // for the "topic-partition--1"
+ partitionIndex = -1;
+ log.warn("Partition index should >=0!");
Review comment:
Thanks for your explanation, but throwing an exception here may not be
particularly appropriate, since this method is called when the topicname is
instantiated to initialize the partionIndex.
I agree that the logs are not printed here because it doesn't matter if it's
a partitioned topic or not. However, output a debug level log here for easy
debugging.
##########
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
##########
@@ -272,7 +272,17 @@ public static int getPartitionIndex(String topic) {
int partitionIndex = -1;
if (topic.contains(PARTITIONED_TOPIC_SUFFIX)) {
try {
- partitionIndex =
Integer.parseInt(topic.substring(topic.lastIndexOf('-') + 1));
+ String idx = StringUtils.substringAfterLast(topic,
PARTITIONED_TOPIC_SUFFIX);
+ partitionIndex = Integer.parseInt(idx);
+ if (partitionIndex < 0) {
+ // for the "topic-partition--1"
+ partitionIndex = -1;
+ log.warn("Partition index should >=0!");
Review comment:
OK, deleted.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
