From c698d7d641290e1ab851ea7eeb109d892294e622 Mon Sep 17 00:00:00 2001 From: masokol <97948057+masokol@users.noreply.github.com> Date: Fri, 18 Aug 2023 13:46:39 +0200 Subject: [PATCH] Fix priority calculation for local queue (#547) Closes #546 --- CHANGES.md | 1 + .../core/repair/RepairSchedulerImpl.java | 3 +-- .../ecchronos/core/repair/TableRepairJob.java | 24 ++++++++++++------- .../core/scheduling/ScheduledJob.java | 10 ++++++++ .../core/scheduling/ScheduledJobQueue.java | 1 + .../core/repair/TestTableRepairJob.java | 3 --- 6 files changed, 28 insertions(+), 14 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 9d67a07e0..0a668078d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,7 @@ ## Version 1.0.8 (Not yet released) +* Fix priority calculation for local queue - Issue #546 * Skip unnecessary reads from repair history - Issue #548 * Fix repair job priority - Issue #515 * Fix malformed IPv6 for JMX - Issue #306 diff --git a/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/repair/RepairSchedulerImpl.java b/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/repair/RepairSchedulerImpl.java index 8addc7dc4..8445c14e5 100644 --- a/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/repair/RepairSchedulerImpl.java +++ b/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/repair/RepairSchedulerImpl.java @@ -173,8 +173,7 @@ private TableRepairJob getRepairJob(TableReference tableReference, RepairConfigu .withRepairLockType(myRepairLockType) .withRepairPolices(myRepairPolicies) .build(); - - job.runnable(); + job.refreshState(); return job; } diff --git a/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/repair/TableRepairJob.java b/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/repair/TableRepairJob.java index 58c563408..e522d8940 100644 --- a/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/repair/TableRepairJob.java +++ b/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/repair/TableRepairJob.java @@ -91,7 +91,8 @@ public Iterator iterator() for (ReplicaRepairGroup replicaRepairGroup : repairStateSnapshot.getRepairGroups()) { - taskList.add(new RepairGroup(getRealPriority(), myTableReference, myRepairConfiguration, + taskList.add(new RepairGroup(getRealPriority(replicaRepairGroup.getLastCompletedAt()), + myTableReference, myRepairConfiguration, replicaRepairGroup, myJmxProxyFactory, myTableRepairMetrics, myRepairLockType.getLockFactory(), new RepairLockFactoryImpl(), myRepairPolicies)); @@ -181,14 +182,6 @@ else if (msSinceLastRepair >= myRepairConfiguration.getRepairWarningTimeInMs()) @Override public boolean runnable() { - try - { - myRepairState.update(); - } catch (Exception e) - { - LOG.warn("Unable to check repair history, {}", this, e); - } - RepairStateSnapshot repairStateSnapshot = myRepairState.getSnapshot(); long lastRepaired = repairStateSnapshot.lastRepairedAt(); @@ -201,6 +194,19 @@ public boolean runnable() return repairStateSnapshot.canRepair() && super.runnable(); } + @Override + public void refreshState() + { + try + { + myRepairState.update(); + } + catch (Exception e) + { + LOG.warn("Unable to check repair history, {}", this, e); + } + } + /** * Calculate real priority based on available tasks. */ diff --git a/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJob.java b/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJob.java index 63972bee9..bffceacdb 100644 --- a/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJob.java +++ b/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJob.java @@ -57,6 +57,16 @@ protected void postExecute(boolean successful) } } + /** + * This method is called every time the scheduler creates a list of jobs to run. + * Use this if you need to do some updates before priority is calculated. + * Default is noop. + */ + protected void refreshState() + { + // NOOP by default + } + /** * Set the job to be runnable again after the given delay has elapsed. * diff --git a/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJobQueue.java b/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJobQueue.java index 3c84e9cda..d914b619e 100644 --- a/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJobQueue.java +++ b/core/src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJobQueue.java @@ -116,6 +116,7 @@ int size() @Override public synchronized Iterator iterator() { + myJobQueues.values().forEach(q -> q.forEach(ScheduledJob::refreshState)); Iterator baseIterator = new ManyToOneIterator<>(myJobQueues.values(), myComparator); return new RunnableJobIterator(baseIterator); diff --git a/core/src/test/java/com/ericsson/bss/cassandra/ecchronos/core/repair/TestTableRepairJob.java b/core/src/test/java/com/ericsson/bss/cassandra/ecchronos/core/repair/TestTableRepairJob.java index 2110b79f3..470204cb7 100644 --- a/core/src/test/java/com/ericsson/bss/cassandra/ecchronos/core/repair/TestTableRepairJob.java +++ b/core/src/test/java/com/ericsson/bss/cassandra/ecchronos/core/repair/TestTableRepairJob.java @@ -165,7 +165,6 @@ public void testPrevalidateNotRepairable() assertThat(myRepairJob.runnable()).isFalse(); - verify(myRepairState, times(1)).update(); verify(myRepairStateSnapshot, times(1)).canRepair(); } @@ -177,7 +176,6 @@ public void testPrevalidateNeedRepair() mockRepairGroup(0L); assertThat(myRepairJob.runnable()).isTrue(); - verify(myRepairState, times(1)).update(); verify(myRepairStateSnapshot, times(2)).canRepair(); } @@ -190,7 +188,6 @@ public void testPrevalidateNotRepairableThenRepairable() assertThat(myRepairJob.runnable()).isFalse(); assertThat(myRepairJob.runnable()).isTrue(); - verify(myRepairState, times(2)).update(); verify(myRepairStateSnapshot, times(3)).canRepair(); }