Only warn when JobFilter is too slow (#961)

jobrunr · Mar 15, 2024 · e16fcd0 · e16fcd0
1 parent 4f4e6fc
commit e16fcd0
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 62 deletions.
diff --git a/.idea/compiler.xml b/.idea/compiler.xml
diff --git a/.idea/misc.xml b/.idea/misc.xml
diff --git a/.idea/modules.xml b/.idea/modules.xml
diff --git a/...t/JobRunr.framework-support.jobrunr-quarkus-extension.quarkus-jobrunr-deployment.main.iml b/...t/JobRunr.framework-support.jobrunr-quarkus-extension.quarkus-jobrunr-deployment.main.iml
diff --git a/...rkus-jobrunr/JobRunr.framework-support.jobrunr-quarkus-extension.quarkus-jobrunr.main.iml b/...rkus-jobrunr/JobRunr.framework-support.jobrunr-quarkus-extension.quarkus-jobrunr.main.iml
diff --git a/core/src/main/java/org/jobrunr/jobs/filters/AbstractJobFilters.java b/core/src/main/java/org/jobrunr/jobs/filters/AbstractJobFilters.java
@@ -10,9 +10,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 import java.util.stream.Stream;
 
@@ -22,7 +20,6 @@
 import static org.jobrunr.utils.reflection.ReflectionUtils.newInstanceCE;
 
 public abstract class AbstractJobFilters {
- private static final Map<Class<? extends JobFilter>, Integer> slowJobFilters = new ConcurrentHashMap<>();
  protected final AbstractJob job;
  private final List<JobFilter> jobFilters;
 
@@ -32,7 +29,7 @@ protected AbstractJobFilters(AbstractJob job, JobDefaultFilters jobDefaultFilter
  }
 
  protected List<JobFilter> jobFilters() {
- return jobFilters.stream().filter(this::isFastJobFilter).collect(toList());
+ return jobFilters;
  }
 
  protected List<JobFilter> initJobFilters(AbstractJob job, List<JobFilter> jobFilters) {
@@ -118,27 +115,8 @@ final <T extends JobFilter> Consumer<T> catchThrowable(Consumer<T> consumer) {
  }
 
  final void logJobFilterTime(JobFilter jobFilter, long durationInNanos) {
- Class<? extends JobFilter> jobFilterClass = jobFilter.getClass();
- if(NANOSECONDS.toMillis(durationInNanos) > 5) {
- slowJobFilters.compute(jobFilterClass, (f, i) -> {
- if(i == null || DefaultJobFilter.class.equals(jobFilterClass) || RetryFilter.class.equals(jobFilterClass)) {
- return 0;
- } else if(i > 3) {
- return 5;
- } else {
- return i + 1;
- }
- });
- } else {
- slowJobFilters.put(jobFilterClass, 0);
- }
- }
-
- private boolean isFastJobFilter(JobFilter jobFilter) {
- boolean isSlowJobFilter = slowJobFilters.getOrDefault(jobFilter.getClass(), -1) > 3;
- if(isSlowJobFilter) {
+ if (NANOSECONDS.toMillis(durationInNanos) > 5) {
  getLogger().warn("JobFilter of type '{}' is skipped because its slow performance (> 5ms) negatively impacts the overall functioning of JobRunr. JobRunr Pro can run slow running Job Filters without a negative performance impact.", jobFilter.getClass().getName());
  }
- return !isSlowJobFilter;
  }
 }
diff --git a/core/src/test/java/org/jobrunr/jobs/filters/AbstractJobFiltersTest.java b/core/src/test/java/org/jobrunr/jobs/filters/AbstractJobFiltersTest.java
@@ -3,24 +3,16 @@
 import ch.qos.logback.LoggerAssert;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.read.ListAppender;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 
-import java.util.Map;
-
+import static java.util.Collections.emptyMap;
 import static org.jobrunr.JobRunrAssertions.assertThat;
 import static org.jobrunr.jobs.JobTestBuilder.anEnqueuedJob;
-import static org.mockito.internal.util.reflection.Whitebox.getInternalState;
 
 class AbstractJobFiltersTest {
 
- @AfterEach
- void resetSlowFilters() {
- ((Map)getInternalState(AbstractJobFilters.class, "slowJobFilters")).clear();
- }
-
  @Test
- void ifJobFilterIsTooSlowItIsExpelledAndMessageIsLogged() {
+ void ifJobFilterIsTooSlowAMessageIsLogged() {
  MyJobFilter myJobFilter = new MyJobFilter();
  JobCreationFilters jobCreationFilters = new JobCreationFilters(anEnqueuedJob().build(), new JobDefaultFilters(myJobFilter));
  final ListAppender<ILoggingEvent> logger = LoggerAssert.initFor(jobCreationFilters);
@@ -31,24 +23,27 @@ void ifJobFilterIsTooSlowItIsExpelledAndMessageIsLogged() {
  jobCreationFilters.logJobFilterTime(myJobFilter, 6000000);
  jobCreationFilters.logJobFilterTime(myJobFilter, 6000000);
 
- assertThat(jobCreationFilters.jobFilters()).doesNotHaveAnyElementsOfTypes(MyJobFilter.class);
- assertThat(logger).hasWarningMessageContaining("JobFilter of type 'org.jobrunr.jobs.filters.AbstractJobFiltersTest$MyJobFilter' is skipped because its slow performance (> 5ms) negatively impacts the overall functioning of JobRunr");
+ assertThat(logger).hasWarningMessageContaining(
+ "JobFilter of type 'org.jobrunr.jobs.filters.AbstractJobFiltersTest$MyJobFilter' is skipped because its slow performance (> 5ms) negatively impacts the overall functioning of JobRunr",
+ 5,
+ emptyMap()
+ );
  }
 
  @Test
- void ifJobFilterIsTemporarySlowButThenFastAgainItIsNotExpelled() {
+ void ifJobFilterIsNotSlowNoMessageIsLogged() {
  MyJobFilter myJobFilter = new MyJobFilter();
  JobCreationFilters jobCreationFilters = new JobCreationFilters(anEnqueuedJob().build(), new JobDefaultFilters(myJobFilter));
+ final ListAppender<ILoggingEvent> logger = LoggerAssert.initFor(jobCreationFilters);
 
- jobCreationFilters.logJobFilterTime(myJobFilter, 6000000);
- jobCreationFilters.logJobFilterTime(myJobFilter, 6000000);
- jobCreationFilters.logJobFilterTime(myJobFilter, 6000000);
- jobCreationFilters.logJobFilterTime(myJobFilter, 6000000);
  jobCreationFilters.logJobFilterTime(myJobFilter, 1000000);
- jobCreationFilters.logJobFilterTime(myJobFilter, 6000000);
+ jobCreationFilters.logJobFilterTime(myJobFilter, 1000000);
+ jobCreationFilters.logJobFilterTime(myJobFilter, 1000000);
 
  assertThat(jobCreationFilters.jobFilters())
  .hasAtLeastOneElementOfType(MyJobFilter.class);
+
+ assertThat(logger).hasNoWarnLogMessages();
  }
 
  public static class MyJobFilter implements JobServerFilter {