Fix priority calculation for local queue (#547)

Closes #546
Ericsson · Aug 18, 2023 · c698d7d · c698d7d
1 parent 43b9a39
commit c698d7d
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 14 deletions.
diff --git 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
@@ -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
@@ -91,7 +91,8 @@ public Iterator<ScheduledTask> 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
@@ -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/...src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJobQueue.java b/...src/main/java/com/ericsson/bss/cassandra/ecchronos/core/scheduling/ScheduledJobQueue.java
@@ -116,6 +116,7 @@ int size()
     @Override
     public synchronized Iterator<ScheduledJob> iterator()
     {
+        myJobQueues.values().forEach(q -> q.forEach(ScheduledJob::refreshState));
         Iterator<ScheduledJob> 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
@@ -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();
     }