umustafi commented on code in PR #3790:
URL: https://github.com/apache/gobblin/pull/3790#discussion_r1343247173
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -86,16 +88,26 @@ protected interface CheckedFunction<T, R> {
private final int epsilon;
private final int linger;
private String thisTableGetInfoStatement;
+ private String thisTableGetInfoStatementForReminder;
private String thisTableSelectAfterInsertStatement;
private String thisTableAcquireLeaseIfMatchingAllStatement;
private String thisTableAcquireLeaseIfFinishedStatement;
// TODO: define retention on this table
+ /*
+ Notes:
+ - Set `event_timestamp` default value to turn off timestamp auto-updates
for row modifications which alters this col
+ in an unexpected way upon completing the lease
+ - Upon reading any timestamps from MySQL we convert the timezone from
session (default) to UTC to consistently
+ use epoch-millis in UTC locally
+ - Upon using any timestamps from local we convert the timezone from UTC to
session
+ - We desire millisecond level precision and denote that with `(3)` for the
TIMESTAMP types
+ */
private static final String CREATE_LEASE_ARBITER_TABLE_STATEMENT = "CREATE
TABLE IF NOT EXISTS %s ("
+ "flow_group varchar(" + ServiceConfigKeys.MAX_FLOW_GROUP_LENGTH + ")
NOT NULL, flow_name varchar("
+ ServiceConfigKeys.MAX_FLOW_GROUP_LENGTH + ") NOT NULL, " + "
flow_action varchar(100) NOT NULL, "
- + "event_timestamp TIMESTAMP, "
- + "lease_acquisition_timestamp TIMESTAMP NULL DEFAULT CURRENT_TIMESTAMP,
"
+ + "event_timestamp TIMESTAMP(3) DEFAULT CURRENT_TIMESTAMP(3), "
Review Comment:
I converted everything to BIGINT and updated the tests but reverted back
because I removed the database laundering after realizing that not having
database laundering made it very hard to deal with `reminderEvents`. We end up
having to think about every possible case of what to do with reminder events
and `withinEpsilon` and `afterLinger` or not while db laundering instead of
using the `triggerTimeMillis` from the hosts may result in out of order
timestamps if we have host drift. With db laundering, we use database timestamp
and it becomes easier to do time conversions with timestamp type. There's a lot
of back and forth that went into this
--
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]
