cmick commented on a change in pull request #16023:
URL: https://github.com/apache/flink/pull/16023#discussion_r642470435
##########
File path:
flink-connectors/flink-connector-rabbitmq/src/main/java/org/apache/flink/streaming/connectors/rabbitmq/common/RMQConnectionConfig.java
##########
@@ -29,21 +29,24 @@
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.util.Optional;
+import java.util.concurrent.TimeUnit;
/**
* Connection Configuration for RMQ. If {@link Builder#setUri(String)} has
been set then {@link
* RMQConnectionConfig#RMQConnectionConfig(String, Integer, Boolean, Boolean,
Integer, Integer,
- * Integer, Integer, Integer)} will be used for initialize the RMQ connection
or {@link
+ * Integer, Integer, Integer, Integer)} will be used for initialize the RMQ
connection or {@link
* RMQConnectionConfig#RMQConnectionConfig(String, Integer, String, String,
String, Integer,
- * Boolean, Boolean, Integer, Integer, Integer, Integer, Integer)} will be
used for initialize the
- * RMQ connection
+ * Boolean, Boolean, Integer, Integer, Integer, Integer, Integer, Integer)}
will be used for
+ * initialize the RMQ connection
*/
public class RMQConnectionConfig implements Serializable {
private static final long serialVersionUID = 1L;
private static final Logger LOG =
LoggerFactory.getLogger(RMQConnectionConfig.class);
+ private static final int DEFAULT_DELIVERY_TIMEOUT = 30000;
Review comment:
Yes, I understand. The issue described FLINK-22698 causes the job to
never stop (gracefully via stop command) if auto-ack is disabled and there is
no new data. So, the current behavior (no timeout) is causing trouble (that's
why its better to set some default here).
IMO the upper bound is dictated by the default Flink _client.timeout_ (60s)
- RMQ connector should not be the cause of CLI timeouts. The lower bound is
related to CPU utilization - if set to some really low values (such as 1ms), it
could start to have a noticeable effect (redundant queue check every 1ms).
Again, there is nothing special with the 30s (e.g. we can set it to 15s as
well), but it **fixes the issue** and meets the above criteria.
--
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:
us...@infra.apache.org
