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 2021/11/11 13:47:25 UTC

[GitHub] [pulsar] mingyifei opened a new pull request #12756: [Issue ] Fix inaccurate naming and simplify interceptor settings.

mingyifei opened a new pull request #12756:
URL: https://github.com/apache/pulsar/pull/12756


   ### Modifications
   1.Method naming is not precise enough. 
   2.BrokerEntryMetadataInterceptor does not need to be traversed and added, because it is already a top-level interface
   
   ### Verifying this change
   No need to write documentation and unit tests.


-- 
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] mingyifei commented on pull request #12756: [Issue ] Fix inaccurate naming and simplify interceptor settings.

Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12756:
URL: https://github.com/apache/pulsar/pull/12756#issuecomment-966316781


   > @mingyifei:Thanks for your contribution. For this PR, do we need to update docs? (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
   
   unnecessary


-- 
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] codelipenghui commented on a change in pull request #12756: [Normative Issues ] Fix inaccurate naming and simplify interceptor settings.

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #12756:
URL: https://github.com/apache/pulsar/pull/12756#discussion_r747551168



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1289,18 +1289,9 @@ private void createPersistentTopic(final String topic, boolean createIfMissing,
                 : CompletableFuture.completedFuture(null);
 
         maxTopicsCheck.thenCompose(__ -> getManagedLedgerConfig(topicName)).thenAccept(managedLedgerConfig -> {
-            if (isBrokerEntryMetadataEnabled()) {
+            if (hasBrokerEntryMetadataInterceptor()) {
                 // init managedLedger interceptor
-                Set<BrokerEntryMetadataInterceptor> interceptors = new HashSet<>();
-                for (BrokerEntryMetadataInterceptor interceptor : brokerEntryMetadataInterceptors) {
-                    // add individual AppendOffsetMetadataInterceptor for each topic
-                    if (interceptor instanceof AppendIndexMetadataInterceptor) {
-                        interceptors.add(new AppendIndexMetadataInterceptor());

Review comment:
       We should use a new `AppendIndexMetadataInterceptor` not a shared `AppendIndexMetadataInterceptor` for each topic.




-- 
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] mingyifei commented on pull request #12756: [Issue ] Fix inaccurate naming and simplify interceptor settings.

Posted by GitBox <gi...@apache.org>.
mingyifei commented on pull request #12756:
URL: https://github.com/apache/pulsar/pull/12756#issuecomment-966318398


   @Jason918 


-- 
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 #12756: [Issue ] Fix inaccurate naming and simplify interceptor settings.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12756:
URL: https://github.com/apache/pulsar/pull/12756#issuecomment-966315762


   @mingyifei:Thanks for your contribution. For this PR, do we need to update docs?
   (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)


-- 
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] mingyifei closed pull request #12756: [Normative Issues ] Fix inaccurate naming and simplify interceptor settings.

Posted by GitBox <gi...@apache.org>.
mingyifei closed pull request #12756:
URL: https://github.com/apache/pulsar/pull/12756


   


-- 
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] mingyifei commented on a change in pull request #12756: [Normative Issues ] Fix inaccurate naming and simplify interceptor settings.

Posted by GitBox <gi...@apache.org>.
mingyifei commented on a change in pull request #12756:
URL: https://github.com/apache/pulsar/pull/12756#discussion_r747574588



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -1289,18 +1289,9 @@ private void createPersistentTopic(final String topic, boolean createIfMissing,
                 : CompletableFuture.completedFuture(null);
 
         maxTopicsCheck.thenCompose(__ -> getManagedLedgerConfig(topicName)).thenAccept(managedLedgerConfig -> {
-            if (isBrokerEntryMetadataEnabled()) {
+            if (hasBrokerEntryMetadataInterceptor()) {
                 // init managedLedger interceptor
-                Set<BrokerEntryMetadataInterceptor> interceptors = new HashSet<>();
-                for (BrokerEntryMetadataInterceptor interceptor : brokerEntryMetadataInterceptors) {
-                    // add individual AppendOffsetMetadataInterceptor for each topic
-                    if (interceptor instanceof AppendIndexMetadataInterceptor) {
-                        interceptors.add(new AppendIndexMetadataInterceptor());

Review comment:
       ok.




-- 
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