Deadline is removed after task processing, allowing follow-up databas…

…e operations to succeed in any case. (#152) * Deadline is removed after task processing, allowing follow-up database operations to succeed in any case.
transferwise · Apr 6, 2022 · 1cd9fbf · 1cd9fbf
1 parent bcd14c7
commit 1cd9fbf
Show file tree

Hide file tree

Showing 19 changed files with 197 additions and 91 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -8,14 +8,27 @@ on:
     branches:
       - master
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: ${{ github.ref != 'refs/heads/master' }}
+
 jobs:
-  build:
-    name: "Build and Test"
+  matrix_build:
+    name: "Matrix Build"
     runs-on:
       - ubuntu-latest
+    strategy:
+      fail-fast: false
+      max-parallel: 100
+      matrix:
+        spring_boot_version:
+          - 2.6.5
+          - 2.5.11
+          - 2.4.13
     env:
+      SPRING_BOOT_VERSION: ${{ matrix.spring_boot_version }}
       GRADLE_OPTS: "-Djava.security.egd=file:/dev/./urandom -Dorg.gradle.parallel=true"
-      IN_CI: true
+      RUNS_IN_CI: true
       MARIADB_TCP_3306: 3306
       MARIADB_TCP_HOST: mysql1
       KAFKA_TCP_9092: 9092
@@ -69,41 +82,56 @@ jobs:
         with:
           path: |
             ~/.gradle
-          key: gradle-v1-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties', '**/*.gradle*') }}
+          key: gradle-v1-${{ matrix.spring_boot_version }}-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties', '**/*.gradle*') }}
       - name: "Assemble jar"
-        run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew assemble --console=plain --stacktrace
+        run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew assemble --console=plain --info --stacktrace
+      ##- name: "Run checks other than tests"
+      ##  run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew -Dspring.profiles.include=continuous-integration check -x test --console=plain --info --stacktrace -Dorg.gradle.parallel=true
       - name: "Run tests"
-        run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew -Dspring.profiles.include=continuous-integration check --console=plain --stacktrace
+        # We will not run tests in parallel, so that the test output is easily understandable
+        run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew -Dspring.profiles.include=continuous-integration test --console=plain --info --stacktrace -Dorg.gradle.parallel=false
       - name: "Test if publishing works"
-        run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew publishToMavenLocal --console=plain --stacktrace
+        run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew publishToMavenLocal --console=plain --info --stacktrace
       - name: "Publish Test Report"
         uses: mikepenz/action-junit-report@v2
         if: always()
         with:
+          check_name: Test Report-(${{ matrix.spring_boot_version }})
           report_paths: '**/build/test-results/**/*.xml'
           github_token: ${{ secrets.GITHUB_TOKEN }}
           require_tests: true
       - name: Publish checkstyle report
         if: always()
         uses: jwgmeligmeyling/checkstyle-github-action@master
         with:
+          name: Checkstyle Report-(${{ matrix.spring_boot_version }})
           path: '**/build/reports/**/*.xml'
       - name: Publish spotbugs report
         if: failure()
         uses: jwgmeligmeyling/spotbugs-github-action@master
         with:
+          name: Spotbugs Report-(${{ matrix.spring_boot_version }})
           path: '**/build/reports/**/*.xml'
       - name: "Collect test reports"
         run: |
-          tar -zcvf all-test-reports.tar.gz **/build/reports
+          tar -zcvf all-test-reports-${{ matrix.spring_boot_version }}.tar.gz **/build/reports
         if: always()
       - name: "Store test results"
         uses: actions/upload-artifact@v2
         if: always()
         with:
-          name: all-test-reports
-          path: all-test-reports.tar.gz
+          name: all-test-reports-${{ matrix.spring_boot_version }}
+          path: all-test-reports-${{ matrix.spring_boot_version }}.tar.gz
           retention-days: 7
       - name: "Tag release"
         if: github.ref == 'refs/heads/master'
-        run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew tagRelease --console=plain --no-daemon
+        run: GRADLE_USER_HOME=$HOME/.gradle ./gradlew tagRelease --console=plain --info --stacktrace
+
+  build:
+    name: "Build and Test"
+    runs-on:
+      - ubuntu-latest
+    needs: matrix_build
+    steps:
+      - name: "Do something"
+        run: echo 'Matrix build was successful'
diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres
 to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+#### 1.33.0 - 2022/04/05
+
+### Fixed
+
+* Deadline is removed after task processing, allowing follow-up database operations to succeed in any case.
+  Example case: task processing threw `DeadlineExceededException` and asking retry time threw it again.
+
 #### 1.32.1 - 2021/01/05
 
 ### Fixed

diff --git a/build.common.gradle b/build.common.gradle
@@ -15,16 +15,52 @@ repositories {
     mavenLocal()
 }
 
+configurations {
+    all {
+        exclude(group: 'junit', module: 'junit')
+        exclude(group: 'org.junit.vintage', module: 'junit-vintage-engine')
+
+        resolutionStrategy {
+            failOnVersionConflict()
+            preferProjectModules()
+            if (System.getenv("RUNS_IN_CI") == "true") {
+                // This is faster, than using Gradle's `--refresh-dependencies`, which will refresh also all non-dynamic things.
+                cacheDynamicVersionsFor 10, 'minutes'
+                cacheChangingModulesFor 10, 'minutes'
+            }
+        }
+    }
+
+    local {
+        canBeResolved(false)
+        canBeConsumed(false)
+    }
+    compileClasspath {
+        extendsFrom(local)
+    }
+    runtimeClasspath {
+        extendsFrom(local)
+    }
+    testCompileClasspath {
+        extendsFrom(local)
+    }
+    testRuntimeClasspath {
+        extendsFrom(local)
+    }
+    annotationProcessor {
+        extendsFrom(local)
+    }
+    testAnnotationProcessor {
+        extendsFrom(local)
+    }
+}
+
 dependencies {
-    implementation platform(libraries.springBootDependencies)
-    annotationProcessor platform(libraries.springBootDependencies)
-    testAnnotationProcessor platform(libraries.springBootDependencies)
-    compileOnly platform(libraries.springBootDependencies)
+    local platform(libraries.springBootDependencies + '!!')
+    local libraries.lombok
 
-    annotationProcessor libraries.lombok
     annotationProcessor libraries.springBootConfigurationProcessor
 
-    compileOnly libraries.lombok
     compileOnly libraries.spotbugsAnnotations
     compileOnly libraries.jakartaValidationApi
 
@@ -34,26 +70,15 @@ dependencies {
     implementation libraries.guava
     implementation libraries.twBaseUtils
 
-    testAnnotationProcessor libraries.lombok
-
-    testCompileOnly libraries.lombok
     testCompileOnly libraries.spotbugsAnnotations
 
-    testImplementation libraries.lombok
     testImplementation libraries.junitApi
     testImplementation libraries.junitEngine
     testImplementation libraries.junitParams
     testImplementation libraries.junitMockito
     testImplementation libraries.assertJCore
 }
 
-configurations {
-    all {
-        exclude(group: 'junit', module: 'junit')
-        exclude(group: 'org.junit.vintage', module: 'junit-vintage-engine')
-    }
-}
-
 java {
     sourceCompatibility = JavaVersion.VERSION_11
     targetCompatibility = JavaVersion.VERSION_11

diff --git a/build.gradle b/build.gradle
@@ -2,14 +2,17 @@ import org.eclipse.jgit.api.errors.RefAlreadyExistsException
 
 buildscript {
     dependencies {
-        classpath "com.avast.gradle:gradle-docker-compose-plugin:0.14.12"
+        classpath "com.avast.gradle:gradle-docker-compose-plugin:0.15.2"
+    }
+
+    dependencies {
+        classpath("org.springframework.boot:spring-boot-gradle-plugin:${System.getenv("SPRING_BOOT_VERSION") ?: '2.5.11'}")
     }
 }
 
 plugins {
-    id "com.github.spotbugs" version "5.0.3" apply false
+    id "com.github.spotbugs" version "5.0.6" apply false
     id "idea"
-    id 'org.springframework.boot' version '2.5.8' apply false
     id 'org.ajoberstar.grgit' version '4.1.1'
     id 'io.github.gradle-nexus.publish-plugin' version "1.1.0"
 }

diff --git a/build.libraries.gradle b/build.libraries.gradle
@@ -1,30 +1,29 @@
 ext {
-    twContextVersion = "0.11.0"
+    twContextVersion = "0.11.1"
     twLeaderSelectorVersion = "1.6.0"
-    springBootVersion = "2.5.8"
-
-
     libraries = [
             // version defined
-            springBootDependencies          : "org.springframework.boot:spring-boot-dependencies:${springBootVersion}",
-            apacheCuratorRecipies           : "org.apache.curator:curator-recipes:5.2.0",
+            awaitility                      : 'org.awaitility:awaitility:4.2.0',
+            gafferJta                       : 'com.transferwise.common:tw-gaffer-jta:1.4.0',
+            apacheCuratorRecipies           : "org.apache.curator:curator-recipes:5.2.1",
             apacheCommonsCollections        : "org.apache.commons:commons-collections4:4.4",
             commonsIo                       : "commons-io:commons-io:2.11.0",
-            guava                           : "com.google.guava:guava:31.0.1-jre",
+            guava                           : "com.google.guava:guava:31.1-jre",
+            newRelic                        : "com.newrelic.agent.java:newrelic-api:7.6.0",
             semver4j                        : "com.vdurmont:semver4j:3.1.0",
+            spotbugsAnnotations             : "com.github.spotbugs:spotbugs-annotations:${spotbugs.toolVersion.get()}",
+            springBootDependencies          : "org.springframework.boot:spring-boot-dependencies:${System.getenv("SPRING_BOOT_VERSION") ?: '2.5.11'}",
             twGracefulShutdown              : "com.transferwise.common:tw-graceful-shutdown:2.3.0",
             twGracefulShutdownIntefaces     : "com.transferwise.common:tw-graceful-shutdown-interfaces:2.3.0",
             twContext                       : "com.transferwise.common:tw-context:${twContextVersion}",
             twContextStarter                : "com.transferwise.common:tw-context-starter:${twContextVersion}",
-            twIncidents                     : 'com.transferwise.common:tw-incidents:1.1.0',
+            twIncidents                     : 'com.transferwise.common:tw-incidents:1.1.1',
             twLeaderSelector                : "com.transferwise.common:tw-leader-selector:${twLeaderSelectorVersion}",
             twLeaderSelectorStarter         : "com.transferwise.common:tw-leader-selector-starter:${twLeaderSelectorVersion}",
-            twBaseUtils                     : "com.transferwise.common:tw-base-utils:1.5.0",
-            spotbugsAnnotations             : "com.github.spotbugs:spotbugs-annotations:4.5.0",
-            newRelic                        : "com.newrelic.agent.java:newrelic-api:7.4.0",
-            awaitility                      : 'org.awaitility:awaitility:4.1.1',
-            gafferJta                       : 'com.transferwise.common:tw-gaffer-jta:1.4.0',
-            twEntryPointsStarter            : 'com.transferwise.common:tw-entrypoints-starter:2.5.0',
+            twBaseUtils                     : "com.transferwise.common:tw-base-utils:1.7.1",
+            twEntryPointsStarter            : 'com.transferwise.common:tw-entrypoints-starter:2.7.1',
+            lubenZstd                       : 'com.github.luben:zstd-jni:1.5.0-2',
+            lz4Java                         : 'org.lz4:lz4-java:1.7.1',
 
             // versions managed by spring-boot-dependencies platform
             flywayCore                      : "org.flywaydb:flyway-core",

diff --git a/build.publish.gradle b/build.publish.gradle
@@ -15,6 +15,19 @@ publishing {
                 artifactId = projectArtifactName
             }
 
+            /*
+             This ensures that libraries will have explicit dependency versions in their Maven POM and Gradle module files, so that there would be less
+             ambiguity and less chances of dependency conflicts.
+            */
+            versionMapping {
+                usage('java-api') {
+                    fromResolutionOf('runtimeClasspath')
+                }
+                usage('java-runtime') {
+                    fromResolutionOf('runtimeClasspath')
+                }
+            }
+
             pom {
                 name = projectName
                 description = projectDescription

diff --git a/demoapp/build.gradle b/demoapp/build.gradle
@@ -40,7 +40,7 @@ dependencies {
 }
 
 springBoot {
-    mainClassName = 'com.transferwise.tasks.demoapp.Application'
+    mainClass = 'com.transferwise.tasks.demoapp.Application'
 }
 
 bootRun {

diff --git a/docs/db-perf-tests.md b/docs/db-perf-tests.md
@@ -64,11 +64,11 @@ Comment out `@Disabled` annotation in `integration-tests/src/test/java/com/trans
 
 Run
 ```shell
-IN_CI=true ./gradlew :integration-tests:test --tests "com.transferwise.tasks.demoapp.DemoAppRealTest.dbPerfTest"
+RUNS_IN_CI=true ./gradlew :integration-tests:test --tests "com.transferwise.tasks.demoapp.DemoAppRealTest.dbPerfTest"
 ```
 
 > This will basically run 1 million tasks through, executing `com.transferwise.tasks.demoapp.DemoAppRealTest.dbPerfTest`.
-> "IN_CI=true" prevents creating another local docker environment used for other local tests.
+> "RUNS_IN_CI=true" prevents creating another local docker environment used for other local tests.
 
 >You may want to truncate following tables at the beginning of each test to get more comparable results.
 >```postgresql

diff --git a/gradle.properties b/gradle.properties
@@ -1,2 +1,2 @@
-version=1.32.1
+version=1.33.0
 org.gradle.internal.http.socketTimeout=120000
diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-7.3.3-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-7.4.2-bin.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
diff --git a/integration-tests/build.gradle b/integration-tests/build.gradle
@@ -40,7 +40,7 @@ test {
 }
 
 
-if (!Boolean.parseBoolean(System.getenv('IN_CI'))) {
+if (!Boolean.parseBoolean(System.getenv('RUNS_IN_CI'))) {
     dockerCompose.isRequiredBy(test)
 }
 

diff --git a/integration-tests/src/test/java/com/transferwise/tasks/testapp/config/TestConfiguration.java b/integration-tests/src/test/java/com/transferwise/tasks/testapp/config/TestConfiguration.java
@@ -158,7 +158,7 @@ public IKafkaListenerConsumerPropertiesProvider twTasksKafkaListenerSpringKafkaC
 
       // Having a separate group id can greatly reduce Kafka re-balancing times for tests.
       props.put(CommonClientConfigs.GROUP_ID_CONFIG, props.get(CommonClientConfigs.GROUP_ID_CONFIG) + "kafka-listener-" + shard);
-      props.put(ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG, CooperativeStickyAssignor.class.getName() + "," + RangeAssignor.class.getName());
+      props.put(ConsumerConfig.PARTITION_ASSIGNMENT_STRATEGY_CONFIG, CooperativeStickyAssignor.class.getName());
 
       return props;
     };

diff --git a/tw-tasks-core/build.gradle b/tw-tasks-core/build.gradle
@@ -21,6 +21,8 @@ dependencies {
     implementation libraries.semver4j
     implementation libraries.jakartaValidationApi
     implementation libraries.kafkaClients
+    implementation libraries.lubenZstd
+    implementation libraries.lz4Java
 
     // TODO: avoid ioc related code or spring boot code in core
     implementation libraries.springTx

diff --git a/tw-tasks-core/src/main/java/com/transferwise/tasks/TasksService.java b/tw-tasks-core/src/main/java/com/transferwise/tasks/TasksService.java
@@ -231,9 +231,6 @@ public boolean canShutdown() {
   private void doTriggerTask(BaseTask task) {
     activeAfterCommitTasks.incrementAndGet();
     try {
-      // TODO: If trigger would not need a database connection or at least no new transaction, deadlock situation could be prevented.
-      // TODO: Do we want to mark NEW tasks immediately as SUBMITTED and then even remove the separate SUBMITTED state?
-      // TODO: We probably still need SUBMITTED to separate NEW and WAITING.
       tasksExecutionTriggerer.trigger(task);
       if (log.isDebugEnabled()) {
         log.debug("Task {} triggered. AfterCommit queue size is {}.", LogUtils.asParameter(task.getVersionId()), inProgressAfterCommitTasks.get());
@@ -267,6 +264,11 @@ public void afterCommit() {
     }
   }
 
+  /**
+   * This system is not required for happy flows anymore, because we do not change database then, task is already in SUBMITTED state.
+   *
+   * <p>However, for unhappy case, the task can move to ERROR and we need to modify database for that.
+   */
   @RequiredArgsConstructor
   private class AsynchronouslyTriggerTaskTxSyncAdapter extends TransactionSynchronizationAdapter {
-Original file line number
+Diff line change
@@ Expand Up / @@ -40,7 +40,7 @@ test { @@
     }
-    if (!Boolean.parseBoolean(System.getenv('IN_CI'))) {
+    if (!Boolean.parseBoolean(System.getenv('RUNS_IN_CI'))) {
         dockerCompose.isRequiredBy(test)
     }
@@ Expand Down @@