UNOMI-979: Judge scheduler lock expiry by the owner's recorded lease, deflake scheduler tests

.github/workflows/unomi-ci-build-tests.yml

@@ -21,7 +21,9 @@ jobs:
   unit-tests:
     name: Execute unit tests
     runs-on: ubuntu-latest
-    timeout-minutes: 15
+    # Slightly above the previous 15: retried failures (rerunFailingTestsCount below) add time
+    # on a red build, and a timeout is a much worse signal than a clean failure.
+    timeout-minutes: 20
     steps:
     - uses: actions/checkout@v5
     - name: Set up JDK 17
@@ -36,12 +38,79 @@ jobs:
         sudo apt-get install -y graphviz
         dot -V
     - name: Build and Unit tests
+      env:
+        # Retry a failing test twice before calling the build red. Several suites (notably the
+        # scheduler ones) are timing-sensitive and this runner has 2 vCPU, so a single unlucky
+        # scheduling hiccup should not fail a whole build. A test that only passes on retry is
+        # NOT silently forgiven: Surefire records it as a flake, and the step below surfaces
+        # every one in the job summary so the flake rate stays visible instead of becoming
+        # invisible green.
+        MAVEN_EXTRA_OPTS: -Dsurefire.rerunFailingTestsCount=2
       run: ./build.sh --ci
     # Keep only third-party dependencies in the post-job Maven cache: Unomi's own
     # snapshots are rebuilt every run and would only bloat the cache / risk staleness
     - name: Clean Unomi artifacts from Maven cache
       if: always()
       run: rm -rf ~/.m2/repository/org/apache/unomi
+    # A flake is a test that failed and then passed on retry. The build is green, so without
+    # this the signal is lost entirely — which is how the scheduler suites stayed unreliable
+    # for as long as they did.
+    - name: Detect flaky tests
+      id: flakes
+      if: always()
+      run: |
+        python3 - <<'PY' >> "$GITHUB_STEP_SUMMARY"
+        import glob, os, xml.etree.ElementTree as ET
+        flaky = []
+        for path in glob.glob('**/target/surefire-reports/TEST-*.xml', recursive=True):
+            try:
+                root = ET.parse(path).getroot()
+            except ET.ParseError:
+                continue
+            for case in root.iter('testcase'):
+                reruns = case.findall('flakyFailure') + case.findall('flakyError')
+                if reruns:
+                    msg = (reruns[0].get('message') or '').strip().replace('\n', ' ')
+                    flaky.append((case.get('classname', '?'), case.get('name', '?'),
+                                  len(reruns), msg[:160]))
+        if flaky:
+            print('### :warning: Flaky tests detected\n')
+            print('These failed and then passed on retry. The build is green, but each one is')
+            print('a real intermittent failure worth investigating.\n')
+            print('| Test | Retries | First failure |')
+            print('| --- | --- | --- |')
+            for cls, name, n, msg in sorted(flaky):
+                print(f'| `{cls}.{name}` | {n} | {msg or "—"} |')
+        else:
+            print('### No flaky tests detected\n')
+        with open(os.environ['GITHUB_OUTPUT'], 'a') as out:
+            out.write(f'found={"true" if flaky else "false"}\n')
+            out.write(f'count={len(flaky)}\n')
+        PY
+    # Uploaded when the build failed OR when something only passed on retry: those are exactly
+    # the runs where the reports (and the scheduler diagnostics dumped into them) are worth
+    # keeping. Skipped on a clean green run so this does not accumulate on every push.
+    - name: Archive unit test reports
+      uses: actions/upload-artifact@v6
+      if: always() && (job.status == 'failure' || steps.flakes.outputs.found == 'true')
+      with:
+        name: unit-test-reports-jdk17-${{ github.run_number }}
+        path: |
+          **/target/surefire-reports/**
+        if-no-files-found: ignore
+        retention-days: 14
+    # Always publish so a later "re-run failed jobs" pass updates the check to green, matching
+    # the integration-test job's behaviour.
+    - name: Publish Test Report
+      uses: mikepenz/action-junit-report@v3
+      if: always()
+      continue-on-error: true
+      with:
+        report_paths: '**/target/surefire-reports/TEST-*.xml'
+        check_name: 'JUnit Test Report (unit tests)'
+        update_check: true
+        fail_on_failure: false
+        require_tests: false
 
   integration-tests:
     name: Execute integration tests
@@ -74,11 +143,19 @@ jobs:
           MAVEN_EXTRA_OPTS: >-
             -Dopensearch.port=${{ matrix.port }}
             -Delasticsearch.port=${{ matrix.port }}
+        # This job is gated on `needs: unit-tests`, so the unit suite and the Javadoc/checkstyle
+        # validation have already passed on this exact commit. Re-running either here is pure
+        # duplication before the integration tests this job exists for, and the legs run
+        # sequentially (max-parallel: 1), so it costs twice over.
+        #   --skip-unit-tests activates the skip-unit-tests profile, which sets surefire's skip
+        #     only: failsafe, and therefore the ITs, still run.
+        #   --no-javadoc drops the two extra full-reactor invocations --ci adds
+        #     (javadoc:javadoc and javadoc-tags-warn checkstyle:check).
         run: |
           if [ "${{ matrix.search-engine }}" = "opensearch" ]; then
-            ./build.sh --ci --integration-tests --use-opensearch
+            ./build.sh --ci --integration-tests --skip-unit-tests --no-javadoc --use-opensearch
           else
-            ./build.sh --ci --integration-tests
+            ./build.sh --ci --integration-tests --skip-unit-tests --no-javadoc
           fi
       # Keep only third-party dependencies in the post-job Maven cache: Unomi's own
       # snapshots are rebuilt every run and would only bloat the cache / risk staleness

api/src/main/java/org/apache/unomi/api/tasks/ScheduledTask.java

@@ -16,6 +16,7 @@
  */
 package org.apache.unomi.api.tasks;
 
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import org.apache.unomi.api.Item;
 
 import java.io.Serializable;
@@ -40,6 +41,11 @@
  * @see org.apache.unomi.api.services.SchedulerService
  * @see TaskExecutor
  */
+// Tolerate unknown properties so a node running THIS version can still deserialize task
+// documents written by a NEWER version that has added fields (rolling upgrade window).
+// Without this, Jackson's default rejects the first unrecognized field and the older node
+// loses access to all scheduler state until it is upgraded.
+@JsonIgnoreProperties(ignoreUnknown = true)
 public class ScheduledTask extends Item implements Serializable {
 
     /**
@@ -86,6 +92,7 @@ public enum TaskStatus {
     private boolean enabled;
     private String lockOwner;
     private Date lockDate;
+    private long lockLeaseMillis;
     private boolean oneShot;
     private boolean allowParallelExecution;
     private TaskStatus status;
@@ -343,13 +350,40 @@ public Date getLockDate() {
 
     /**
      * Sets the date when the current lock was acquired.
-     * 
+     *
      * @param lockDate the lock acquisition date
      */
     public void setLockDate(Date lockDate) {
         this.lockDate = lockDate;
     }
 
+    /**
+     * Duration in milliseconds for which the current lock is valid, as declared by the node that
+     * acquired or last renewed it.
+     * <p>
+     * A lock's lifetime is a lease granted by its <em>owner</em>: the owner renews it on a cadence
+     * derived from its own configured lock timeout, so only the owner's timeout describes when a
+     * missing renewal actually means the owner is dead. Observers must judge expiry against this
+     * recorded lease, never against their own configured timeout — a node configured with a shorter
+     * timeout than the owner's renewal cadence would otherwise "recover" a lock whose owner is alive
+     * and mid-execution, and the task would run twice.
+     *
+     * @return the lease duration in milliseconds, or {@code 0} when the lock predates lease
+     *         recording (legacy documents) and the observer's own timeout is the only guide
+     */
+    public long getLockLeaseMillis() {
+        return lockLeaseMillis;
+    }
+
+    /**
+     * Sets the lease duration granted with the current lock.
+     *
+     * @param lockLeaseMillis the lease duration in milliseconds, {@code 0} when unlocked or unknown
+     */
+    public void setLockLeaseMillis(long lockLeaseMillis) {
+        this.lockLeaseMillis = lockLeaseMillis;
+    }
+
     /**
      * Determines whether this task should execute only once.
      * Tasks with period=0 are automatically marked as one-shot tasks.

build.sh

@@ -279,8 +279,11 @@ RESOLVER_DEBUG=false
 KEEP_CONTAINER=false
 IT_SEARCH_ENGINE_LOGS=false
 IT_MEMORY_SAMPLER=true
-IT_MEMORY_INTERVAL=30
+# 10s, matching itests/sample-it-memory.sh: at 30s a single sample spans several ITs, so a
+# stall cannot be attributed to the test that caused it.
+IT_MEMORY_INTERVAL=10
 JAVADOC=false
+NO_JAVADOC=false
 LOG_FILE=""
 LOG_FILE_ONLY=false
 
@@ -327,8 +330,9 @@ EOF
         echo -e "  ${CYAN}--keep-container${NC}           Keep search engine container running after tests (for post-failure inspection)"
         echo -e "  ${CYAN}--search-engine-logs${NC}       Stream search engine Docker logs to the Maven console during integration tests"
         echo -e "  ${CYAN}--no-memory-sampler${NC}        Disable JVM/system memory sampling during integration tests"
-        echo -e "  ${CYAN}--memory-interval SEC${NC}    Memory sample interval in seconds (default: 30)"
+        echo -e "  ${CYAN}--memory-interval SEC${NC}    Memory sample interval in seconds (default: 10)"
         echo -e "  ${CYAN}--javadoc${NC}                  Build and validate Javadoc after install (doclint errors fail; public/protected tag gaps warn)"
+        echo -e "  ${CYAN}--no-javadoc${NC}               Skip Javadoc/checkstyle validation (overrides --ci; use when another job already ran it)"
         echo -e "  ${CYAN}--ci${NC}                       CI mode: no Karaf, non-interactive, includes Javadoc"
         echo -e "  ${CYAN}--log-file PATH${NC}            Tee all output to PATH (console + file)"
         echo -e "  ${CYAN}--log-file-only${NC}            With --log-file: write to file only, suppress console"
@@ -371,8 +375,9 @@ EOF
         echo "  --keep-container          Keep search engine container running after tests (for post-failure inspection)"
         echo "  --search-engine-logs      Stream search engine Docker logs to the Maven console during integration tests"
         echo "  --no-memory-sampler       Disable JVM/system memory sampling during integration tests"
-        echo "  --memory-interval SEC     Memory sample interval in seconds (default: 30)"
+        echo "  --memory-interval SEC     Memory sample interval in seconds (default: 10)"
         echo "  --javadoc                 Build and validate Javadoc after install (doclint errors fail; public/protected tag gaps warn)"
+        echo "  --no-javadoc              Skip Javadoc/checkstyle validation (overrides --ci; use when another job already ran it)"
         echo "  --ci                      CI mode: no Karaf, non-interactive, includes Javadoc"
         echo "  --log-file PATH           Tee all output to PATH (console + file)"
         echo "  --log-file-only           With --log-file: write to file only, suppress console"
@@ -549,6 +554,11 @@ while [ "$1" != "" ]; do
         --javadoc)
             JAVADOC=true
             ;;
+        --no-javadoc)
+            # Explicit veto, applied after argument parsing so it wins regardless of whether it
+            # appears before or after --ci (which turns Javadoc on).
+            NO_JAVADOC=true
+            ;;
         --log-file)
             shift
             LOG_FILE="$1"
@@ -1167,6 +1177,12 @@ echo "Estimated time: 3-5 minutes for build, 50-60 minutes with integration test
 start_timer
 
 # Build phases with enhanced output
+# Apply the --no-javadoc veto now that all arguments are parsed, so it wins over --ci
+# regardless of flag order.
+if [ "$NO_JAVADOC" = true ]; then
+    JAVADOC=false
+fi
+
 [ "$JAVADOC" = true ] && total_steps=4 || total_steps=2
 current_step=0