[
https://issues.apache.org/jira/browse/GOBBLIN-1923?focusedWorklogId=883436&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-883436
]
ASF GitHub Bot logged work on GOBBLIN-1923:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 04/Oct/23 22:17
Start Date: 04/Oct/23 22:17
Worklog Time Spent: 10m
Work Description: phet commented on code in PR #3792:
URL: https://github.com/apache/gobblin/pull/3792#discussion_r1346381821
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -87,13 +89,14 @@ protected interface CheckedFunction<T, R> {
private final String constantsTableName;
private final int epsilon;
private final int linger;
+ private final int retention;
Review Comment:
this name is not descriptive (if it's a time unit, it should be long and
have a suffix for its units)
##########
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 = 300000000;
Review Comment:
small nit: if you're not using `TimeUnit`, which is self-documenting:
```
TimeUnit.HOURS.toMillis(80);
TimeUnit.DAYS.toMillis(3);
```
please at least use time-esque values:
```
80 * 60 * 60 * 1000; // (80 hours in ms)
3 * 24 * 60 * 60 * 1000; // (3 days in ms)
```
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -221,6 +231,35 @@ 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() {
+ ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1);
+ Runnable retentionTask = () -> {
+ try {
+ Thread.sleep(10000);
+ withPreparedStatement(thisTableRetentionStatement,
+ retentionStatement -> {
+ int i = 0;
+ retentionStatement.setInt(++i, retention);
Review Comment:
`++i` seems obfuscated compared to hard-coding 1
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -221,6 +231,35 @@ 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() {
Review Comment:
seems worth abstracting into a utility along the lines of "run this
arbitrary SQL in a STPE using interval T"
you decide right now whether to do or merely "TODO" ;)
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -221,6 +231,35 @@ 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() {
+ ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1);
+ Runnable retentionTask = () -> {
+ try {
+ Thread.sleep(10000);
+ withPreparedStatement(thisTableRetentionStatement,
+ retentionStatement -> {
+ int i = 0;
+ retentionStatement.setInt(++i, retention);
+ int numRowsDeleted = retentionStatement.executeUpdate();
+ if (numRowsDeleted != 0) {
+ log.info("Multi-active lease arbiter retention thread deleted
{} rows from the lease arbiter table",
+ numRowsDeleted);
+ }
+ return numRowsDeleted;
+ }, true);
+ } catch (InterruptedException | IOException e) {
+ log.warn("Failing to run retention on lease arbiter table. Unbounded
growth can lead to database slowness and "
Review Comment:
`log.error`?
Issue Time Tracking
-------------------
Worklog Id: (was: 883436)
Time Spent: 1h 20m (was: 1h 10m)
> Add retention thread for lease arbiter table
> --------------------------------------------
>
> Key: GOBBLIN-1923
> URL: https://issues.apache.org/jira/browse/GOBBLIN-1923
> Project: Apache Gobblin
> Issue Type: Bug
> Components: gobblin-service
> Reporter: Urmi Mustafi
> Assignee: Abhishek Tiwari
> Priority: Major
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> Add retention to lease arbiter table so it does not grow unbounded. The table
> can be as large as O(number of flows) which may grow so large that
> reading/writing from this table becomes time consuming and slows down our
> throughput of obtaining and evaluating leases for launching flows.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
