[GitHub] [pulsar] aloyszhang commented on a change in pull request #9091: fix BKIncorrectParameterException

[GitBox <gi...@apache.org>]

aloyszhang commented on a change in pull request #9091:
URL: https://github.com/apache/pulsar/pull/9091#discussion_r550010559



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/intercept/ManagedLedgerInterceptorImpl.java
##########
@@ -83,9 +83,9 @@ public void onManagedLedgerPropertiesInitialize(Map<String, String> propertiesMa
     public void onManagedLedgerLastLedgerInitialize(String name, LedgerHandle lh) {
         try {
             for (BrokerEntryMetadataInterceptor interceptor : brokerEntryMetadataInterceptors) {
-                if (interceptor instanceof AppendIndexMetadataInterceptor) {
+                if (interceptor instanceof AppendIndexMetadataInterceptor && lh.getLastAddConfirmed() >= 0) {

Review comment:
       Agree with you that instanceof is not a good practice if we have a better choice. 
   There are two class `AppendIndexMetadataInterceptor` and `AppendBrokerTimestampMetadataInterceptor` implement `BrokerEntryMetadataInterceptor`. 
   If using polymorphism and the interceptor is a `AppendBrokerTimestampMetadataInterceptor` whose `recoveryIndexGenerator` method do nothing like 
   ```java
   public void recoveryIndexGenerator(long index) {
          // do nothing.
       }
   ```
   , we still meed to deserialize the `BrokerEntryMetadata`  from entry.




----------------------------------------------------------------
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:
users@infra.apache.org