Skip to content

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 <[email protected]>
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