boneanxs commented on code in PR #6468:
URL: https://github.com/apache/hudi/pull/6468#discussion_r1250338414
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala:
##########
@@ -594,6 +595,10 @@ object HoodieSparkSqlWriter {
(syncHiveSuccess, common.util.Option.ofNullable(instantTime))
}
+ def cleanup() : Unit = {
Review Comment:
@junyuc25 Since we already have a shutdown hook to close the metric, is
explicitly shutdown metric here still necessary?
I found an issue that if the metric server experiences problems such as high
load, it fails to respond quickly, causing queries to be blocked until the
timeout, the `Metrics.shutdown` here could block the query to finish until the
timeout.
```java
23/07/03 14:16:31 WARN GraphiteReporter: Unable to report to Graphite
java.net.ConnectException: Connection timed out (Connection timed out)
at java.net.PlainSocketImpl.socketConnect(Native Method)
at
java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
at
java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
at
java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
at java.net.Socket.connect(Socket.java:607)
at java.net.Socket.connect(Socket.java:556)
at java.net.Socket.<init>(Socket.java:452)
at java.net.Socket.<init>(Socket.java:262)
at
javax.net.DefaultSocketFactory.createSocket(SocketFactory.java:277)
at
org.apache.hudi.com.codahale.metrics.graphite.Graphite.connect(Graphite.java:130)
at
org.apache.hudi.com.codahale.metrics.graphite.GraphiteReporter.report(GraphiteReporter.java:263)
at
org.apache.hudi.com.codahale.metrics.ScheduledReporter.report(ScheduledReporter.java:237)
at
org.apache.hudi.metrics.MetricsGraphiteReporter.report(MetricsGraphiteReporter.java:74)
at
org.apache.hudi.metrics.Metrics.reportAndStopReporter(Metrics.java:61)
at org.apache.hudi.metrics.Metrics.shutdown(Metrics.java:105)
at java.lang.Thread.run(Thread.java:748)
```
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -53,14 +53,15 @@ private Metrics(HoodieWriteConfig metricConfig) {
}
reporter.start();
- Runtime.getRuntime().addShutdownHook(new
Thread(this::reportAndCloseReporter));
+ Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
Review Comment:
we better to catch exceptions to fail it silently?
--
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-unsubscr...@hudi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
