[GitHub] [pulsar] lhotari commented on a diff in pull request #15678: [improve][proxy] Support zero-copy of NIC to NIC on Proxy

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

lhotari commented on code in PR #15678:
URL: https://github.com/apache/pulsar/pull/15678#discussion_r887742932


##########
site2/docs/reference-configuration.md:
##########
@@ -847,6 +847,7 @@ The [Pulsar proxy](concepts-architecture-overview.md#pulsar-proxy) can be config
 |tokenAudienceClaim| The token audience "claim" name, e.g. "aud". It is used to get the audience from token. If it is not set, the audience is not verified. ||
 | tokenAudience | The token audience stands for this broker. The field `tokenAudienceClaim` of a valid token need contains this parameter.| |
 |haProxyProtocolEnabled | Enable or disable the [HAProxy](http://www.haproxy.org/) protocol. |false|
+|proxyZeroCopyModeEnabled | Enable or disable the `zero-copy` of NIC to NIC. If enabled, we will use [splice](https://man7.org/linux/man-pages/man2/splice.2.html) system call instead of read/write to transport data of NIC to NIC, it only work when `proxyLogLevel == 0` and connection without TLS. | true |

Review Comment:
   Please reword the explanation to make it easier to read. 
   ```suggestion
   |proxyZeroCopyModeEnabled |Enables zero-copy transport of data across network interfaces using the [splice](https://man7.org/linux/man-pages/man2/splice.2.html) system call. Zero copy mode cannot be used when TLS is enabled or when `proxyLogLevel is > 0`. | true |
   ```
   
   
   
   
   



##########
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java:
##########
@@ -259,6 +271,33 @@ public void channelRead(final ChannelHandlerContext ctx, Object msg) throws Exce
         }
     }
 
+    /**
+     * Use splice to zero-copy of NIC to NIC.
+     * @param inboundChannel input channel
+     * @param outboundChannel output channel
+     */
+    protected void spliceNIC2NIC(EpollSocketChannel inboundChannel, EpollSocketChannel outboundChannel) {
+        inboundChannel.config().setAutoRead(false);
+
+        ChannelPromise promise = ctx.newPromise();
+        inboundChannel.spliceTo(outboundChannel, SPLICE_BYTES, promise);
+        promise.addListener((ChannelFutureListener) future -> {
+            if (!future.isSuccess()) {
+                future.channel().pipeline().fireExceptionCaught(future.cause());
+            } else {
+                ProxyService.OPS_COUNTER.inc();
+                directProxyHandler.getInboundChannelRequestsRate().recordEvent(SPLICE_BYTES);
+                ProxyService.BYTES_COUNTER.inc(SPLICE_BYTES);
+            }
+        });
+
+        inboundChannel.config().setAutoRead(true);

Review Comment:
   Doesn't this have to happen in the promise listener after the `spliceTo` operation has completed?



##########
conf/proxy.conf:
##########
@@ -60,6 +60,10 @@ advertisedAddress=
 # Enable or disable the HAProxy protocol.
 haProxyProtocolEnabled=false
 
+# Enable or disable the zero-copy of NIC to NIC. If enabled, we will use splice system call instead of read/write to
+# transport data of NIC to NIC, it only work when `proxyLogLevel == 0` and connection without TLS.

Review Comment:
   Please reword the explanation to make it easier to read. 
   
   ```suggestion
   # Enables zero-copy transport of data across network interfaces using the splice system call.
   # Zero copy mode cannot be used when TLS is enabled or when proxyLogLevel is > 0
   ```
   
   



##########
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConfiguration.java:
##########
@@ -237,6 +237,10 @@ public class ProxyConfiguration implements PulsarConfiguration {
             doc = "Enable or disable the proxy protocol.")
     private boolean haProxyProtocolEnabled;
 
+    @FieldContext(category = CATEGORY_SERVER,
+            doc = "Enable or disable the zero-copy of NIC to NIC.")

Review Comment:
   Improve the doc
   ```suggestion
               doc = "Enables zero-copy transport of data across network interfaces using the spice. Zero copy mode cannot be used when TLS is enabled or when proxyLogLevel is > 0.")
   ```



##########
pulsar-proxy/src/test/resources/proxy.conf:
##########
@@ -60,6 +60,10 @@ advertisedAddress=
 # Enable or disable the HAProxy protocol.
 haProxyProtocolEnabled=false
 
+# Enable or disable the zero-copy of NIC to NIC. If enabled, we will use splice system call instead of read/write to
+# transport data of NIC to NIC, it only work when `proxyLogLevel == 0` and connection without TLS.

Review Comment:
   Please reword the explanation to make it easier to read. 
   
   ```suggestion
   # Enables zero-copy transport of data across network interfaces using the splice system call.
   # Zero copy mode cannot be used when TLS is enabled or when proxyLogLevel is > 0
   ```
   
   



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