ashniku commented on code in PR #6412:
URL: https://github.com/apache/hive/pull/6412#discussion_r3458307717
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -163,6 +166,14 @@
*/
public class HiveConnection implements java.sql.Connection {
private static final Logger LOG =
LoggerFactory.getLogger(HiveConnection.class);
+
+ /**
+ * Last effective {@code hive.query.timeout.seconds} in seconds, or {@code
-1} if not yet set.
+ * Seeded from the JDBC URL at connect time; a JDBC {@link
java.sql.Connection} may be shared
+ * across threads with concurrent {@link
org.apache.hive.jdbc.HiveStatement}s on one HS2 session,
+ * so this field uses an {@link AtomicLong} to keep updates well-defined.
+ */
+ private final AtomicLong sessionQueryTimeoutSeconds = new AtomicLong(-1L);
Review Comment:
@deniskuzZ
Thanks for the review.
You're right — for our usage a plain volatile long is sufficient. We only do
a single write at connect time (from the JDBC URL) and reads from HiveStatement
when building the timeout message; there are no read-modify-write operations,
so AtomicLong is more than we need.
I'll change it to:
private volatile long sessionQueryTimeoutSeconds = -1L;
and update the Javadoc accordingly. The original AtomicLong was chosen out
of caution for a shared connection, but volatile covers the visibility we
actually need here.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
