CLDSRV-992: Build the uploadPartCopy fixtures once per suite, not per test - #6281
CLDSRV-992: Build the uploadPartCopy fixtures once per suite, not per test#6281anurag4DSB wants to merge 1 commit into
Conversation
… test Both `uploadPartCopy` describes in the PartNumber tests rebuild a large MPU fixture in a beforeEach, although all four of their tests only issue a GetObject. Each rebuild moves roughly 200 MB: completeMPU uploads ten 5 MB parts one at a time with async.eachSeries, createMPUAndPutTwoParts copies that 50 MB object and then puts a 50 MB body, and the overwrite variant adds a 5 MB part and a second 50 MB UploadPartCopy. Doing that twice per describe, once for each read-only test, is what pushes the hook past the 40 s mocha timeout on the s3c backend. Move the setup to `before` and the DeleteObject cleanup to `after`. The fixture is identical for both tests in each describe, and the enclosing describe creates the bucket in its own `before` and drops it in `after`, so the fixture safely outlives the individual tests. Halves the data moved by these two describes and removes the redundant rebuild, rather than raising the timeout. Clears CLDSRV-992 row F4 (run 33630581671) and the timeout half of row F8 (run 33647777908, fixed for the 9.2 line separately). Issue: CLDSRV-992
Hello anurag4dsb,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| async-migration-report: | ||
| runs-on: ubuntu-24.04 | ||
| if: startsWith(github.ref, 'refs/heads/development/') | ||
| if: false # CLDSRV-992 stress: unrelated job |
There was a problem hiding this comment.
Stress-testing scaffolding: this if: false disables the async-migration-report job. Along with similar if: false guards on multiple-backend, mongo-v0-ft-tests, mongo-v1-ft-tests, file-ft-tests, utapi-v2-tests, sur-tests, kmip-ft-tests, kmip-cluster-ft-tests, sse-kms-migration-tests, and the final aggregation job — 11 CI jobs are disabled. These changes must be reverted before merge, otherwise CI coverage is permanently lost.
| # Configure and run as Integration run S3C tests | ||
| s3c-ft-tests: | ||
| strategy: | ||
| fail-fast: false |
There was a problem hiding this comment.
Stress-testing scaffolding: the original matrix had three configurations (v0, v0-null-compat, v1) but this replaces them with 10 identical v0-null-compat entries. The v0 (without null-compat) and v1 configurations are dropped entirely. This must be reverted to the original matrix before merge — merging as-is permanently removes v1 and non-null-compat v0 test coverage.
00cfff2 to
6d9d021
Compare
|
Moving back to draft — the rationale for this change does not hold up. I justified it as "the fixture is too expensive for the 40 s budget", but measuring the CI logs shows the fixture is not slow: the tests around it run in 38–657 ms, and in the failing runs the same fixture builds successfully three or four times and then stalls once. That is an intermittent stall, not slow work, so building it once per suite only halves the exposure without explaining the cause — and it gives up per-test independence to do so, which is a bad trade. Independence here is also load-bearing rather than incidental: sibling tests in the same describe do plain These jobs are also the ones that capture no cloudserver log, so the stall is currently undiagnosable. #6283 adds that capture; I will come back to this row once a recurrence can name its own cause. |
2 participants
Intent: why does this change exist?
NEW FIX. Both
uploadPartCopydescribes in the PartNumber tests rebuild a ~200 MB MPU fixture in abeforeEach, though all four of their tests only issue a GetObject. The redundant second build is what exceeds the 40 s mocha timeout. Clears CLDSRV-992 row F4.System impact: what's affected, including downstream?
One functional test file, setup and teardown only. No product code, no assertions touched. hotfix/9.2.36 needs the same change and has #6280.
Preserved behavior: what explicitly stays the same?
The fixture is identical and both tests read exactly what they read before. No timeout raised, no retry added, nothing skipped.
Intended change: what's different after this PR?
Setup moves to
beforeand cleanup toafter, so each describe builds its fixture once. The enclosing describe already owns the bucket in its ownbefore/after, so the fixture safely outlives the tests.Verification: how do we know this worked, or how would we know if it didn't?
The cost is arithmetic from the fixture itself: a serial 10x5 MB completeMPU, a 50 MB UploadPartCopy, a 50 MB UploadPart, then a further 5 MB part and second 50 MB copy. Both tests are pure GetObject calls, so the second build did no work they depended on. s3c-ft-tests-v0-null-compat stressed 10x on this branch.