Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Add context cancelation #15374

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

klauspost
Copy link
Contributor

Description

A huge number of goroutines would build up from various monitors

When creating test filesystems provide a context so they can shut down when no longer needed.

How to test this PR?

Tests only.

Types of changes

  • Optimization (provides speedup with no functional changes)
  • Unit tests added/updated

A huge number of goroutines would build up from various monitors

When creating test filesystems provide a context so they can shut down when no longer needed.
@minio-trusted
Copy link
Contributor

Mint Automation

Test Result
mint-large-bucket.sh ✔️
mint-fs.sh ✔️
mint-gateway-s3.sh ✔️
mint-erasure.sh ✔️
mint-dist-erasure.sh ✔️
mint-compress-encrypt-dist-erasure.sh ✔️
mint-pools.sh ✔️
Deleting image on docker hub
Deleting image locally

@harshavardhana harshavardhana merged commit 1e332f0 into minio:master Jul 21, 2022
harshavardhana pushed a commit that referenced this pull request Jul 21, 2022
This reverts commit 1e332f0.

Reverting this as tests are failing randomly.
@harshavardhana
Copy link
Member

Reverted this since all tests are failing randomly across all PRs after this change

commit 564a0afae1c6ddfb6e57152f70e0e0730796164c (HEAD -> master, origin/master)
Author: Minio Trusted <trusted@minio.io>
Date:   Thu Jul 21 13:58:51 2022 -0700

    Revert "tests: Add context cancelation (#15374)"
    
    This reverts commit 1e332f0eb1ef864bf34631ab4f30f14679dcebe8.
    
    Reverting this as tests are failing randomly.

Perhaps an investigation is needed.

@klauspost
Copy link
Contributor Author

@harshavardhana Do you have any information? I see master built fine after merge, so no idea what you are talking about.

@klauspost
Copy link
Contributor Author

Seems like tests are just failing anyways.

@harshavardhana
Copy link
Member

Seems like tests are just failing anyways.

No not really this silent failure

WARNING: Host local has more than 0 drives of set. A host failure will result in data becoming unavailable.
--- PASS: TestPrintCLIAccessMsg (0.03s)
=== RUN   TestPrintStartupMessage
Formatting 1st pool, 1 set(s), 1 drives per set.
WARNING: Host local has more than 0 drives of set. A host failure will result in data becoming unavailable.
MinIO Object Storage Server
Copyright: 2015-0000 MinIO, Inc.
License: GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>
Version: DEVELOPMENT.GOGET (go1.17.12 linux/amd64)
Status:         32 Online, 0 Offline. 
API: http://127.0.0.1:9000
Console: http://10.1.0.230:13333 http://172.17.0.1:13333 http://127.0.0.1:133
Documentation: https://docs.min.io
--- PASS: TestPrintStartupMessage (0.06s)
=== RUN   TestServerSuite
=== RUN   TestServerSuite/Test:_1,_ServerType:_ErasureSD
Formatting 1st pool, 1 set(s), 1 drives per set.
WARNING: Host local has more than 0 drives of set. A host failure will result in data becoming unavailable.
=== RUN   TestServerSuite/Test:_2,_ServerType:_ErasureSD
Formatting 1st pool, 1 set(s), 1 drives per set.
WARNING: Host local has more than 0 drives of set. A host failure will result in data becoming unavailable.
=== RUN   TestServerSuite/Test:_3,_ServerType:_ErasureSD
Formatting 1st pool, 1 set(s), 1 drives per set.
WARNING: Host local has more than 0 drives of set. A host failure will result in data becoming unavailable.
=== RUN   TestServerSuite/Test:_4,_ServerType:_Erasure
Formatting 1st pool, 1 set(s), 16 drives per set.
WARNING: Host local has more than 4 drives of set. A host failure will result in data becoming unavailable.
=== RUN   TestServerSuite/Test:_5,_ServerType:_ErasureSet
Formatting 1st pool, 2 set(s), 16 drives per set.
WARNING: Host local has more than 4 drives of set. A host failure will result in data becoming unavailable.
WARNING: Host local has more than 4 drives of set. A host failure will result in data becoming unavailable.
FAIL	github.com/minio/minio/cmd	2972.346s
FAIL
make: *** [Makefile:57: test-race] Error 1
Error: Process completed with exit code 2.
```

https://github.com/minio/minio/runs/7455671632?check_suite_focus=true

This is a harmless change however the tests failed silently, after reverting the context change no more failures. 

@klauspost klauspost deleted the tests-add-context-cancel branch October 14, 2022 09:48
@klauspost klauspost restored the tests-add-context-cancel branch October 14, 2022 09:48
harshavardhana added a commit to harshavardhana/minio that referenced this pull request Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants