phet commented on code in PR #3790:
URL: https://github.com/apache/gobblin/pull/3790#discussion_r1343260707
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/Orchestrator.java:
##########
@@ -264,9 +265,9 @@ public void orchestrate(Spec spec, Properties jobProps,
long triggerTimestampMil
String flowExecutionId =
flowMetadata.get(TimingEvent.FlowEventConstants.FLOW_EXECUTION_ID_FIELD);
DagActionStore.DagAction flowAction =
new DagActionStore.DagAction(flowGroup, flowName, flowExecutionId,
DagActionStore.FlowActionType.LAUNCH);
- flowTriggerHandler.get().handleTriggerEvent(jobProps, flowAction,
triggerTimestampMillis);
- _log.info("Multi-active scheduler finished handling trigger event:
[{}, triggerEventTimestamp: {}]", flowAction,
- triggerTimestampMillis);
+ flowTriggerHandler.get().handleTriggerEvent(jobProps, flowAction,
triggerTimestampMillis, isReminderEvent);
+ _log.info("Multi-active scheduler finished handling trigger event:
[{}, triggerEventTimestamp: {}]" +
+ "(is " + (isReminderEvent ? "reminder" : "original") + ")",
flowAction, triggerTimestampMillis);
Review Comment:
now that you're using the `{}` placeholders, it seems clearest to use
throughout, as:
```
log.info("Multi-active ... event: [{}, triggerEventTimestamp: {}] (is {})",
flowAction, ttMillis, isReminderEvent ? "reminder" : "original");
```
writing it that way makes spotting the lack of space before `"(is "` easier
to see.
also, to entertain which we prefer:
```
event: [{1}, triggerEventTimestamp: {2}] (is {3})
{3} event: [{1}, triggerEventTimestamp: {2}]
event: [{1}, is: {3}, triggerEventTimestamp: {2}]
```
(numbered purely for illustration)
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -104,32 +116,50 @@ protected interface CheckedFunction<T, R> {
+ "VALUES(1, ?, ?) ON DUPLICATE KEY UPDATE epsilon=VALUES(epsilon),
linger=VALUES(linger)";
protected static final String WHERE_CLAUSE_TO_MATCH_KEY = "WHERE
flow_group=? AND flow_name=? AND flow_action=?";
protected static final String WHERE_CLAUSE_TO_MATCH_ROW =
WHERE_CLAUSE_TO_MATCH_KEY
- + " AND event_timestamp=? AND lease_acquisition_timestamp=?";
- protected static final String SELECT_AFTER_INSERT_STATEMENT = "SELECT
event_timestamp, lease_acquisition_timestamp, "
- + "linger FROM %s, %s " + WHERE_CLAUSE_TO_MATCH_KEY;
+ + " AND event_timestamp=CONVERT_TZ(?, '+00:00', @@session.time_zone)"
+ + " AND lease_acquisition_timestamp=CONVERT_TZ(?, '+00:00',
@@session.time_zone)";
Review Comment:
aren't we on mysql 8? if so, how about this -
https://dba.stackexchange.com/a/279140 ?
```
event_timestamp TIMESTAMP(3) NOT NULL DEFAULT (UTC_TIMESTAMP)
```
?
--
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]
