Uploaded image for project: 'Dev - Nexus Repo'
  1. Dev - Nexus Repo
  2. NEXUS-7789

Scheduler improvements

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0-m3
    • Component/s: Scheduled Tasks
    • Labels:
      None
    • Sprint:
      Sprint 31, Sprint 32

      Description

      Second round as NEXUS-7292 received reviews, comments, suggestions and we saw problems on CI.

      Such as:
      Some feedback to consider after this is merged:

      • I agree that enum in weekly schedule would be better rather than a series of constants
      • also remember that enums can have methods, which you can use to replace switch-style behaviour (ie. you just call `enumValue.someMethod(someArg)`, which can have different implementations per value)
      • in the H2 settings prefer FILE_LOCK=FS over FILE_LOCK=SOCKET - note when you do this you also need to update the lock recovery code - see this example in the [nexus-nuget-plugin](https://github.com/sonatype/nexus-pro/blob/master/plugins/other/nexus-nuget-plugin/src/main/java/com/sonatype/nexus/plugins/nuget/odata/H2ODataConnection.java#L83) which now uses the FS lock
      • you can use `UUID.randomUUID().toString()` to generate unique ids instead of using `nanoTime`
      • use of sentinel integer `LAST_DAY_OF_MONTH` in Monthly schedule is a bit messy
      • what was the reasoning behind having `RunState` and `EndState` separate from `State`?
      • why did `nextRun` become `getNextRun` but other model fields stayed the same? (such as `lastRun`)

      Minor/nitpicking comments:

      • `CancelableSupport` mentions `TaskSupport.isCanceled()` is also available for tasks, but all the tasks I saw just used `CancelableSupport.checkCancellation()`. Does this additional method pull enough weight?
      • prefer `@VisibleForTesting` over comments when exposing methods for testing
      • misplaced TODO SLEEPING>RUNNING comment in Nexus4066TaskMutualExclusionIT-
      • `setToList`/`listToSet` looks more like `setToCSV`/`csvToSet`
      • still a couple of references to `nexusTaskExecutor` which should be `nexusTaskScheduler`
      • some methods use `taskType` as the parameter name, when it's really a `taskId`
      • I also saw `taskInstance` used as a variable name when it was actually a `taskConfiguration`
      • I spotted some dangling imports which aren't needed anymore

      And more (Alin)

      • extract scheduler API from nexus-core as nexus-sheduler-api and make that a dependency of nexus-core. There are a few places that use classes from core as:
        • the events that extend from AbstractEvent/Event but the events do not need to depend on those
        • RepositoryTaskSupport remains in core as this is not about api
        • NexusTaskFailureAlertEmailSender remains in core as this is not about api

      And more (from CI) and Stuart and Alin:

      • fix non-fatal NPE seen during startup caused by various components (ie. HealthCheckTaskManager) trying to access Tasks before Scheduler capability is available, Quartz is created -> NPE (is NEXUS-7786)

        Attachments

          Activity

            People

            Assignee:
            jhesse Jeffry Hesse
            Reporter:
            cstamas Tamás Cservenák
            Last Updated By:
            Peter Lynch Peter Lynch
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:
              Date of First Response:

                tigCommentSecurity.panel-title