agam-99 commented on code in PR #4191:
URL: https://github.com/apache/gobblin/pull/4191#discussion_r3232783184
##########
gobblin-metrics-libs/gobblin-metrics-base/src/main/avro/DagActionStoreChangeEvent.avsc:
##########
@@ -50,6 +50,11 @@
},
"doc" : "type of dag action",
"compliance" : "NONE"
- }
- ]
+ }, {
+ "name" : "dbUpdateTimeMillis",
Review Comment:
Renamed to `storeInsertTimeMillis` end-to-end in b1ae98288 — Avro field,
JobSpec key, and `LeaseParams` field now share the same name across the chain.
##########
gobblin-metrics-libs/gobblin-metrics-base/src/main/avro/DagActionStoreChangeEvent.avsc:
##########
@@ -50,6 +50,11 @@
},
"doc" : "type of dag action",
"compliance" : "NONE"
- }
- ]
+ }, {
+ "name" : "dbUpdateTimeMillis",
Review Comment:
Good question. The setter lives in the LinkedIn-internal MySQL CDC producer
(binlog reader) that publishes onto Kafka — `storeInsertTimeMillis` is
populated from the binlog event timestamp at row-INSERT time. We're keeping
this PR schema-only/plumbing here so the optional field can flow through the
existing pipeline. On the read side, the new `MysqlMultiActiveLeaseArbiter` +
`DagActionReminderScheduler` changes in b1ae98288 now propagate the value
through consensus and Quartz reminders so `LaunchDagProc` actually stamps the
JobSpec.
For any other open-source producer that wants to populate it, the Avro
builder exposes `.setStoreInsertTimeMillis(long)` after regeneration; omitting
it falls back to `null` and consumers see UNKNOWN — fully backwards-compatible.
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagActionStore.java:
##########
@@ -100,19 +100,37 @@ public Dag.DagId getDagId() {
* {@link DagAction} along with the time it was requested, denoted by the
`eventTimeMillis` field. It also tracks
* whether it has been previously passed to the {@link
MultiActiveLeaseArbiter} to attempt ownership over the flow
* event, indicated by the 'isReminder' field (true when it has been
previously attempted).
+ *
+ * The `storeInsertTimeMillis` field carries the original DagAction store
row-insert time (sourced upstream
+ * from the CDC binlog event timestamp) when known. It is independent of
`eventTimeMillis`, which the lease
+ * arbiter may rewrite via consensus. A value of {@link
#UNKNOWN_STORE_INSERT_TIME_MILLIS} signals "not provided"
+ * and is the default for callers that do not have access to the source
timestamp.
*/
+ long UNKNOWN_STORE_INSERT_TIME_MILLIS = -1L;
Review Comment:
Moved into `LeaseParams` as `public static final long
UNKNOWN_STORE_INSERT_TIME_MILLIS = -1L;` in b1ae98288. Updated the two external
references (`DagActionReminderScheduler`, `LaunchDagProc`) to
`DagActionStore.LeaseParams.UNKNOWN_STORE_INSERT_TIME_MILLIS` and adjusted the
surrounding javadoc link.
--
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]
