From 402d9180a09429dbfd257da6106f6335a10925a0 Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Sun, 3 Sep 2023 10:19:26 +0530 Subject: [PATCH 01/10] HDDS-9233. Investigate whether listStatus() is correctly iterating cache. --- .github/workflows/ci.yml | 289 ++------------------------------------- 1 file changed, 11 insertions(+), 278 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70cb55e4aea4..a647e4981289 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,13 +26,13 @@ jobs: GITHUB_CONTEXT: ${{ toJson(github) }} outputs: basic-checks: ${{ steps.selective-checks.outputs.basic-checks }} - needs-basic-checks: ${{ steps.selective-checks.outputs.needs-basic-checks }} - needs-build: ${{ steps.selective-checks.outputs.needs-build }} - needs-compile: ${{ steps.selective-checks.outputs.needs-compile }} - needs-compose-tests: ${{ steps.selective-checks.outputs.needs-compose-tests }} - needs-dependency-check: ${{ steps.selective-checks.outputs.needs-dependency-check }} - needs-integration-tests: ${{ steps.selective-checks.outputs.needs-integration-tests }} - needs-kubernetes-tests: ${{ steps.selective-checks.outputs.needs-kubernetes-tests }} + needs-basic-checks: false + needs-build: true + needs-compile: false + needs-compose-tests: false + needs-dependency-check: false + needs-integration-tests: true + needs-kubernetes-tests: false steps: - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" uses: actions/checkout@v3 @@ -115,227 +115,15 @@ jobs: rm -rf ~/.m2/repository/org/apache/ozone/hdds* rm -rf ~/.m2/repository/org/apache/ozone/ozone* if: always() - compile: - needs: - - build-info - - build - runs-on: ubuntu-20.04 - timeout-minutes: 30 - if: needs.build-info.outputs.needs-compile == 'true' - strategy: - matrix: - java: [ 11, 17 ] - fail-fast: false - steps: - - name: Download Ozone source tarball - uses: actions/download-artifact@v3 - with: - name: ozone-src - - name: Untar sources - run: | - tar --strip-components 1 -xzvf ozone*-src.tar.gz - - name: Cache for maven dependencies - uses: actions/cache@v3 - with: - path: ~/.m2/repository - key: maven-repo-${{ hashFiles('**/pom.xml') }}-${{ matrix.java }} - restore-keys: | - maven-repo-${{ hashFiles('**/pom.xml') }} - maven-repo- - - name: Setup java - uses: actions/setup-java@v3 - with: - distribution: 'temurin' - java-version: ${{ matrix.java }} - - name: Compile Ozone using Java ${{ matrix.java }} - run: hadoop-ozone/dev-support/checks/build.sh -Dskip.npx -Dskip.installnpx -Djavac.version=${{ matrix.java }} - env: - OZONE_WITH_COVERAGE: false - - name: Delete temporary build artifacts before caching - run: | - #Never cache local artifacts - rm -rf ~/.m2/repository/org/apache/ozone/hdds* - rm -rf ~/.m2/repository/org/apache/ozone/ozone* - if: always() - basic: - needs: - - build-info - runs-on: ubuntu-20.04 - timeout-minutes: 90 - if: needs.build-info.outputs.needs-basic-checks == 'true' - strategy: - matrix: - check: ${{ fromJson(needs.build-info.outputs.basic-checks) }} - fail-fast: false - steps: - - name: Checkout project - uses: actions/checkout@v3 - if: matrix.check != 'bats' - - name: Checkout project with history - uses: actions/checkout@v3 - with: - fetch-depth: 0 - if: matrix.check == 'bats' - - name: Cache for maven dependencies - uses: actions/cache@v3 - with: - path: ~/.m2/repository - key: maven-repo-${{ hashFiles('**/pom.xml') }}-8-${{ matrix.check }} - restore-keys: | - maven-repo-${{ hashFiles('**/pom.xml') }}-8 - maven-repo-${{ hashFiles('**/pom.xml') }} - maven-repo- - if: ${{ !contains('author,bats,docs', matrix.check) }} - - name: Setup java - uses: actions/setup-java@v3 - with: - distribution: 'temurin' - java-version: 8 - - name: Execute tests - run: hadoop-ozone/dev-support/checks/${{ matrix.check }}.sh - - name: Summary of failures - run: cat target/${{ matrix.check }}/summary.txt - if: ${{ !cancelled() }} - - name: Archive build results - uses: actions/upload-artifact@v3 - if: ${{ !cancelled() }} - with: - name: ${{ matrix.check }} - path: target/${{ matrix.check }} - continue-on-error: true - - name: Delete temporary build artifacts before caching - run: | - #Never cache local artifacts - rm -rf ~/.m2/repository/org/apache/ozone/hdds* - rm -rf ~/.m2/repository/org/apache/ozone/ozone* - if: always() - dependency: - needs: - - build-info - - build - runs-on: ubuntu-20.04 - timeout-minutes: 5 - if: needs.build-info.outputs.needs-dependency-check == 'true' - steps: - - name: Checkout project - uses: actions/checkout@v3 - - name: Download compiled Ozone binaries - uses: actions/download-artifact@v3 - with: - name: ozone-bin - - name: Untar binaries - run: | - mkdir dist - tar -C dist --strip-components 1 -xzf ozone*.tar.gz - - name: Execute tests - run: | - export OZONE_DIST_DIR=`pwd`/dist - ./hadoop-ozone/dev-support/checks/dependency.sh - - name: Archive build results - uses: actions/upload-artifact@v3 - if: always() - with: - name: dependency - path: target/dependency - continue-on-error: true - acceptance: - needs: - - build-info - - build - runs-on: ubuntu-20.04 - timeout-minutes: 150 - if: needs.build-info.outputs.needs-compose-tests == 'true' - strategy: - matrix: - suite: - - secure - - unsecure - - compat - - EC - - HA-secure - - HA-unsecure - - MR - - misc - - cert-rotation - fail-fast: false - steps: - - name: Checkout project - uses: actions/checkout@v3 - - name: Download compiled Ozone binaries - uses: actions/download-artifact@v3 - with: - name: ozone-bin - - name: Untar binaries - run: | - mkdir -p hadoop-ozone/dist/target - tar xzvf ozone*.tar.gz -C hadoop-ozone/dist/target - sudo chmod -R a+rwX hadoop-ozone/dist/target - - name: Execute tests - run: | - pushd hadoop-ozone/dist/target/ozone-* - sudo mkdir .aws && sudo chmod 777 .aws && sudo chown 1000 .aws - popd - ./hadoop-ozone/dev-support/checks/acceptance.sh - env: - KEEP_IMAGE: false - OZONE_ACCEPTANCE_SUITE: ${{ matrix.suite }} - OZONE_VOLUME_OWNER: 1000 - - name: Archive build results - uses: actions/upload-artifact@v3 - if: always() - with: - name: acceptance-${{ matrix.suite }} - path: target/acceptance - continue-on-error: true - kubernetes: - needs: - - build-info - - build - runs-on: ubuntu-20.04 - timeout-minutes: 60 - if: needs.build-info.outputs.needs-kubernetes-tests == 'true' - steps: - - name: Checkout project - uses: actions/checkout@v3 - - name: Download compiled Ozone binaries - uses: actions/download-artifact@v3 - with: - name: ozone-bin - - name: Untar binaries - run: | - mkdir -p hadoop-ozone/dist/target - tar xzvf ozone*.tar.gz -C hadoop-ozone/dist/target - - name: Execute tests - run: | - pushd hadoop-ozone/dist/target/ozone-* - sudo mkdir .aws && sudo chmod 777 .aws && sudo chown 1000 .aws - popd - ./hadoop-ozone/dev-support/checks/kubernetes.sh - - name: Archive build results - uses: actions/upload-artifact@v3 - if: always() - with: - name: kubernetes - path: target/kubernetes - continue-on-error: true integration: needs: - build-info runs-on: ubuntu-20.04 - timeout-minutes: 150 + timeout-minutes: 300 if: needs.build-info.outputs.needs-integration-tests == 'true' strategy: matrix: - profile: - - client - - contract - - filesystem - - hdds - - om - - ozone - - scm - - shell - - flaky + profile: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ] fail-fast: false steps: - name: Checkout project @@ -355,11 +143,8 @@ jobs: distribution: 'temurin' java-version: 8 - name: Execute tests - run: hadoop-ozone/dev-support/checks/integration.sh -P${{ matrix.profile }} - if: matrix.profile != 'flaky' - - name: Execute flaky tests - run: hadoop-ozone/dev-support/checks/integration.sh -P${{ matrix.profile }} -Dsurefire.rerunFailingTestsCount=5 -Dsurefire.fork.timeout=3600 - if: matrix.profile == 'flaky' + run: hadoop-ozone/dev-support/checks/integration.sh -Dtest=org.apache.hadoop.fs.ozone.TestOzoneFileSystem,org.apache.hadoop.fs.ozone.TestOzoneFSWithObjectStoreCreate,org.apache.hadoop.fs.ozone.TestOzoneFileSystemWithFSO + if: always() - name: Summary of failures run: cat target/${{ github.job }}/summary.txt if: always() @@ -376,55 +161,3 @@ jobs: rm -rf ~/.m2/repository/org/apache/ozone/hdds* rm -rf ~/.m2/repository/org/apache/ozone/ozone* if: always() - coverage: - runs-on: ubuntu-20.04 - timeout-minutes: 30 - if: github.repository == 'apache/ozone' && github.event_name != 'pull_request' - needs: - - acceptance - - basic - - integration - steps: - - name: Checkout project - uses: actions/checkout@v3 - - name: Cache for maven dependencies - uses: actions/cache@v3 - with: - path: ~/.m2/repository - key: maven-repo-${{ hashFiles('**/pom.xml') }}-8-${{ github.job }} - restore-keys: | - maven-repo-${{ hashFiles('**/pom.xml') }}-8 - maven-repo-${{ hashFiles('**/pom.xml') }} - maven-repo- - - name: Download artifacts - uses: actions/download-artifact@v3 - with: - path: target/artifacts - - name: Untar binaries - run: | - mkdir -p hadoop-ozone/dist/target - tar xzvf target/artifacts/ozone-bin/ozone*.tar.gz -C hadoop-ozone/dist/target - - name: Calculate combined coverage - run: ./hadoop-ozone/dev-support/checks/coverage.sh - - name: Setup java 11 - uses: actions/setup-java@v3 - with: - distribution: 'temurin' - java-version: 11 - - name: Upload coverage to Sonar - run: ./hadoop-ozone/dev-support/checks/sonar.sh - env: - SONAR_TOKEN: ${{ secrets.SONARCLOUD_TOKEN }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - name: Archive build results - uses: actions/upload-artifact@v3 - with: - name: coverage - path: target/coverage - continue-on-error: true - - name: Delete temporary build artifacts before caching - run: | - #Never cache local artifacts - rm -rf ~/.m2/repository/org/apache/ozone/hdds* - rm -rf ~/.m2/repository/org/apache/ozone/ozone* - if: always() From 6c1542b4eddbe49a23fe545a282d005b3b65a367 Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Mon, 4 Sep 2023 15:42:57 +0530 Subject: [PATCH 02/10] HDDS-9233. Investigate whether listStatus() is correctly iterating cache. --- .github/workflows/ci.yml | 116 ++---------------- hadoop-ozone/dev-support/checks/junit.sh | 8 +- .../hadoop/fs/ozone/TestOzoneFileSystem.java | 11 +- .../hadoop/ozone/om/KeyManagerImpl.java | 34 +++-- .../ozone/om/OzoneListStatusHelper.java | 6 +- 5 files changed, 50 insertions(+), 125 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a647e4981289..12751d6bb452 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,107 +20,9 @@ env: MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3 OZONE_WITH_COVERAGE: ${{ github.repository == 'apache/ozone' && github.event_name != 'pull_request' }} jobs: - build-info: + unit: runs-on: ubuntu-20.04 - env: - GITHUB_CONTEXT: ${{ toJson(github) }} - outputs: - basic-checks: ${{ steps.selective-checks.outputs.basic-checks }} - needs-basic-checks: false - needs-build: true - needs-compile: false - needs-compose-tests: false - needs-dependency-check: false - needs-integration-tests: true - needs-kubernetes-tests: false - steps: - - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" - uses: actions/checkout@v3 - with: - persist-credentials: false - - name: Fetch incoming commit ${{ github.sha }} with its parent - uses: actions/checkout@v3 - with: - ref: ${{ github.sha }} - fetch-depth: 2 - persist-credentials: false - if: github.event_name == 'pull_request' - - name: Selective checks - id: selective-checks - env: - PR_LABELS: "${{ toJSON(github.event.pull_request.labels.*.name) }}" - PR_DRAFT: "${{ github.event.pull_request.draft }}" - run: | - if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then - # Run selective checks - dev-support/ci/selective_ci_checks.sh "${GITHUB_SHA}" - else - # Run all checks - dev-support/ci/selective_ci_checks.sh - fi - build: - needs: - - build-info - runs-on: ubuntu-20.04 - timeout-minutes: 45 - if: needs.build-info.outputs.needs-build == 'true' - strategy: - matrix: - java: [ 8 ] - fail-fast: false - steps: - - name: Checkout project - uses: actions/checkout@v3 - - name: Cache for npm dependencies - uses: actions/cache@v3 - with: - path: | - ~/.pnpm-store - **/node_modules - key: ${{ runner.os }}-pnpm-${{ hashFiles('**/pnpm-lock.yaml') }} - restore-keys: | - ${{ runner.os }}-pnpm- - - name: Cache for maven dependencies - uses: actions/cache@v3 - with: - path: ~/.m2/repository - key: maven-repo-${{ hashFiles('**/pom.xml') }}-${{ matrix.java }} - restore-keys: | - maven-repo-${{ hashFiles('**/pom.xml') }} - maven-repo- - - name: Setup java - uses: actions/setup-java@v3 - with: - distribution: 'temurin' - java-version: ${{ matrix.java }} - - name: Run a full build - run: hadoop-ozone/dev-support/checks/build.sh -Pdist -Psrc - - name: Store binaries for tests - uses: actions/upload-artifact@v3 - with: - name: ozone-bin - path: | - hadoop-ozone/dist/target/ozone-*.tar.gz - !hadoop-ozone/dist/target/ozone-*-src.tar.gz - retention-days: 1 - - name: Store source tarball for compilation - uses: actions/upload-artifact@v3 - with: - name: ozone-src - path: hadoop-ozone/dist/target/ozone-*-src.tar.gz - retention-days: 1 - - name: Delete temporary build artifacts before caching - run: | - #Never cache local artifacts - rm -rf ~/.m2/repository/org/apache/ozone/hdds* - rm -rf ~/.m2/repository/org/apache/ozone/ozone* - if: always() - integration: - needs: - - build-info - runs-on: ubuntu-20.04 - timeout-minutes: 300 - if: needs.build-info.outputs.needs-integration-tests == 'true' + timeout-minutes: 150 strategy: matrix: profile: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ] @@ -143,21 +45,25 @@ jobs: distribution: 'temurin' java-version: 8 - name: Execute tests - run: hadoop-ozone/dev-support/checks/integration.sh -Dtest=org.apache.hadoop.fs.ozone.TestOzoneFileSystem,org.apache.hadoop.fs.ozone.TestOzoneFSWithObjectStoreCreate,org.apache.hadoop.fs.ozone.TestOzoneFileSystemWithFSO - if: always() + run: bash hadoop-ozone/dev-support/checks/junit.sh -am -pl :ozone-recon -Dtest=org.apache.hadoop.fs.ozone.TestOzoneFileSystem,org.apache.hadoop.fs.ozone.TestOzoneFSWithObjectStoreCreate,org.apache.hadoop.fs.ozone.TestOzoneFileSystemWithFSO + env: + ITERATIONS: 10 - name: Summary of failures run: cat target/${{ github.job }}/summary.txt if: always() + - name: Number of failures + run: cat target/${{ github.job }}/failures + if: always() - name: Archive build results uses: actions/upload-artifact@v3 if: always() with: - name: it-${{ matrix.profile }} - path: target/integration + name: unit-${{ matrix.profile }} + path: target/unit continue-on-error: true - name: Delete temporary build artifacts before caching run: | #Never cache local artifacts rm -rf ~/.m2/repository/org/apache/ozone/hdds* rm -rf ~/.m2/repository/org/apache/ozone/ozone* - if: always() + if: always() \ No newline at end of file diff --git a/hadoop-ozone/dev-support/checks/junit.sh b/hadoop-ozone/dev-support/checks/junit.sh index 74cefe6a975d..462d96ad37c1 100644 --- a/hadoop-ozone/dev-support/checks/junit.sh +++ b/hadoop-ozone/dev-support/checks/junit.sh @@ -29,7 +29,7 @@ if [[ ${ITERATIONS} -le 0 ]]; then fi export MAVEN_OPTS="-Xmx4096m $MAVEN_OPTS" -MAVEN_OPTIONS='-B -Dskip.npx -Dskip.installnpx -Dnative.lib.tmp.dir=/tmp --no-transfer-progress' +MAVEN_OPTIONS='-B -Dskip.npx -Dskip.installnpx --no-transfer-progress' if [[ "${OZONE_WITH_COVERAGE}" != "true" ]]; then MAVEN_OPTIONS="${MAVEN_OPTIONS} -Djacoco.skip" @@ -41,7 +41,7 @@ else MAVEN_OPTIONS="${MAVEN_OPTIONS} --fail-at-end" fi -if [[ "${CHECK}" == "integration" ]] || [[ ${ITERATIONS} -gt 1 ]]; then +if [[ "${CHECK}" == "integration" ]]; then mvn ${MAVEN_OPTIONS} -DskipTests clean install fi @@ -76,6 +76,10 @@ for i in $(seq 1 ${ITERATIONS}); do fi done +if [[ ${ITERATIONS} -gt 1 ]]; then + grep -c "exit code: [^0]" "${REPORT_DIR}/summary.txt" > "${REPORT_DIR}/failures" +fi + if [[ "${OZONE_WITH_COVERAGE}" == "true" ]]; then #Archive combined jacoco records mvn -B -N jacoco:merge -Djacoco.destFile=$REPORT_DIR/jacoco-combined.exec diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java index 4525595f87ca..3894c108d57e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java @@ -803,19 +803,16 @@ protected void deleteRootDir() throws IOException, InterruptedException { } deleteRootRecursively(fileStatuses); - // Waiting for double buffer flush before calling listStatus() again - // seem to have mitigated the flakiness in cleanup(), but at the cost of - // almost doubling the test run time. M1 154s->283s (all 4 sets of params) - cluster.getOzoneManager().awaitDoubleBufferFlush(); - // TODO: Investigate whether listStatus() is correctly iterating cache. - fileStatuses = fs.listStatus(ROOT); + StringBuilder sb = new StringBuilder(); if (fileStatuses != null) { for (FileStatus fileStatus : fileStatuses) { + sb.append(fileStatus.getPath().toString()); + sb.append(","); LOG.error("Unexpected file, should have been deleted: {}", fileStatus); } Assert.assertEquals( - "Delete root failed!", 0, fileStatuses.length); + "Delete root failed! ::: " + sb.toString(), 0, fileStatuses.length); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 8bce4eb70f59..d2c08d07897c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1426,7 +1426,10 @@ private void listStatusFindKeyInTableCache( } OzoneFileStatus fileStatus = new OzoneFileStatus( cacheOmKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(cacheKey)); - cacheKeyMap.put(cacheKey, fileStatus); + cacheKeyMap.putIfAbsent(cacheKey, fileStatus); + } else if (cacheOmKeyInfo == null) { + LOG.error("####DEVESH#### :::: {}", cacheKey); + cacheKeyMap.putIfAbsent(cacheKey, null); } } } @@ -1540,8 +1543,14 @@ public List listStatus(OmKeyArgs args, boolean recursive, countEntries = 0; // Convert results in cacheKeyMap to List for (OzoneFileStatus fileStatus : cacheKeyMap.values()) { - // No need to check if a key is deleted or not here, this is handled - // when adding entries to cacheKeyMap from DB. + // Here need to check if a key is deleted as cacheKeyMap will contain + // deleted entries as well. Adding deleted entries in cacheKeyMap is done + // as there is a possible race condition where table cache iterator is + // flushed already and isKeyDeleted check may not work as expected + // before putting entries in cacheKeyMap in findKeyInDbWithIterator call. + if (fileStatus == null) { + continue; + } fileStatusList.add(fileStatus); countEntries++; if (countEntries >= numEntries) { @@ -1611,8 +1620,9 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, String entryKeyName = omKeyInfo.getKeyName(); if (recursive) { // for recursive list all the entries - - if (!isKeyDeleted(entryInDb, keyTable)) { + // Since putIfAbsent doesn't work as expected in case of null value, + // so had to explicitly check using containsKey + if (!cacheKeyMap.containsKey(entryInDb)) { cacheKeyMap.putIfAbsent(entryInDb, new OzoneFileStatus(omKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(entryKeyName))); countEntries++; @@ -1626,23 +1636,27 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, .getImmediateChild(entryKeyName, keyName); boolean isFile = OzoneFSUtils.isFile(immediateChild); if (isFile) { - if (!isKeyDeleted(entryInDb, keyTable)) { - cacheKeyMap.put(entryInDb, + // Since putIfAbsent doesn't work as expected in case of null + // value, so had to explicitly check using containsKey + if (!cacheKeyMap.containsKey(entryInDb)) { + cacheKeyMap.putIfAbsent(entryInDb, new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile)); countEntries++; } } else { // if entry is a directory - if (!isKeyDeleted(entryInDb, keyTable)) { + // Since putIfAbsent doesn't work as expected in case of null + // value, so had to explicitly check using containsKey + if (!cacheKeyMap.containsKey(entryInDb)) { if (!entryKeyName.equals(immediateChild)) { OmKeyInfo fakeDirEntry = createDirectoryKey( omKeyInfo, immediateChild); - cacheKeyMap.put(entryInDb, + cacheKeyMap.putIfAbsent(entryInDb, new OzoneFileStatus(fakeDirEntry, scmBlockSize, true)); } else { // If entryKeyName matches dir name, we have the info - cacheKeyMap.put(entryInDb, + cacheKeyMap.putIfAbsent(entryInDb, new OzoneFileStatus(omKeyInfo, 0, true)); } countEntries++; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java index 758206f200b7..8cf7127d85f0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java @@ -215,7 +215,11 @@ public Collection listStatusFSO(OmKeyArgs args, HeapEntry entry = heapIterator.next(); OzoneFileStatus status = entry.getStatus(prefixKey, scmBlockSize, volumeName, bucketName, replication); - map.putIfAbsent(entry.key, status); + // Since putIfAbsent doesn't work as expected in case of null value, + // so had to explicitly check using containsKey + if (!map.containsKey(entry.key)) { + map.putIfAbsent(entry.key, status); + } } } From 269d0ace7f89958be55f3b4cd33705202ea69d1d Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Mon, 4 Sep 2023 15:58:42 +0530 Subject: [PATCH 03/10] HDDS-9233. Investigate whether listStatus() is correctly iterating cache. --- .github/workflows/ci.yml | 383 ++++++++++++++++++++++- hadoop-ozone/dev-support/checks/junit.sh | 8 +- 2 files changed, 374 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 12751d6bb452..70cb55e4aea4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,12 +20,322 @@ env: MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3 OZONE_WITH_COVERAGE: ${{ github.repository == 'apache/ozone' && github.event_name != 'pull_request' }} jobs: - unit: + build-info: + runs-on: ubuntu-20.04 + env: + GITHUB_CONTEXT: ${{ toJson(github) }} + outputs: + basic-checks: ${{ steps.selective-checks.outputs.basic-checks }} + needs-basic-checks: ${{ steps.selective-checks.outputs.needs-basic-checks }} + needs-build: ${{ steps.selective-checks.outputs.needs-build }} + needs-compile: ${{ steps.selective-checks.outputs.needs-compile }} + needs-compose-tests: ${{ steps.selective-checks.outputs.needs-compose-tests }} + needs-dependency-check: ${{ steps.selective-checks.outputs.needs-dependency-check }} + needs-integration-tests: ${{ steps.selective-checks.outputs.needs-integration-tests }} + needs-kubernetes-tests: ${{ steps.selective-checks.outputs.needs-kubernetes-tests }} + steps: + - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )" + uses: actions/checkout@v3 + with: + persist-credentials: false + - name: Fetch incoming commit ${{ github.sha }} with its parent + uses: actions/checkout@v3 + with: + ref: ${{ github.sha }} + fetch-depth: 2 + persist-credentials: false + if: github.event_name == 'pull_request' + - name: Selective checks + id: selective-checks + env: + PR_LABELS: "${{ toJSON(github.event.pull_request.labels.*.name) }}" + PR_DRAFT: "${{ github.event.pull_request.draft }}" + run: | + if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then + # Run selective checks + dev-support/ci/selective_ci_checks.sh "${GITHUB_SHA}" + else + # Run all checks + dev-support/ci/selective_ci_checks.sh + fi + build: + needs: + - build-info + runs-on: ubuntu-20.04 + timeout-minutes: 45 + if: needs.build-info.outputs.needs-build == 'true' + strategy: + matrix: + java: [ 8 ] + fail-fast: false + steps: + - name: Checkout project + uses: actions/checkout@v3 + - name: Cache for npm dependencies + uses: actions/cache@v3 + with: + path: | + ~/.pnpm-store + **/node_modules + key: ${{ runner.os }}-pnpm-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: | + ${{ runner.os }}-pnpm- + - name: Cache for maven dependencies + uses: actions/cache@v3 + with: + path: ~/.m2/repository + key: maven-repo-${{ hashFiles('**/pom.xml') }}-${{ matrix.java }} + restore-keys: | + maven-repo-${{ hashFiles('**/pom.xml') }} + maven-repo- + - name: Setup java + uses: actions/setup-java@v3 + with: + distribution: 'temurin' + java-version: ${{ matrix.java }} + - name: Run a full build + run: hadoop-ozone/dev-support/checks/build.sh -Pdist -Psrc + - name: Store binaries for tests + uses: actions/upload-artifact@v3 + with: + name: ozone-bin + path: | + hadoop-ozone/dist/target/ozone-*.tar.gz + !hadoop-ozone/dist/target/ozone-*-src.tar.gz + retention-days: 1 + - name: Store source tarball for compilation + uses: actions/upload-artifact@v3 + with: + name: ozone-src + path: hadoop-ozone/dist/target/ozone-*-src.tar.gz + retention-days: 1 + - name: Delete temporary build artifacts before caching + run: | + #Never cache local artifacts + rm -rf ~/.m2/repository/org/apache/ozone/hdds* + rm -rf ~/.m2/repository/org/apache/ozone/ozone* + if: always() + compile: + needs: + - build-info + - build + runs-on: ubuntu-20.04 + timeout-minutes: 30 + if: needs.build-info.outputs.needs-compile == 'true' + strategy: + matrix: + java: [ 11, 17 ] + fail-fast: false + steps: + - name: Download Ozone source tarball + uses: actions/download-artifact@v3 + with: + name: ozone-src + - name: Untar sources + run: | + tar --strip-components 1 -xzvf ozone*-src.tar.gz + - name: Cache for maven dependencies + uses: actions/cache@v3 + with: + path: ~/.m2/repository + key: maven-repo-${{ hashFiles('**/pom.xml') }}-${{ matrix.java }} + restore-keys: | + maven-repo-${{ hashFiles('**/pom.xml') }} + maven-repo- + - name: Setup java + uses: actions/setup-java@v3 + with: + distribution: 'temurin' + java-version: ${{ matrix.java }} + - name: Compile Ozone using Java ${{ matrix.java }} + run: hadoop-ozone/dev-support/checks/build.sh -Dskip.npx -Dskip.installnpx -Djavac.version=${{ matrix.java }} + env: + OZONE_WITH_COVERAGE: false + - name: Delete temporary build artifacts before caching + run: | + #Never cache local artifacts + rm -rf ~/.m2/repository/org/apache/ozone/hdds* + rm -rf ~/.m2/repository/org/apache/ozone/ozone* + if: always() + basic: + needs: + - build-info + runs-on: ubuntu-20.04 + timeout-minutes: 90 + if: needs.build-info.outputs.needs-basic-checks == 'true' + strategy: + matrix: + check: ${{ fromJson(needs.build-info.outputs.basic-checks) }} + fail-fast: false + steps: + - name: Checkout project + uses: actions/checkout@v3 + if: matrix.check != 'bats' + - name: Checkout project with history + uses: actions/checkout@v3 + with: + fetch-depth: 0 + if: matrix.check == 'bats' + - name: Cache for maven dependencies + uses: actions/cache@v3 + with: + path: ~/.m2/repository + key: maven-repo-${{ hashFiles('**/pom.xml') }}-8-${{ matrix.check }} + restore-keys: | + maven-repo-${{ hashFiles('**/pom.xml') }}-8 + maven-repo-${{ hashFiles('**/pom.xml') }} + maven-repo- + if: ${{ !contains('author,bats,docs', matrix.check) }} + - name: Setup java + uses: actions/setup-java@v3 + with: + distribution: 'temurin' + java-version: 8 + - name: Execute tests + run: hadoop-ozone/dev-support/checks/${{ matrix.check }}.sh + - name: Summary of failures + run: cat target/${{ matrix.check }}/summary.txt + if: ${{ !cancelled() }} + - name: Archive build results + uses: actions/upload-artifact@v3 + if: ${{ !cancelled() }} + with: + name: ${{ matrix.check }} + path: target/${{ matrix.check }} + continue-on-error: true + - name: Delete temporary build artifacts before caching + run: | + #Never cache local artifacts + rm -rf ~/.m2/repository/org/apache/ozone/hdds* + rm -rf ~/.m2/repository/org/apache/ozone/ozone* + if: always() + dependency: + needs: + - build-info + - build + runs-on: ubuntu-20.04 + timeout-minutes: 5 + if: needs.build-info.outputs.needs-dependency-check == 'true' + steps: + - name: Checkout project + uses: actions/checkout@v3 + - name: Download compiled Ozone binaries + uses: actions/download-artifact@v3 + with: + name: ozone-bin + - name: Untar binaries + run: | + mkdir dist + tar -C dist --strip-components 1 -xzf ozone*.tar.gz + - name: Execute tests + run: | + export OZONE_DIST_DIR=`pwd`/dist + ./hadoop-ozone/dev-support/checks/dependency.sh + - name: Archive build results + uses: actions/upload-artifact@v3 + if: always() + with: + name: dependency + path: target/dependency + continue-on-error: true + acceptance: + needs: + - build-info + - build + runs-on: ubuntu-20.04 + timeout-minutes: 150 + if: needs.build-info.outputs.needs-compose-tests == 'true' + strategy: + matrix: + suite: + - secure + - unsecure + - compat + - EC + - HA-secure + - HA-unsecure + - MR + - misc + - cert-rotation + fail-fast: false + steps: + - name: Checkout project + uses: actions/checkout@v3 + - name: Download compiled Ozone binaries + uses: actions/download-artifact@v3 + with: + name: ozone-bin + - name: Untar binaries + run: | + mkdir -p hadoop-ozone/dist/target + tar xzvf ozone*.tar.gz -C hadoop-ozone/dist/target + sudo chmod -R a+rwX hadoop-ozone/dist/target + - name: Execute tests + run: | + pushd hadoop-ozone/dist/target/ozone-* + sudo mkdir .aws && sudo chmod 777 .aws && sudo chown 1000 .aws + popd + ./hadoop-ozone/dev-support/checks/acceptance.sh + env: + KEEP_IMAGE: false + OZONE_ACCEPTANCE_SUITE: ${{ matrix.suite }} + OZONE_VOLUME_OWNER: 1000 + - name: Archive build results + uses: actions/upload-artifact@v3 + if: always() + with: + name: acceptance-${{ matrix.suite }} + path: target/acceptance + continue-on-error: true + kubernetes: + needs: + - build-info + - build + runs-on: ubuntu-20.04 + timeout-minutes: 60 + if: needs.build-info.outputs.needs-kubernetes-tests == 'true' + steps: + - name: Checkout project + uses: actions/checkout@v3 + - name: Download compiled Ozone binaries + uses: actions/download-artifact@v3 + with: + name: ozone-bin + - name: Untar binaries + run: | + mkdir -p hadoop-ozone/dist/target + tar xzvf ozone*.tar.gz -C hadoop-ozone/dist/target + - name: Execute tests + run: | + pushd hadoop-ozone/dist/target/ozone-* + sudo mkdir .aws && sudo chmod 777 .aws && sudo chown 1000 .aws + popd + ./hadoop-ozone/dev-support/checks/kubernetes.sh + - name: Archive build results + uses: actions/upload-artifact@v3 + if: always() + with: + name: kubernetes + path: target/kubernetes + continue-on-error: true + integration: + needs: + - build-info runs-on: ubuntu-20.04 timeout-minutes: 150 + if: needs.build-info.outputs.needs-integration-tests == 'true' strategy: matrix: - profile: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ] + profile: + - client + - contract + - filesystem + - hdds + - om + - ozone + - scm + - shell + - flaky fail-fast: false steps: - name: Checkout project @@ -45,25 +355,76 @@ jobs: distribution: 'temurin' java-version: 8 - name: Execute tests - run: bash hadoop-ozone/dev-support/checks/junit.sh -am -pl :ozone-recon -Dtest=org.apache.hadoop.fs.ozone.TestOzoneFileSystem,org.apache.hadoop.fs.ozone.TestOzoneFSWithObjectStoreCreate,org.apache.hadoop.fs.ozone.TestOzoneFileSystemWithFSO - env: - ITERATIONS: 10 + run: hadoop-ozone/dev-support/checks/integration.sh -P${{ matrix.profile }} + if: matrix.profile != 'flaky' + - name: Execute flaky tests + run: hadoop-ozone/dev-support/checks/integration.sh -P${{ matrix.profile }} -Dsurefire.rerunFailingTestsCount=5 -Dsurefire.fork.timeout=3600 + if: matrix.profile == 'flaky' - name: Summary of failures run: cat target/${{ github.job }}/summary.txt if: always() - - name: Number of failures - run: cat target/${{ github.job }}/failures - if: always() - name: Archive build results uses: actions/upload-artifact@v3 if: always() with: - name: unit-${{ matrix.profile }} - path: target/unit + name: it-${{ matrix.profile }} + path: target/integration continue-on-error: true - name: Delete temporary build artifacts before caching run: | #Never cache local artifacts rm -rf ~/.m2/repository/org/apache/ozone/hdds* rm -rf ~/.m2/repository/org/apache/ozone/ozone* - if: always() \ No newline at end of file + if: always() + coverage: + runs-on: ubuntu-20.04 + timeout-minutes: 30 + if: github.repository == 'apache/ozone' && github.event_name != 'pull_request' + needs: + - acceptance + - basic + - integration + steps: + - name: Checkout project + uses: actions/checkout@v3 + - name: Cache for maven dependencies + uses: actions/cache@v3 + with: + path: ~/.m2/repository + key: maven-repo-${{ hashFiles('**/pom.xml') }}-8-${{ github.job }} + restore-keys: | + maven-repo-${{ hashFiles('**/pom.xml') }}-8 + maven-repo-${{ hashFiles('**/pom.xml') }} + maven-repo- + - name: Download artifacts + uses: actions/download-artifact@v3 + with: + path: target/artifacts + - name: Untar binaries + run: | + mkdir -p hadoop-ozone/dist/target + tar xzvf target/artifacts/ozone-bin/ozone*.tar.gz -C hadoop-ozone/dist/target + - name: Calculate combined coverage + run: ./hadoop-ozone/dev-support/checks/coverage.sh + - name: Setup java 11 + uses: actions/setup-java@v3 + with: + distribution: 'temurin' + java-version: 11 + - name: Upload coverage to Sonar + run: ./hadoop-ozone/dev-support/checks/sonar.sh + env: + SONAR_TOKEN: ${{ secrets.SONARCLOUD_TOKEN }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Archive build results + uses: actions/upload-artifact@v3 + with: + name: coverage + path: target/coverage + continue-on-error: true + - name: Delete temporary build artifacts before caching + run: | + #Never cache local artifacts + rm -rf ~/.m2/repository/org/apache/ozone/hdds* + rm -rf ~/.m2/repository/org/apache/ozone/ozone* + if: always() diff --git a/hadoop-ozone/dev-support/checks/junit.sh b/hadoop-ozone/dev-support/checks/junit.sh index 462d96ad37c1..74cefe6a975d 100644 --- a/hadoop-ozone/dev-support/checks/junit.sh +++ b/hadoop-ozone/dev-support/checks/junit.sh @@ -29,7 +29,7 @@ if [[ ${ITERATIONS} -le 0 ]]; then fi export MAVEN_OPTS="-Xmx4096m $MAVEN_OPTS" -MAVEN_OPTIONS='-B -Dskip.npx -Dskip.installnpx --no-transfer-progress' +MAVEN_OPTIONS='-B -Dskip.npx -Dskip.installnpx -Dnative.lib.tmp.dir=/tmp --no-transfer-progress' if [[ "${OZONE_WITH_COVERAGE}" != "true" ]]; then MAVEN_OPTIONS="${MAVEN_OPTIONS} -Djacoco.skip" @@ -41,7 +41,7 @@ else MAVEN_OPTIONS="${MAVEN_OPTIONS} --fail-at-end" fi -if [[ "${CHECK}" == "integration" ]]; then +if [[ "${CHECK}" == "integration" ]] || [[ ${ITERATIONS} -gt 1 ]]; then mvn ${MAVEN_OPTIONS} -DskipTests clean install fi @@ -76,10 +76,6 @@ for i in $(seq 1 ${ITERATIONS}); do fi done -if [[ ${ITERATIONS} -gt 1 ]]; then - grep -c "exit code: [^0]" "${REPORT_DIR}/summary.txt" > "${REPORT_DIR}/failures" -fi - if [[ "${OZONE_WITH_COVERAGE}" == "true" ]]; then #Archive combined jacoco records mvn -B -N jacoco:merge -Djacoco.destFile=$REPORT_DIR/jacoco-combined.exec From 79b8603afbc86e855a8c4ff239d0d783904f8864 Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Mon, 4 Sep 2023 16:08:30 +0530 Subject: [PATCH 04/10] HDDS-9233. Investigate whether listStatus() is correctly iterating cache. --- .../src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index d2c08d07897c..debc1b10220f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1428,7 +1428,6 @@ private void listStatusFindKeyInTableCache( cacheOmKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(cacheKey)); cacheKeyMap.putIfAbsent(cacheKey, fileStatus); } else if (cacheOmKeyInfo == null) { - LOG.error("####DEVESH#### :::: {}", cacheKey); cacheKeyMap.putIfAbsent(cacheKey, null); } } From 14aac127287853fb3705d537bef5e7bb15c22364 Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Tue, 5 Sep 2023 16:49:40 +0530 Subject: [PATCH 05/10] HDDS-9233. Investigate whether listStatus() is correctly iterating cache. --- .../org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java index 3894c108d57e..9ca10efe51e0 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java @@ -804,15 +804,11 @@ protected void deleteRootDir() throws IOException, InterruptedException { deleteRootRecursively(fileStatuses); fileStatuses = fs.listStatus(ROOT); - StringBuilder sb = new StringBuilder(); if (fileStatuses != null) { for (FileStatus fileStatus : fileStatuses) { - sb.append(fileStatus.getPath().toString()); - sb.append(","); LOG.error("Unexpected file, should have been deleted: {}", fileStatus); } - Assert.assertEquals( - "Delete root failed! ::: " + sb.toString(), 0, fileStatuses.length); + Assert.assertEquals("Delete root failed!", 0, fileStatuses.length); } } From 39a8b55e4b254e8ab23313720a1cd73190fde88d Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Wed, 6 Sep 2023 17:32:42 +0530 Subject: [PATCH 06/10] HDDS-9233. Fixing review comments to update isKeyDeleted check logic. --- .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index debc1b10220f..6f40259fb5b9 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1389,11 +1389,14 @@ public void refresh(OmKeyInfo key) throws IOException { refreshPipeline(Arrays.asList(key)); } - public static boolean isKeyDeleted(String key, Table keyTable) { + public static boolean isKeyDeleted(String key, Table keyTable) + throws IOException { CacheValue omKeyInfoCacheValue = keyTable.getCacheValue(new CacheKey(key)); - return omKeyInfoCacheValue != null - && omKeyInfoCacheValue.getCacheValue() == null; + if (null == omKeyInfoCacheValue) { + return null == keyTable.getSkipCache(key); + } + return omKeyInfoCacheValue.getCacheValue() == null; } /** @@ -1427,7 +1430,9 @@ private void listStatusFindKeyInTableCache( OzoneFileStatus fileStatus = new OzoneFileStatus( cacheOmKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(cacheKey)); cacheKeyMap.putIfAbsent(cacheKey, fileStatus); - } else if (cacheOmKeyInfo == null) { + } else if (cacheOmKeyInfo == null + && cacheKey.startsWith(startCacheKey) + && cacheKey.compareTo(startCacheKey) >= 0) { cacheKeyMap.putIfAbsent(cacheKey, null); } } From f295d22cec5598de0bcb1bf3d38af6c0a7fd62f1 Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Thu, 7 Sep 2023 10:23:00 +0530 Subject: [PATCH 07/10] HDDS-9233. Fixing merge conflict in TestOzoneFileSystem. --- .../java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 6f40259fb5b9..e564abea46df 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1389,14 +1389,11 @@ public void refresh(OmKeyInfo key) throws IOException { refreshPipeline(Arrays.asList(key)); } - public static boolean isKeyDeleted(String key, Table keyTable) - throws IOException { + public static boolean isKeyDeleted(String key, Table keyTable) { CacheValue omKeyInfoCacheValue = keyTable.getCacheValue(new CacheKey(key)); - if (null == omKeyInfoCacheValue) { - return null == keyTable.getSkipCache(key); - } - return omKeyInfoCacheValue.getCacheValue() == null; + return omKeyInfoCacheValue != null + && omKeyInfoCacheValue.getCacheValue() == null; } /** From 1dde84304c112f9eab5f391c31ef470ba61b6290 Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Thu, 7 Sep 2023 13:27:57 +0530 Subject: [PATCH 08/10] HDDS-9233. Fixing review comment of map putIfAbsent usage. --- .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 11 ++++++----- .../apache/hadoop/ozone/om/OzoneListStatusHelper.java | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index e564abea46df..f66e198630ca 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1429,8 +1429,9 @@ private void listStatusFindKeyInTableCache( cacheKeyMap.putIfAbsent(cacheKey, fileStatus); } else if (cacheOmKeyInfo == null && cacheKey.startsWith(startCacheKey) - && cacheKey.compareTo(startCacheKey) >= 0) { - cacheKeyMap.putIfAbsent(cacheKey, null); + && cacheKey.compareTo(startCacheKey) >= 0 + && !cacheKeyMap.containsKey(cacheKey)) { + cacheKeyMap.put(cacheKey, null); } } } @@ -1624,7 +1625,7 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, // Since putIfAbsent doesn't work as expected in case of null value, // so had to explicitly check using containsKey if (!cacheKeyMap.containsKey(entryInDb)) { - cacheKeyMap.putIfAbsent(entryInDb, new OzoneFileStatus(omKeyInfo, + cacheKeyMap.put(entryInDb, new OzoneFileStatus(omKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(entryKeyName))); countEntries++; } @@ -1640,7 +1641,7 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, // Since putIfAbsent doesn't work as expected in case of null // value, so had to explicitly check using containsKey if (!cacheKeyMap.containsKey(entryInDb)) { - cacheKeyMap.putIfAbsent(entryInDb, + cacheKeyMap.put(entryInDb, new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile)); countEntries++; } @@ -1652,7 +1653,7 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, if (!entryKeyName.equals(immediateChild)) { OmKeyInfo fakeDirEntry = createDirectoryKey( omKeyInfo, immediateChild); - cacheKeyMap.putIfAbsent(entryInDb, + cacheKeyMap.put(entryInDb, new OzoneFileStatus(fakeDirEntry, scmBlockSize, true)); } else { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java index 8cf7127d85f0..76fd4cbedc93 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java @@ -218,7 +218,7 @@ public Collection listStatusFSO(OmKeyArgs args, // Since putIfAbsent doesn't work as expected in case of null value, // so had to explicitly check using containsKey if (!map.containsKey(entry.key)) { - map.putIfAbsent(entry.key, status); + map.put(entry.key, status); } } } From 3ed51f8a983a54753792d513c112c026d870fb05 Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Fri, 8 Sep 2023 11:17:30 +0530 Subject: [PATCH 09/10] HDDS-9233. Fixing failure of testListStatusWithTableCacheRecursive test in TestKeyManagerImpl. --- .../java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index f66e198630ca..c89694ed5b1b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1427,10 +1427,7 @@ private void listStatusFindKeyInTableCache( OzoneFileStatus fileStatus = new OzoneFileStatus( cacheOmKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(cacheKey)); cacheKeyMap.putIfAbsent(cacheKey, fileStatus); - } else if (cacheOmKeyInfo == null - && cacheKey.startsWith(startCacheKey) - && cacheKey.compareTo(startCacheKey) >= 0 - && !cacheKeyMap.containsKey(cacheKey)) { + } else if (cacheOmKeyInfo == null && !cacheKeyMap.containsKey(cacheKey)) { cacheKeyMap.put(cacheKey, null); } } @@ -1658,7 +1655,7 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, scmBlockSize, true)); } else { // If entryKeyName matches dir name, we have the info - cacheKeyMap.putIfAbsent(entryInDb, + cacheKeyMap.put(entryInDb, new OzoneFileStatus(omKeyInfo, 0, true)); } countEntries++; From 7917578fdcbb8b673d8fa7c1f4967926cc1ab54c Mon Sep 17 00:00:00 2001 From: deveshsingh Date: Sat, 9 Sep 2023 11:20:44 +0530 Subject: [PATCH 10/10] HDDS-9233. Fixing review comments for updating comments. --- .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 10 ++++------ .../apache/hadoop/ozone/om/OzoneListStatusHelper.java | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index c89694ed5b1b..10d60033fa6f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1427,6 +1427,10 @@ private void listStatusFindKeyInTableCache( OzoneFileStatus fileStatus = new OzoneFileStatus( cacheOmKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(cacheKey)); cacheKeyMap.putIfAbsent(cacheKey, fileStatus); + // This else block has been added to capture deleted entries in cache. + // Adding deleted entries in cacheKeyMap as there is a possible race + // condition where table cache iterator is flushed already when + // using in the caller of this method. } else if (cacheOmKeyInfo == null && !cacheKeyMap.containsKey(cacheKey)) { cacheKeyMap.put(cacheKey, null); } @@ -1619,8 +1623,6 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, String entryKeyName = omKeyInfo.getKeyName(); if (recursive) { // for recursive list all the entries - // Since putIfAbsent doesn't work as expected in case of null value, - // so had to explicitly check using containsKey if (!cacheKeyMap.containsKey(entryInDb)) { cacheKeyMap.put(entryInDb, new OzoneFileStatus(omKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(entryKeyName))); @@ -1635,8 +1637,6 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, .getImmediateChild(entryKeyName, keyName); boolean isFile = OzoneFSUtils.isFile(immediateChild); if (isFile) { - // Since putIfAbsent doesn't work as expected in case of null - // value, so had to explicitly check using containsKey if (!cacheKeyMap.containsKey(entryInDb)) { cacheKeyMap.put(entryInDb, new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile)); @@ -1644,8 +1644,6 @@ private void findKeyInDbWithIterator(boolean recursive, String startKey, } } else { // if entry is a directory - // Since putIfAbsent doesn't work as expected in case of null - // value, so had to explicitly check using containsKey if (!cacheKeyMap.containsKey(entryInDb)) { if (!entryKeyName.equals(immediateChild)) { OmKeyInfo fakeDirEntry = createDirectoryKey( diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java index 76fd4cbedc93..4bfe888d430c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java @@ -215,8 +215,8 @@ public Collection listStatusFSO(OmKeyArgs args, HeapEntry entry = heapIterator.next(); OzoneFileStatus status = entry.getStatus(prefixKey, scmBlockSize, volumeName, bucketName, replication); - // Since putIfAbsent doesn't work as expected in case of null value, - // so had to explicitly check using containsKey + // Caution: DO NOT use putIfAbsent. putIfAbsent undesirably overwrites + // the value with `status` when the existing value in the map is null. if (!map.containsKey(entry.key)) { map.put(entry.key, status); }