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

Remote: Postpone the block waiting in afterCommand to BlockWaitingModule #14618

Closed
wants to merge 3 commits into from

Conversation

coeuvre
Copy link
Member

@coeuvre coeuvre commented Jan 21, 2022

When implementing async upload, we introduced a block waiting behaviour in RemoteModule#afterCommand so that uploads happened in the background can be waited before the whole build complete.

However, there are other block waiting code in other module's afterCommand method (e.g. BES module). Block waiting in remote module will prevent other modules' afterCommand from executing until it's completed. This causes issues like #14576.

This PR adds a new module BlockWaitingModule whose sole purpose is to accept tasks submitted by other modules in afterCommand and block waiting all the tasks in its own afterCommand method. So those tasks can be executed in parallel.

This PR only updates RemoteModule's afterCommand method to submit block waiting task. Other modules should be updated to use BlockWaitingModule as well but that's beyond the scope this this PR.

This PR along with 73a76a8 fix #14576.

@coeuvre coeuvre requested a review from meisterT January 21, 2022 12:47
@coeuvre coeuvre requested a review from a team as a code owner January 21, 2022 12:47
@brentleyjones
Copy link
Contributor

Does this also fix the fact that --bes_upload_mode=async stopped working?

@coeuvre
Copy link
Member Author

coeuvre commented Jan 21, 2022

My gut feeling is yes but do we have an issue already where I can learn more details from?

@brentleyjones
Copy link
Contributor

#14620

@coeuvre
Copy link
Member Author

coeuvre commented Jan 21, 2022

Well, this PR won't fix #14620 but I have another fix which can fix both #14576 and #14620. Closing this for now. We still need this PR to prevent other cases from blocking BES module's afterCommand.

@coeuvre coeuvre closed this Jan 21, 2022
@coeuvre coeuvre reopened this Jan 21, 2022
@brentleyjones
Copy link
Contributor

@Wyverald This fixes a 5.0 regression. Can we add it to the 5.0.1 milestone?

@Wyverald
Copy link
Member

The linked issue is already on the milestone :)

@bazel-io bazel-io closed this in 621649d Jan 26, 2022
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Feb 16, 2022
…Module`

When implementing async upload, we introduced a block waiting behaviour in `RemoteModule#afterCommand` so that uploads happened in the background can be waited before the whole build complete.

However, there are other block waiting code in other module's `afterCommand` method (e.g. BES module). Block waiting in remote module will prevent other modules' `afterCommand` from executing until it's completed. This causes issues like bazelbuild#14576.

This PR adds a new module `BlockWaitingModule` whose sole purpose is to accept tasks submitted by other modules in `afterCommand` and block waiting all the tasks in its own `afterCommand` method. So those tasks can be executed in parallel.

This PR only updates RemoteModule's `afterCommand` method to submit block waiting task. Other modules should be updated to use `BlockWaitingModule` as well but that's beyond the scope this this PR.

This PR along with 73a76a8 fix bazelbuild#14576.

Closes bazelbuild#14618.

PiperOrigin-RevId: 424295121
(cherry picked from commit 621649d)
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Feb 16, 2022
…Module`

When implementing async upload, we introduced a block waiting behaviour in `RemoteModule#afterCommand` so that uploads happened in the background can be waited before the whole build complete.

However, there are other block waiting code in other module's `afterCommand` method (e.g. BES module). Block waiting in remote module will prevent other modules' `afterCommand` from executing until it's completed. This causes issues like bazelbuild#14576.

This PR adds a new module `BlockWaitingModule` whose sole purpose is to accept tasks submitted by other modules in `afterCommand` and block waiting all the tasks in its own `afterCommand` method. So those tasks can be executed in parallel.

This PR only updates RemoteModule's `afterCommand` method to submit block waiting task. Other modules should be updated to use `BlockWaitingModule` as well but that's beyond the scope this this PR.

This PR along with 73a76a8 fix bazelbuild#14576.

Closes bazelbuild#14618.

PiperOrigin-RevId: 424295121
(cherry picked from commit 621649d)
brentleyjones pushed a commit to brentleyjones/bazel that referenced this pull request Feb 16, 2022
…Module`

When implementing async upload, we introduced a block waiting behaviour in `RemoteModule#afterCommand` so that uploads happened in the background can be waited before the whole build complete.

However, there are other block waiting code in other module's `afterCommand` method (e.g. BES module). Block waiting in remote module will prevent other modules' `afterCommand` from executing until it's completed. This causes issues like bazelbuild#14576.

This PR adds a new module `BlockWaitingModule` whose sole purpose is to accept tasks submitted by other modules in `afterCommand` and block waiting all the tasks in its own `afterCommand` method. So those tasks can be executed in parallel.

This PR only updates RemoteModule's `afterCommand` method to submit block waiting task. Other modules should be updated to use `BlockWaitingModule` as well but that's beyond the scope this this PR.

This PR along with 73a76a8 fix bazelbuild#14576.

Closes bazelbuild#14618.

PiperOrigin-RevId: 424295121
(cherry picked from commit 621649d)
Wyverald pushed a commit that referenced this pull request Feb 16, 2022
…Module` (#14833)

When implementing async upload, we introduced a block waiting behaviour in `RemoteModule#afterCommand` so that uploads happened in the background can be waited before the whole build complete.

However, there are other block waiting code in other module's `afterCommand` method (e.g. BES module). Block waiting in remote module will prevent other modules' `afterCommand` from executing until it's completed. This causes issues like #14576.

This PR adds a new module `BlockWaitingModule` whose sole purpose is to accept tasks submitted by other modules in `afterCommand` and block waiting all the tasks in its own `afterCommand` method. So those tasks can be executed in parallel.

This PR only updates RemoteModule's `afterCommand` method to submit block waiting task. Other modules should be updated to use `BlockWaitingModule` as well but that's beyond the scope this this PR.

This PR along with 73a76a8 fix #14576.

Closes #14618.

PiperOrigin-RevId: 424295121
(cherry picked from commit 621649d)

Co-authored-by: Chi Wang <chiwang@google.com>
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.

--bes_timeout is ignored in Bazel 5
4 participants