Merge branch 'main' into fcHealth

m-nagarajan · Aug 11, 2023 · 7b978f5 · 7b978f5
2 parents e310537 + 6632a2c
commit 7b978f5
Show file tree

Hide file tree

Showing 437 changed files with 10,058 additions and 5,543 deletions.
diff --git a/.github/workflows/gh-ci-pulsar-tests.yml b/.github/workflows/gh-ci-pulsar-tests.yml
@@ -4,22 +4,6 @@ name: PulsarVeniceIntegrationCI
 on: [push, pull_request, workflow_dispatch]
 
 jobs:
-  ValidateGradleWrapper:
-    strategy:
-      fail-fast: false
-      matrix:
-        os: [ubuntu-latest]
-    runs-on: ${{ matrix.os }}
-    timeout-minutes: 5
-    steps:
-      - uses: actions/checkout@v3
-        with:
-          fetch-depth: 0
-      - name: Validate Gradle wrapper
-        uses: gradle/wrapper-validation-action@v1
-      - name: Set up Docker
-        uses: crazy-max/ghaction-setup-docker@v1
-
   PulsarVeniceIntegrationTests:
     strategy:
       fail-fast: false
@@ -46,7 +30,7 @@ jobs:
       - name: Setup Gradle
         uses: gradle/gradle-build-action@v2
       - name: Build with gradle
-        run: ./gradlew assemble --stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1
+        run: ./gradlew assemble --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1
 
       - name: Build docker images for Venice (latest-dev tag)
         shell: bash

diff --git a/.github/workflows/gh-ci.yml b/.github/workflows/gh-ci.yml
@@ -53,7 +53,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon clean check --parallel -Pspotallbugs -x test -x integrationTest -x jacocoTestCoverageVerification"
+          arguments: "--continue --no-daemon clean check --parallel -Pspotallbugs -x test -x integrationTest -x jacocoTestCoverageVerification"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -182,7 +182,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestA"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestA"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -225,7 +225,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestB"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestB"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -268,7 +268,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestC"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestC"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -311,7 +311,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestD"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestD"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -354,7 +354,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestE"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestE"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -397,7 +397,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestF"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestF"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -440,7 +440,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestG"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestG"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -483,7 +483,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestH"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestH"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -526,7 +526,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestI"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestI"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -569,7 +569,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestJ"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestJ"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -612,7 +612,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestK"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestK"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -655,7 +655,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestL"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestL"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -698,7 +698,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestM"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestM"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -741,7 +741,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestN"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestN"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -784,7 +784,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestO"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestO"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -827,7 +827,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestP"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestP"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -870,7 +870,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestQ"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestQ"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -913,7 +913,7 @@ jobs:
       - name: Build with Gradle
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestZ"
+          arguments: "--continue --no-daemon -DforkEvery=1 -DmaxParallelForks=1 integrationTestZ"
       - name: Package Build Artifacts
         if: success() || failure()
         shell: bash
@@ -984,7 +984,7 @@ jobs:
         if: steps.check_alpini_files_changed.outputs.alpini == 'true'
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DmaxParallelForks=1 alpiniUnitTest"
+          arguments: "--continue --no-daemon -DmaxParallelForks=1 alpiniUnitTest"
       - name: Package Build Artifacts
         if: steps.check_alpini_files_changed.outputs.alpini == 'true' && (success() || failure())
         shell: bash
@@ -1055,7 +1055,7 @@ jobs:
         if: steps.check_alpini_files_changed.outputs.alpini == 'true'
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: "--stacktrace --continue --no-daemon -DmaxParallelForks=1 alpiniFunctionalTest"
+          arguments: "--continue --no-daemon -DmaxParallelForks=1 alpiniFunctionalTest"
       - name: Package Build Artifacts
         if: steps.check_alpini_files_changed.outputs.alpini == 'true' && (success() || failure())
         shell: bash

diff --git a/build.gradle b/build.gradle
@@ -38,7 +38,7 @@ if (project.hasProperty('overrideBuildEnvironment')) {
   }
 }
 
-def avroVersion = '1.9.2'
+def avroVersion = '1.10.2'
 def avroUtilVersion = '0.2.150'
 def grpcVersion = '1.54.1'
 def kafkaGroup = 'com.linkedin.kafka'
@@ -236,7 +236,6 @@ subprojects {
     }
     compileOnly {
       // These dependencies are transitively used at runtime, so we cannot exclude them further than compileOnly
-      exclude group: 'com.google.guava'
       exclude group: 'com.typesafe.scala-logging'
       exclude group: 'log4j'
       exclude group: 'org.slf4j'
@@ -515,7 +514,7 @@ subprojects {
         }
 
         violationRules {
-          minBranches = project.ext.has('diffCoverageThreshold') ? project.ext.diffCoverageThreshold : 0.33
+          minBranches = project.ext.has('diffCoverageThreshold') ? project.ext.diffCoverageThreshold : 0.40
           failOnViolation = true
         }
       }
@@ -714,22 +713,44 @@ idea.project.ipr {
 ext.createDiffFile = { ->
   // Files that we don't plan to write unit tests for now. Will be worked in the future
   def exclusionFilter = [
-        ':!services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java',
-        ':!services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java',
-        ':!internal/venice-test-common/*',
-        ':!services/venice-server/src/main/java/com/linkedin/venice/server/VeniceServer.java',
-        ':!internal/venice-common/src/main/java/com/linkedin/venice/controllerapi/ControllerClient.java',
-        ':!services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceController.java',
-        ':!services/venice-router/src/main/java/com/linkedin/venice/router/RouterServer.java',
-        ':!services/venice-router/src/main/java/com/linkedin/venice/router/streaming/VeniceChunkedResponse.java',
-        ':!services/venice-standalone/*', // exclude the entire standalone project
-        ':!clients/venice-client/src/main/java/com/linkedin/venice/fastclient/factory/ClientFactory.java',
-        ':!clients/da-vinci-client/src/main/java/com/linkedin/davinci/DaVinciBackend.java',
-        ':!clients/venice-producer/src/main/java/com/linkedin/venice/producer/online/OnlineProducerFactory.java',
-        ':!clients/venice-producer/src/main/java/com/linkedin/venice/producer/online/ProducerTool.java',
-
-        // Other files that have tests but are not executed in the regular unit test task
-        ':!internal/alpini/*'
+      // Keep this sorted
+      // da-vinci-client
+      ':!clients/da-vinci-client/src/main/java/com/linkedin/davinci/DaVinciBackend.java',
+
+      // venice-client
+      ':!clients/venice-client/src/main/java/com/linkedin/venice/fastclient/factory/ClientFactory.java',
+      // unit test for gRPC Transport Client is not straightforward, adding to exclusion list for now
+      ':!clients/venice-client/src/main/java/com/linkedin/venice/fastclient/transport/GrpcTransportClient.java',
+
+      // venice-producer
+      ':!clients/venice-producer/src/main/java/com/linkedin/venice/producer/online/OnlineProducerFactory.java',
+      ':!clients/venice-producer/src/main/java/com/linkedin/venice/producer/online/ProducerTool.java',
+
+      // venice-common
+      ':!internal/venice-common/src/main/java/com/linkedin/venice/controllerapi/ControllerClient.java',
+
+      // venice-test-common
+      ':!internal/venice-test-common/*',
+
+      // venice-controller
+      ':!services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceController.java',
+      ':!services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceControllerService.java',
+      ':!services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java',
+      ':!services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java',
+
+      // venice-router
+      ':!services/venice-router/src/main/java/com/linkedin/venice/router/RouterServer.java',
+      ':!services/venice-router/src/main/java/com/linkedin/venice/router/streaming/VeniceChunkedResponse.java',
+
+      // venice-server
+      ':!services/venice-server/src/main/java/com/linkedin/venice/server/VeniceServer.java',
+
+      // venice-standalone
+      ':!services/venice-standalone/*', // exclude the entire standalone project
+
+      // Keep this last
+      // Other files that have tests but are not executed in the regular unit test task
+      ':!internal/alpini/*'
   ]
   def file = Files.createTempFile(URLEncoder.encode(project.name, 'UTF-8'), '.diff').toFile()
   def diffBase = "${git.getUpstreamRemote()}/main"

diff --git a/clients/da-vinci-client/build.gradle b/clients/da-vinci-client/build.gradle
@@ -31,6 +31,7 @@ dependencies {
   implementation libraries.zkclient // It's necessary to pull in the most recent version of zkclient explicitly, otherwise Helix won't have it...
 
   testImplementation project(':internal:venice-test-common')
+  testImplementation project(':internal:venice-client-common').sourceSets.test.output
   testImplementation libraries.kafkaClientsTest
 }
 

diff --git a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/DaVinciBackend.java b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/DaVinciBackend.java
@@ -41,7 +41,7 @@
 import com.linkedin.venice.meta.StoreDataChangedListener;
 import com.linkedin.venice.meta.SubscriptionBasedReadOnlyStoreRepository;
 import com.linkedin.venice.meta.Version;
-import com.linkedin.venice.pubsub.api.PubSubClientsFactory;
+import com.linkedin.venice.pubsub.PubSubClientsFactory;
 import com.linkedin.venice.pushmonitor.ExecutionStatus;
 import com.linkedin.venice.pushstatushelper.PushStatusStoreWriter;
 import com.linkedin.venice.schema.SchemaReader;

diff --git a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/VersionBackend.java b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/VersionBackend.java
@@ -234,8 +234,8 @@ public GenericRecord compute(
         compressor.get());
 
     return ComputeUtils.computeResult(
-        computeRequestWrapper.getComputeRequestVersion(),
         computeRequestWrapper.getOperations(),
+        computeRequestWrapper.getOperationResultFields(),
         sharedContext,
         reusableValueRecord,
         computeResultSchema);
@@ -258,8 +258,8 @@ public void computeWithKeyPrefixFilter(
           @Override
           public void onRecordReceived(GenericRecord key, GenericRecord value) {
             GenericRecord computeResult = ComputeUtils.computeResult(
-                computeRequestWrapper.getComputeRequestVersion(),
                 computeRequestWrapper.getOperations(),
+                computeRequestWrapper.getOperationResultFields(),
                 sharedContext,
                 value,
                 computeResultSchema);

diff --git a/...s/da-vinci-client/src/main/java/com/linkedin/davinci/client/AvroGenericDaVinciClient.java b/...s/da-vinci-client/src/main/java/com/linkedin/davinci/client/AvroGenericDaVinciClient.java
@@ -389,7 +389,6 @@ private Schema getComputeResultSchema(ComputeRequestWrapper computeRequestWrappe
       ComputeUtils.checkResultSchema(
           computeResultSchema,
           computeRequestWrapper.getValueSchema(),
-          computeRequestWrapper.getComputeRequestVersion(),
           computeRequestWrapper.getOperations());
       computeResultSchemaCache.putIfAbsent(computeResultSchemaStr, computeResultSchema);
     }