codelipenghui commented on a change in pull request #14460:
URL: https://github.com/apache/pulsar/pull/14460#discussion_r815386247
##########
File path:
pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java
##########
@@ -1525,11 +1525,6 @@ private static ByteBufPair
serializeCommandSendWithSize(BaseCommand cmd, Checksu
return command;
}
- public static ByteBuf addBrokerEntryMetadata(ByteBuf headerAndPayload,
Review comment:
@eolivelli Do you know which protocol handler uses this method? I'm
curious how the protocol handler works with the method, add broker entry
metadata is only used by the managed ledger internal. If the protocol handler
wants to introduce broker entry metadata changes, it should implement a new
`BrokerEntryMetadataInterceptor`.
IMO, essentially, this is not a public API at the beginning of the design,
It's just that all the pulsar command-related methods are put here.
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerServiceException.java
##########
@@ -77,12 +77,6 @@ public TopicClosedException(Throwable t) {
}
}
- public static class AddEntryMetadataException extends
BrokerServiceException {
Review comment:
@michaeljmarshall For the protocol handler, it should not touch this
exception. Here is an example
https://github.com/apache/pulsar/pull/9039/files#diff-e7b41a1d0ffec3009ff825684ca865095884454d29d81b8f0edf6a6c129e21f4R26-R53
for adding a new entry metadata interceptor, I don't think a protocol handler
will touch this exception unless the protocol handler implements it in the
wrong way. This is the part of the implementation that was forgotten to delete
at the time (in 2.8.0). I have no objection with deprecating it first, just
feel like we're adding complexity to something that was should be removed and
never used.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
