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.

Sign up for GitHub

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

Jump to bottom

fix data race in client and daemon #2168

Merged
merged 6 commits into from
Jul 30, 2021
Merged

fix data race in client and daemon #2168

merged 6 commits into from
Jul 30, 2021

Conversation

YangKeao
Copy link
Member

Signed-off-by: YangKeao keao.yang@yahoo.com

What problem does this PR solve?

Fix two possible data race and add an UpdatedStatusClient:

  1. The blockingBuffer is full of data race while reading and writing at the same time.
  2. The object returned from the cache could be the same one, and the reference stored in it will be simply copied out. Run DeepCopy before assigning it to the new one.

@YangKeao
fix data race in client and daemon
26618cc 
Signed-off-by: YangKeao <keao.yang@yahoo.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jul 29, 2021 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • Hexilee
  • STRRL

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@YangKeao YangKeao requested a review from Hexilee July 29, 2021 10:56
@YangKeao YangKeao requested review from STRRL and removed request for Gallardot July 29, 2021 10:57
sonatype-lift[bot]
sonatype-lift bot reviewed Jul 29, 2021
View reviewed changes
go.mod Show resolved Hide resolved
sonatype-lift[bot]
sonatype-lift bot reviewed Jul 29, 2021
View reviewed changes
pkg/bpm/buffer.go Outdated Show resolved Hide resolved
@YangKeao
fix test
e8ec4ae 
Signed-off-by: YangKeao <keao.yang@yahoo.com>
@YangKeao
remove unused struct
d1fd393 
Signed-off-by: YangKeao <keao.yang@yahoo.com>
sonatype-lift[bot]
sonatype-lift bot reviewed Jul 29, 2021
View reviewed changes
pkg/bpm/buffer.go Outdated Show resolved Hide resolved
@YangKeao
Copy link
Member Author

/run-e2e-tests

@YangKeao
fix updated client test
7233033 
Signed-off-by: YangKeao <keao.yang@yahoo.com>
@YangKeao
add a sleep to give it time to sync the cache
9a8a57d 
Signed-off-by: YangKeao <keao.yang@yahoo.com>
@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2021 •
edited
Loading

Codecov Report

Merging #2168 (4da88d4) into master (7e9ff3f) will decrease coverage by 12.60%.
The diff coverage is 39.32%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #2168       +/-   ##
===========================================
- Coverage   55.78%   43.17%   -12.61%     
===========================================
  Files          68      128       +60     
  Lines        4383     8854     +4471     
===========================================
+ Hits         2445     3823     +1378     
- Misses       1768     4703     +2935     
- Partials      170      328      +158
Impacted Files Coverage Δ
api/v1alpha1/awschaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/dnschaos_type.go 0.00% <0.00%> (ø)
api/v1alpha1/dnschaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/gcpchaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/httpchaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/httpchaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/iochaos_types.go 0.00% <0.00%> (-40.00%) ⬇️
api/v1alpha1/jvmchaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/kernelchaos_types.go 0.00% <0.00%> (-20.00%) ⬇️
api/v1alpha1/kinds.go 23.52% <0.00%> (-3.14%) ⬇️
... and 188 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1eb4025...4da88d4. Read the comment docs.

Hexilee
Hexilee approved these changes Jul 30, 2021
View reviewed changes
Copy link
Member

@Hexilee Hexilee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

STRRL
STRRL approved these changes Jul 30, 2021
View reviewed changes
Copy link
Member

@STRRL STRRL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@STRRL
Copy link
Member

STRRL commented Jul 30, 2021

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 9a8a57d

@STRRL
Copy link
Member

STRRL commented Jul 30, 2021

does this PR need to be cherrypicked into release-2.0? @YangKeao

@ti-chi-bot ti-chi-bot merged commit d0f184c into chaos-mesh:master Jul 30, 2021
@ti-srebot ti-srebot mentioned this pull request Aug 3, 2021
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-2.0 in PR #2175

ti-chi-bot pushed a commit that referenced this pull request Aug 4, 2021
@ti-srebot @YangKeao
fix data race in client and daemon (#2168) (#2175)
96904c6 
* fix data race in client and daemon

Signed-off-by: YangKeao <keao.yang@yahoo.com>
Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

* fix test

Signed-off-by: YangKeao <keao.yang@yahoo.com>
Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

* remove unused struct

Signed-off-by: YangKeao <keao.yang@yahoo.com>
Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

* fix updated client test

Signed-off-by: YangKeao <keao.yang@yahoo.com>
Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

* add a sleep to give it time to sync the cache

Signed-off-by: YangKeao <keao.yang@yahoo.com>
Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

Co-authored-by: YangKeao <yangkeao@chunibyo.icu>
Co-authored-by: YangKeao <keao.yang@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonatype-lift sonatype-lift[bot] sonatype-lift left review comments

@STRRL STRRL STRRL approved these changes

@Hexilee Hexilee Hexilee approved these changes

Assignees
No one assigned
Labels
needs-cherry-pick-release-2.0 size/L status/can-merge status/LGT2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@YangKeao @ti-chi-bot @codecov-commenter @STRRL @ti-srebot @Hexilee