exceptionfactory commented on code in PR #8061:
URL: https://github.com/apache/nifi/pull/8061#discussion_r1410787708
##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -463,6 +463,39 @@ public class CaptureChangeMySQL extends
AbstractSessionFactoryProcessor {
SSL_MODE_VERIFY_IDENTITY)
.build();
+ public static final PropertyDescriptor KEEP_ALIVE = new
PropertyDescriptor.Builder()
+ .name("Keep Alive")
+ .displayName("Keep Alive")
+ .description("Used to specify heartbeat messages or queries to be
sent after a connection has been idle for a period of time to ensure that " +
+ "the connection remains active. This prevents the database
server or middleware from closing the " +
+ "connection on a connection that has been inactive for a
long time")
+ .required(false)
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
+ .build();
Review Comment:
Instead having this property, what do you think about just having the `Keep
Alive Interval` property? Setting that value to `0` could effectively disable
Keep Alive, and then this property would not be necessary.
##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -463,6 +463,39 @@ public class CaptureChangeMySQL extends
AbstractSessionFactoryProcessor {
SSL_MODE_VERIFY_IDENTITY)
.build();
+ public static final PropertyDescriptor KEEP_ALIVE = new
PropertyDescriptor.Builder()
+ .name("Keep Alive")
+ .displayName("Keep Alive")
+ .description("Used to specify heartbeat messages or queries to be
sent after a connection has been idle for a period of time to ensure that " +
+ "the connection remains active. This prevents the database
server or middleware from closing the " +
+ "connection on a connection that has been inactive for a
long time")
+ .required(false)
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
+ .build();
+
+ public static final PropertyDescriptor KEEPALIVE_INTERVAL = new
PropertyDescriptor.Builder()
+ .name("KeepAlive Interval")
+ .displayName("KeepAlive Interval")
+ .description("Set the Keep-Alive interval in milliseconds for the
connection. the default is 60 seconds")
+ .required(false)
+ .addValidator(StandardValidators.POSITIVE_LONG_VALIDATOR)
+ .expressionLanguageSupported(ExpressionLanguageScope.ENVIRONMENT)
+ .defaultValue("60000")
Review Comment:
Instead of using a number of milliseconds, this property can use the Time
Period Validator, which supports different expressions, such as `60 s` or `500
ms`. That provides a more flexible configuration approach, which allowing the
implementation code to get the value as milliseconds either way.
##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -692,7 +728,11 @@ public void setup(ProcessContext context) {
Long serverId =
context.getProperty(SERVER_ID).evaluateAttributeExpressions().asLong();
- connect(hosts, username, password, serverId, driverLocation,
driverName, connectTimeout, sslContextService, sslMode);
+ Boolean keepAlive = context.getProperty(KEEP_ALIVE).asBoolean();
+ Long keepAliveInterval =
context.getProperty(KEEPALIVE_INTERVAL).evaluateAttributeExpressions().asLong();
+ Long heartbeatInterval =
context.getProperty(HEARTBEAT_INTERVAL).evaluateAttributeExpressions().asLong();
Review Comment:
Recommend declaring these values as `final` and using the `long` primitive
type. With these properties required, they should not be null.
##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -463,6 +463,39 @@ public class CaptureChangeMySQL extends
AbstractSessionFactoryProcessor {
SSL_MODE_VERIFY_IDENTITY)
.build();
+ public static final PropertyDescriptor KEEP_ALIVE = new
PropertyDescriptor.Builder()
+ .name("Keep Alive")
+ .displayName("Keep Alive")
+ .description("Used to specify heartbeat messages or queries to be
sent after a connection has been idle for a period of time to ensure that " +
+ "the connection remains active. This prevents the database
server or middleware from closing the " +
+ "connection on a connection that has been inactive for a
long time")
+ .required(false)
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
+ .build();
+
+ public static final PropertyDescriptor KEEPALIVE_INTERVAL = new
PropertyDescriptor.Builder()
+ .name("KeepAlive Interval")
+ .displayName("KeepAlive Interval")
+ .description("Set the Keep-Alive interval in milliseconds for the
connection. the default is 60 seconds")
+ .required(false)
Review Comment:
Since a default value is specified, this property can be marked as required.
##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -463,6 +463,39 @@ public class CaptureChangeMySQL extends
AbstractSessionFactoryProcessor {
SSL_MODE_VERIFY_IDENTITY)
.build();
+ public static final PropertyDescriptor KEEP_ALIVE = new
PropertyDescriptor.Builder()
+ .name("Keep Alive")
+ .displayName("Keep Alive")
+ .description("Used to specify heartbeat messages or queries to be
sent after a connection has been idle for a period of time to ensure that " +
+ "the connection remains active. This prevents the database
server or middleware from closing the " +
+ "connection on a connection that has been inactive for a
long time")
+ .required(false)
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
+ .build();
+
+ public static final PropertyDescriptor KEEPALIVE_INTERVAL = new
PropertyDescriptor.Builder()
+ .name("KeepAlive Interval")
+ .displayName("KeepAlive Interval")
+ .description("Set the Keep-Alive interval in milliseconds for the
connection. the default is 60 seconds")
+ .required(false)
+ .addValidator(StandardValidators.POSITIVE_LONG_VALIDATOR)
+ .expressionLanguageSupported(ExpressionLanguageScope.ENVIRONMENT)
+ .defaultValue("60000")
+ .build();
+
+ public static final PropertyDescriptor HEARTBEAT_INTERVAL = new
PropertyDescriptor.Builder()
+ .name("Heartbeat Interval")
+ .displayName("Heartbeat Interval")
+ .description("heartbeatInterval – heartbeat period in
milliseconds.the default is 1 seconds." +
+ "Note that when used together with keepAlive
heartbeatInterval MUST be set less than keepAliveInterval.")
+ .required(false)
+ .addValidator(StandardValidators.POSITIVE_LONG_VALIDATOR)
+ .expressionLanguageSupported(ExpressionLanguageScope.ENVIRONMENT)
+ .defaultValue("1000")
Review Comment:
See note above regarding the use of the Time Period Validator.
##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -850,6 +890,10 @@ protected void connect(List<InetSocketAddress> hosts,
String username, String pa
binlogClient.setSslSocketFactory(sslSocketFactory);
}
+ binlogClient.setKeepAlive(keepAlive);
Review Comment:
As mentioned above, based on using `0` as an indicator of disabled keep
alive, this property could be set conditionally.
##########
nifi-nar-bundles/nifi-cdc/nifi-cdc-mysql-bundle/nifi-cdc-mysql-processors/src/main/java/org/apache/nifi/cdc/mysql/processors/CaptureChangeMySQL.java:
##########
@@ -463,6 +463,39 @@ public class CaptureChangeMySQL extends
AbstractSessionFactoryProcessor {
SSL_MODE_VERIFY_IDENTITY)
.build();
+ public static final PropertyDescriptor KEEP_ALIVE = new
PropertyDescriptor.Builder()
+ .name("Keep Alive")
+ .displayName("Keep Alive")
+ .description("Used to specify heartbeat messages or queries to be
sent after a connection has been idle for a period of time to ensure that " +
+ "the connection remains active. This prevents the database
server or middleware from closing the " +
+ "connection on a connection that has been inactive for a
long time")
+ .required(false)
+ .allowableValues("true", "false")
+ .defaultValue("true")
+ .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
+ .build();
+
+ public static final PropertyDescriptor KEEPALIVE_INTERVAL = new
PropertyDescriptor.Builder()
+ .name("KeepAlive Interval")
+ .displayName("KeepAlive Interval")
+ .description("Set the Keep-Alive interval in milliseconds for the
connection. the default is 60 seconds")
Review Comment:
It is not necessary to mention the default value in the description, but it
would be worth mentioning that setting the value to 0 disables Keep Alive
messages.
--
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]
