nodece commented on a change in pull request #12367:
URL: https://github.com/apache/pulsar/pull/12367#discussion_r729745426
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/systopic/TopicPoliciesSystemTopicClient.java
##########
@@ -81,24 +81,24 @@ private TopicPolicyWriter(Producer<PulsarEvent> producer,
SystemTopicClient<Puls
@Override
public MessageId write(PulsarEvent event) throws PulsarClientException
{
+ validateActionType(event);
return
producer.newMessage().key(getEventKey(event)).value(event).send();
}
@Override
public CompletableFuture<MessageId> writeAsync(PulsarEvent event) {
+ validateActionType(event);
return
producer.newMessage().key(getEventKey(event)).value(event).sendAsync();
}
@Override
public MessageId delete(PulsarEvent event) throws
PulsarClientException {
- validateActionType(event);
- return
producer.newMessage().key(getEventKey(event)).value(null).send();
+ return this.write(event);
Review comment:
@315157973 sorry, I don't notice the previous PR, but I still think that
we don't should set null as policy message value when deleting the topic
policy, we can set the `event.getTopicPoliciesEvent().policies` as `null`
instead of policy message value with `null`, which will cause the data in
compaction topic to never be cleaned up, but this approach can fix the `When
Topic Policies are deleted, it will not trigger all Topics to update, which
looks like a bug`.
If you have any approaches, please let me know.
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java
##########
@@ -97,8 +97,7 @@ public SystemTopicBasedTopicPoliciesService(PulsarService
pulsarService) {
result.completeExceptionally(ex);
} else {
PulsarEvent event = getPulsarEvent(topicName, actionType,
policies);
- CompletableFuture<MessageId> actionFuture =
- ActionType.DELETE.equals(actionType) ?
writer.deleteAsync(event) : writer.writeAsync(event);
+ CompletableFuture<MessageId> actionFuture =
writer.writeAsync(event);
Review comment:
`writer.deleteAsync()` has been update, same as `writer.writeAsync()`.
--
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]
