exceptionfactory commented on a change in pull request #5386:
URL: https://github.com/apache/nifi/pull/5386#discussion_r708600644
##########
File path:
nifi-nar-bundles/nifi-extension-utils/nifi-event-transport/src/main/java/org/apache/nifi/event/transport/netty/NettyEventServer.java
##########
@@ -20,23 +20,52 @@
import io.netty.channel.EventLoopGroup;
import org.apache.nifi.event.transport.EventServer;
+import java.util.concurrent.TimeUnit;
+
/**
* Netty Event Server
*/
class NettyEventServer implements EventServer {
+ static final long SHUTDOWN_QUIET_PERIOD = 2000; // netty default
+ static final long SHUTDOWN_TIMEOUT = 15000; // netty default
+ static final TimeUnit SHUTDOWN_TIME_UNIT = TimeUnit.MILLISECONDS;
Review comment:
In light of the duplication of these properties between
`NettyEventSender` and `NettyEventServer`, it seems like they should be
refactored to a shared location. Perhaps an `enum` named something like
`ShutdownDuration` with `TIMEOUT` and `QUIET_PERIOD` properties?
##########
File path:
nifi-nar-bundles/nifi-extension-utils/nifi-event-transport/src/main/java/org/apache/nifi/event/transport/netty/NettyEventSender.java
##########
@@ -25,20 +25,31 @@
import org.apache.nifi.event.transport.EventSender;
import java.net.SocketAddress;
+import java.util.concurrent.TimeUnit;
/**
* Netty Event Sender with Channel Pool
*
* @param <T> Event Type
*/
class NettyEventSender<T> implements EventSender<T> {
+ static final long SHUTDOWN_QUIET_PERIOD = 2000; // netty default
+ static final long SHUTDOWN_TIMEOUT = 15000; // netty default
+ static final TimeUnit SHUTDOWN_TIME_UNIT = TimeUnit.MILLISECONDS;
+
private final EventLoopGroup group;
private final ChannelPool channelPool;
private final SocketAddress remoteAddress;
- private boolean singleEventPerConnection;
+ private final boolean singleEventPerConnection;
+
+ private final long shutdownQuietPeriod;
+
+ private final long shutdownTimeout;
+
+ private final TimeUnit shutdownTimeUnit;
Review comment:
Rather than having separate properties, it seems like the implementation
could be streamlined using `java.time.Duration`. This would remove the need
for the `shutdownTimeUnit` property.
--
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]
