phet commented on code in PR #3792:
URL: https://github.com/apache/gobblin/pull/3792#discussion_r1344827785
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -204,6 +212,14 @@ public MysqlMultiActiveLeaseArbiter(Config config) throws
IOException {
}
initializeConstantsTable();
+ Thread retentionThread = new Thread(new Runnable() {
Review Comment:
rather than a sleeping/blocking thread, how about a scheduled thread pool
executor taking this `Runnable` and an exec interval?
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -110,9 +111,13 @@ protected interface CheckedFunction<T, R> {
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(3) DEFAULT CURRENT_TIMESTAMP(3), "
- + "lease_acquisition_timestamp TIMESTAMP(3) NULL DEFAULT NULL, "
+ + "event_timestamp TIMESTAMP NOT NULL, "
+ + "lease_acquisition_timestamp TIMESTAMP NULL, "
Review Comment:
as far as migrating this schema... will it require manual intervention to
either `drop` or `alter table`?
##########
gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java:
##########
@@ -101,6 +101,8 @@ public class ConfigurationKeys {
public static final String DEFAULT_MULTI_ACTIVE_SCHEDULER_CONSTANTS_DB_TABLE
= "gobblin_multi_active_scheduler_constants_store";
public static final String SCHEDULER_LEASE_DETERMINATION_STORE_DB_TABLE_KEY
= MYSQL_LEASE_ARBITER_PREFIX + ".schedulerLeaseArbiter.store.db.table";
public static final String
DEFAULT_SCHEDULER_LEASE_DETERMINATION_STORE_DB_TABLE =
"gobblin_scheduler_lease_determination_store";
+ public static final String
SCHEDULER_LEASE_DETERMINATION_TABLE_RETENTION_PERIOD_MILLIS_KEY =
MYSQL_LEASE_ARBITER_PREFIX + ".retentionPeriodMillis";
+ public static final int
DEFAULT_SCHEDULER_LEASE_DETERMINATION_TABLE_RETENTION_PERIOD_MILLIS = 500000;
Review Comment:
500 seconds? seems way too low... at least for debugging. I'd look at more
like 72 hours
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -221,6 +237,31 @@ private void initializeConstantsTable() throws IOException
{
}, true);
}
+ /**
+ * Periodically deletes all rows in the table with event_timestamp older
than the retention period defined by config.
+ */
+ private void runRetentionOnArbitrationTable() {
+ while (true) {
+ try {
+ Thread.sleep(10000);
Review Comment:
tip: (same as if you use scheduled TP executor) - set sleep in time-esque
values (such as 60). also, 10s looks WAY too frequent, given a lease may
itself last for minutes! maybe we try this every six hours (4/daily)?
--
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]
