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

Remove unnecessary locking in Start/Stop #3041

Closed
jrcamp opened this issue Apr 28, 2021 · 3 comments · Fixed by open-telemetry/opentelemetry-collector-contrib#4046
Closed
Assignees
Labels
help wanted Good issue for contributors to OpenTelemetry Service to pick up

Comments

@jrcamp
Copy link
Contributor

jrcamp commented Apr 28, 2021

Don't think this locking is needed is it? Starting and Stopping components get run in same goroutine so I assume memory ordering should be ok. I think most components depend on this memory consistency as well.

Originally posted by @jrcamp in #3037 (comment)

@jrcamp jrcamp self-assigned this Apr 28, 2021
@bogdandrutu bogdandrutu added this to the Phase2-GA-Roadmap milestone Apr 28, 2021
@bogdandrutu bogdandrutu added the help wanted Good issue for contributors to OpenTelemetry Service to pick up label Apr 28, 2021
@bhautikpip
Copy link
Contributor

Fix for this issue #3070 is merged so we can probably close out this issue.

@alolita
Copy link
Member

alolita commented May 12, 2021

@mxiamxia can you please check if there are other components that have this unnecessary mutex lock? Please comment on the issue with your findings?

This issue should not be a GA blocker.

@bogdandrutu
Copy link
Member

@bhautikpip there are more places where that happens.

jrcamp added a commit to jrcamp/opentelemetry-collector that referenced this issue Jun 18, 2021
The lock wasn't called from more than one location.

Relates open-telemetry#3041
bogdandrutu pushed a commit that referenced this issue Jun 21, 2021
The lock wasn't called from more than one location.

Relates #3041
jrcamp added a commit to jrcamp/opentelemetry-collector-contrib that referenced this issue Jun 30, 2021
This removes locking that is only used for calls to Startup/Shutdown. These
functions are run in the same goroutine so there should be no
memory-inconsistency in calls to Startup and Shutdown.

Fixes open-telemetry/opentelemetry-collector#3041
jrcamp added a commit to jrcamp/opentelemetry-collector-contrib that referenced this issue Jun 30, 2021
This removes locking that is only used for calls to Startup/Shutdown. These
functions are run in the same goroutine so there should be no
memory-inconsistency in calls to Startup and Shutdown.

Fixes open-telemetry/opentelemetry-collector#3041
jrcamp added a commit to jrcamp/opentelemetry-collector-contrib that referenced this issue Jun 30, 2021
This removes locking that is only used for calls to Startup/Shutdown. These
functions are run in the same goroutine so there should be no
memory-inconsistency in calls to Startup and Shutdown.

Fixes open-telemetry/opentelemetry-collector#3041
bogdandrutu pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this issue Jun 30, 2021
This removes locking that is only used for calls to Startup/Shutdown. These
functions are run in the same goroutine so there should be no
memory-inconsistency in calls to Startup and Shutdown.

Fixes open-telemetry/opentelemetry-collector#3041
mstumpfx pushed a commit to mstumpfx/opentelemetry-collector-contrib that referenced this issue Aug 31, 2021
This removes locking that is only used for calls to Startup/Shutdown. These
functions are run in the same goroutine so there should be no
memory-inconsistency in calls to Startup and Shutdown.

Fixes open-telemetry/opentelemetry-collector#3041
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Good issue for contributors to OpenTelemetry Service to pick up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants