phet commented on code in PR #3715:
URL: https://github.com/apache/gobblin/pull/3715#discussion_r1265977660
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -287,32 +304,43 @@ else if (leaseValidityStatus == 2) {
}
}
- protected Optional<GetEventInfoResult> createGetInfoResult(ResultSet
resultSet) {
+ protected GetEventInfoResult createGetInfoResult(ResultSet resultSet) throws
SQLException {
try {
// Extract values from result set
Timestamp dbEventTimestamp = resultSet.getTimestamp("event_timestamp");
Timestamp dbLeaseAcquisitionTimestamp =
resultSet.getTimestamp("lease_acquisition_timestamp");
- boolean withinEpsilon = resultSet.getBoolean("isWithinEpsilon");
- int leaseValidityStatus = resultSet.getInt("leaseValidityStatus");
+ boolean withinEpsilon = resultSet.getBoolean("is_within_epsilon");
+ int leaseValidityStatus = resultSet.getInt("lease_validity_status");
int dbLinger = resultSet.getInt("linger");
Timestamp dbCurrentTimestamp =
resultSet.getTimestamp("CURRENT_TIMESTAMP");
- return Optional.of(new GetEventInfoResult(dbEventTimestamp,
dbLeaseAcquisitionTimestamp, withinEpsilon, leaseValidityStatus,
- dbLinger, dbCurrentTimestamp));
- } catch (SQLException exception) {
- log.warn("Failed to retrieve values from GET event info query resultSet.
Exception: ", exception);
- // Note: this will proceed to CASE 1 of acquiring a lease above
- return Optional.absent();
+ return new GetEventInfoResult(dbEventTimestamp,
dbLeaseAcquisitionTimestamp, withinEpsilon, leaseValidityStatus,
+ dbLinger, dbCurrentTimestamp);
+ } catch (SQLException e) {
+ throw e;
+ } finally {
+ if (resultSet != null) {
+ resultSet.close();
+ }
}
}
protected SelectInfoResult createSelectInfoResult(ResultSet resultSet)
throws SQLException {
- if (!resultSet.next()) {
- log.error("Expected num rows and lease_acquisition_timestamp returned
from query but received nothing");
+ try {
+ if (!resultSet.next()) {
+ resultSet.close();
Review Comment:
this is already in the `finally`... why add here too? (seems confusing)
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -287,32 +304,43 @@ else if (leaseValidityStatus == 2) {
}
}
- protected Optional<GetEventInfoResult> createGetInfoResult(ResultSet
resultSet) {
+ protected GetEventInfoResult createGetInfoResult(ResultSet resultSet) throws
SQLException {
try {
// Extract values from result set
Timestamp dbEventTimestamp = resultSet.getTimestamp("event_timestamp");
Timestamp dbLeaseAcquisitionTimestamp =
resultSet.getTimestamp("lease_acquisition_timestamp");
- boolean withinEpsilon = resultSet.getBoolean("isWithinEpsilon");
- int leaseValidityStatus = resultSet.getInt("leaseValidityStatus");
+ boolean withinEpsilon = resultSet.getBoolean("is_within_epsilon");
+ int leaseValidityStatus = resultSet.getInt("lease_validity_status");
int dbLinger = resultSet.getInt("linger");
Timestamp dbCurrentTimestamp =
resultSet.getTimestamp("CURRENT_TIMESTAMP");
- return Optional.of(new GetEventInfoResult(dbEventTimestamp,
dbLeaseAcquisitionTimestamp, withinEpsilon, leaseValidityStatus,
- dbLinger, dbCurrentTimestamp));
- } catch (SQLException exception) {
- log.warn("Failed to retrieve values from GET event info query resultSet.
Exception: ", exception);
- // Note: this will proceed to CASE 1 of acquiring a lease above
- return Optional.absent();
+ return new GetEventInfoResult(dbEventTimestamp,
dbLeaseAcquisitionTimestamp, withinEpsilon, leaseValidityStatus,
+ dbLinger, dbCurrentTimestamp);
+ } catch (SQLException e) {
+ throw e;
Review Comment:
1. if this is all you want, it's the implicit behavior, which you need not
write explicitly.
2. even so, elsewhere, we wrap `SQLException` in an `IOException`. do we
want that here too... or is there already a higher layer wrapping around this
invocation where that will happen for us?
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -375,17 +473,61 @@ public boolean recordLeaseSuccess(LeaseObtainedStatus
status)
}
/** Abstracts recurring pattern around resource management and exception
re-mapping. */
- protected <T> T withPreparedStatement(String sql,
CheckedFunction<PreparedStatement, T> f, boolean shouldCommit) throws
IOException {
+ protected <T> T withPreparedStatement(String sql,
CheckedFunction<PreparedStatement, T> f, boolean shouldCommit)
+ throws IOException {
try (Connection connection = this.dataSource.getConnection();
PreparedStatement statement = connection.prepareStatement(sql)) {
T result = f.apply(statement);
if (shouldCommit) {
connection.commit();
}
+ statement.close();
return result;
} catch (SQLException e) {
- log.warn("Received SQL exception that can result from invalid
connection. Checking if validation query is set {} Exception is {}",
((HikariDataSource) this.dataSource).getConnectionTestQuery(), e);
+ log.warn("Received SQL exception that can result from invalid
connection. Checking if validation query is set {} "
+ + "Exception is {}", ((HikariDataSource)
this.dataSource).getConnectionTestQuery(), e);
throw new IOException(e);
}
}
+
+
+ /**
+ * DTO for arbiter's current lease state for a FlowActionEvent
+ */
+ @Data
+ static class GetEventInfoResult {
+ private Timestamp dbEventTimestamp;
+ private Timestamp dbLeaseAcquisitionTimestamp;
+ private boolean withinEpsilon;
+ private int leaseValidityStatus;
+ private int dbLinger;
+ private Timestamp dbCurrentTimestamp;
+
+ GetEventInfoResult(Timestamp eventTimestamp, Timestamp
leaseAcquisitionTimestamp, boolean isWithinEpsilon,
+ int validityStatus, int linger, Timestamp currentTimestamp) {
+ // Extract values from result set
+ dbEventTimestamp = eventTimestamp;
+ dbLeaseAcquisitionTimestamp = leaseAcquisitionTimestamp;
+ withinEpsilon = isWithinEpsilon;
+ leaseValidityStatus = validityStatus;
+ dbLinger = linger;
+ dbCurrentTimestamp = currentTimestamp;
+ }
Review Comment:
won't the `@Data` annotation generate such an all-args ctor for us?
--
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]
