Skip to content

feat: Add Sample Locking + Massive Parallel test#15

Merged
hsluoyz merged 2 commits intoapache:masterfrom
pboyd04:AddSampleLocking
May 21, 2025
Merged

feat: Add Sample Locking + Massive Parallel test#15
hsluoyz merged 2 commits intoapache:masterfrom
pboyd04:AddSampleLocking

Conversation

@pboyd04
Copy link
Copy Markdown
Contributor

@pboyd04 pboyd04 commented May 13, 2025

I've added locking around the samples slice to avoid any race conditions. I could not cause the existing code to fail in anyway so I'm not really sure what the issue with the race condition was, but this should solve it.

@see-sharp-dev
Copy link
Copy Markdown

now the data race which I was talking about is visible in the failing tests

@pboyd04
Copy link
Copy Markdown
Contributor Author

pboyd04 commented May 14, 2025

Man I really wish there was a way to label a race as "doesn't matter" because it really doesn't in this case, but fixed at any rate.

@see-sharp-dev
Copy link
Copy Markdown

thank you.
@hsluoyz can we merge this one? that will solve the issue with parallel test execution.

@hsluoyz
Copy link
Copy Markdown
Member

hsluoyz commented May 21, 2025

@pboyd04 what issue does this PR fix? Better with an issue

@see-sharp-dev
Copy link
Copy Markdown

@hsluoyz we use Casbin enforcer, which under the hood uses this package.
after uptaking a recent version of this package parallel tests began to fail because of the detected race condition during unsynchronized access to the global variables.
similar was detected in previous test executions here

@hsluoyz hsluoyz changed the title Add Sample Locking + Massive Parallel test feat: Add Sample Locking + Massive Parallel test May 21, 2025
@hsluoyz hsluoyz merged commit d795199 into apache:master May 21, 2025
7 checks passed
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 1.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants