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

Add option for async completion of StreamingWorkunitHandlers, disable by default in containers #12392

Merged
merged 2 commits into from
Jul 21, 2021

Conversation

stuhood
Copy link
Member

@stuhood stuhood commented Jul 21, 2021

When run inside of a container that exits as soon as the client returns, async completion of workunit handlers can lead to loss of metrics. Since async completion has significant performance benefits, we disable it conditionally based on whether it is likely that we are running inside of a container.

Fixes #11833.

[ci skip-rust]

… and disable it when we are likely running inside a container.

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]
@stuhood stuhood requested review from asherf and Eric-Arellano July 21, 2021 16:48
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks. Cherry-pick to 2.6?

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@stuhood stuhood modified the milestones: 2.6.x, 2.5.x Jul 21, 2021
@stuhood stuhood merged commit 6444fe2 into pantsbuild:main Jul 21, 2021
@stuhood stuhood deleted the stuhood/auto-async-swh branch July 21, 2021 23:16
stuhood added a commit to stuhood/pants that referenced this pull request Jul 22, 2021
… by default in containers (pantsbuild#12392)

When run inside of a container that exits as soon as the client returns, async completion of workunit handlers can lead to loss of metrics. Since async completion has significant performance benefits, we disable it conditionally based on whether it is likely that we are running inside of a container.

Fixes pantsbuild#11833.

[ci skip-rust]
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
stuhood added a commit to stuhood/pants that referenced this pull request Jul 22, 2021
… by default in containers (pantsbuild#12392)

When run inside of a container that exits as soon as the client returns, async completion of workunit handlers can lead to loss of metrics. Since async completion has significant performance benefits, we disable it conditionally based on whether it is likely that we are running inside of a container.

Fixes pantsbuild#11833.

[ci skip-rust]
Eric-Arellano pushed a commit that referenced this pull request Jul 22, 2021
… by default in containers (Cherry-pick of #12392) (#12398)

When run inside of a container that exits as soon as the client returns, async completion of workunit handlers can lead to loss of metrics. Since async completion has significant performance benefits, we disable it conditionally based on whether it is likely that we are running inside of a container.

Fixes #11833.

[ci skip-rust]
[ci skip-build-wheels]
Eric-Arellano pushed a commit that referenced this pull request Jul 22, 2021
… by default in containers (Cherry-pick of #12392) (#12399)

When run inside of a container that exits as soon as the client returns, async completion of workunit handlers can lead to loss of metrics. Since async completion has significant performance benefits, we disable it conditionally based on whether it is likely that we are running inside of a container.

Fixes #11833.

[ci skip-rust]
[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The anonymous-telemetry subsystem ~doubles Pants no-op latency when non-async callbacks are enabled.
3 participants