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

Correct TopicName#getPartitionIndex implementation. #10902

Merged
merged 4 commits into from Jun 18, 2021

Conversation

yangl
Copy link
Contributor

@yangl yangl commented Jun 11, 2021

Motivation

Current partitioned topic check logic is not rigorous enough, such as the mytopic-partition--1 this should not partitioned topic.
For details of the discussion see http://mail-archives.apache.org/mod_mbox/pulsar-dev/202106.mbox/raw/%3CCACcefgerbLU88rKqPNCTgBSCWn_FJ5MB_E7oa1tKfxTsMAfpfg%40mail.gmail.com%3E

Modifications

  1. change the getPartitionIndex logic.

@yangl
Copy link
Contributor Author

yangl commented Jun 11, 2021

/pulsarbot run-failure-checks

Signed-off-by: YANGLiiN <ielin@qq.com>
@yangl
Copy link
Contributor Author

yangl commented Jun 14, 2021

/pulsarbot run-failure-checks

@BewareMyPower
Copy link
Contributor

BewareMyPower commented Jun 14, 2021

Could you please also remove the unused log field in TopicName? Just apply following diff.

diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
index bf6db5ff147..2cb9f23153b 100644
--- a/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
+++ b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
@@ -29,16 +29,12 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.pulsar.common.util.Codec;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Encapsulate the parsing of the completeTopicName name.
  */
 public class TopicName implements ServiceUnitId {
 
-    private static final Logger log = LoggerFactory.getLogger(TopicName.class);
-
     public static final String PUBLIC_TENANT = "public";
     public static final String DEFAULT_NAMESPACE = "default";
 

@yangl
Copy link
Contributor Author

yangl commented Jun 15, 2021

Could you please also remove the unused log field in TopicName? Just apply following diff.

diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
index bf6db5ff147..2cb9f23153b 100644
--- a/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
+++ b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/TopicName.java
@@ -29,16 +29,12 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.pulsar.common.util.Codec;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Encapsulate the parsing of the completeTopicName name.
  */
 public class TopicName implements ServiceUnitId {
 
-    private static final Logger log = LoggerFactory.getLogger(TopicName.class);
-
     public static final String PUBLIC_TENANT = "public";
     public static final String DEFAULT_NAMESPACE = "default";
 

Done.

@sijie sijie added this to the 2.9.0 milestone Jun 18, 2021
@BewareMyPower BewareMyPower merged commit 0ead803 into apache:master Jun 18, 2021
@yangl yangl deleted the partition_index branch June 20, 2021 08:37
yangl added a commit to yangl/pulsar that referenced this pull request Jun 23, 2021
### Motivation

Current partitioned topic check logic is not rigorous enough, such as the `mytopic-partition--1` this should not partitioned topic. 
For details of the discussion see http://mail-archives.apache.org/mod_mbox/pulsar-dev/202106.mbox/raw/%3CCACcefgerbLU88rKqPNCTgBSCWn_FJ5MB_E7oa1tKfxTsMAfpfg%40mail.gmail.com%3E

### Modifications
1. change the `getPartitionIndex` logic.
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
### Motivation

Current partitioned topic check logic is not rigorous enough, such as the `mytopic-partition--1` this should not partitioned topic. 
For details of the discussion see http://mail-archives.apache.org/mod_mbox/pulsar-dev/202106.mbox/raw/%3CCACcefgerbLU88rKqPNCTgBSCWn_FJ5MB_E7oa1tKfxTsMAfpfg%40mail.gmail.com%3E

### Modifications
1. change the `getPartitionIndex` logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants