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 2020/11/12 22:33:54 UTC
[GitHub] [pulsar] sijie commented on a change in pull request #6720: Make ServerCnx, Producer and Consumer independent of Netty
sijie commented on a change in pull request #6720:
URL: https://github.com/apache/pulsar/pull/6720#discussion_r522398710
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/TransportCnx.java
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.service;
+
+import org.apache.bookkeeper.mledger.Entry;
+import org.apache.pulsar.broker.authentication.AuthenticationDataSource;
+import org.apache.pulsar.common.api.proto.PulsarApi.ServerError;
+
+import java.net.SocketAddress;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+
+public interface TransportCnx {
+
+ default String getClientVersion() {
+ return null;
Review comment:
why do you make this a default method and return `null`?
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PulsarCommandSender.java
##########
@@ -26,39 +29,92 @@
public interface PulsarCommandSender {
-
- void sendPartitionMetadataResponse(PulsarApi.ServerError error, String errorMsg, long requestId);
-
- void sendPartitionMetadataResponse(int partitions, long requestId);
-
- void sendSuccessResponse(long requestId);
-
- void sendErrorResponse(long requestId, PulsarApi.ServerError error, String message);
-
- void sendProducerSuccessResponse(long requestId, String producerName, SchemaVersion schemaVersion);
-
- void sendProducerSuccessResponse(long requestId, String producerName, long lastSequenceId,
- SchemaVersion schemaVersion);
-
- void sendSendReceiptResponse(long producerId, long sequenceId, long highestId, long ledgerId,
- long entryId);
-
- void sendSendError(long producerId, long sequenceId, PulsarApi.ServerError error, String errorMsg);
-
- void sendGetTopicsOfNamespaceResponse(List<String> topics, long requestId);
-
- void sendGetSchemaResponse(long requestId, SchemaInfo schema, SchemaVersion version);
-
- void sendGetSchemaErrorResponse(long requestId, PulsarApi.ServerError error, String errorMessage);
-
- void sendGetOrCreateSchemaResponse(long requestId, SchemaVersion schemaVersion);
-
- void sendGetOrCreateSchemaErrorResponse(long requestId, PulsarApi.ServerError error, String errorMessage);
-
- void sendConnectedResponse(int clientProtocolVersion, int maxMessageSize);
-
- void sendLookupResponse(String brokerServiceUrl, String brokerServiceUrlTls, boolean authoritative,
- PulsarApi.CommandLookupTopicResponse.LookupType response, long requestId, boolean proxyThroughServiceUrl);
-
- void sendLookupResponse(PulsarApi.ServerError error, String errorMsg, long requestId);
+ default void sendPartitionMetadataResponse(PulsarApi.ServerError error, String errorMsg, long requestId) {
Review comment:
why do you change it to the default method?
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/TransportCnx.java
##########
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.service;
+
+import org.apache.bookkeeper.mledger.Entry;
+import org.apache.pulsar.broker.authentication.AuthenticationDataSource;
+import org.apache.pulsar.common.api.proto.PulsarApi.ServerError;
+
+import java.net.SocketAddress;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+
+public interface TransportCnx {
Review comment:
Can we please avoid `default` methods here? Any transport cnx implementation should implement all the following methods. I don't think we should make them default methods. Making them default methods is error-prone.
----------------------------------------------------------------
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