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

Unexpected lock when two CLI clients are active #982

Closed
pkolaczk opened this issue Jul 25, 2019 · 8 comments
Closed

Unexpected lock when two CLI clients are active #982

pkolaczk opened this issue Jul 25, 2019 · 8 comments
Labels
cli ergonomics Any change that affects developer ergonomics and the easiness of use of bloop. feature task / compile task / test

Comments

@pkolaczk
Copy link
Contributor

pkolaczk commented Jul 25, 2019

I can't run a test or do anything with bloop from another terminal when the background compilation is waiting for file changes.

I get:

Waiting on external CLI client to release lock on this build...

Why is it needed?
Why can't we release the lock during the time it is not compiling and just waiting?

This seriously limits usefullness of background compilation mode.
We care about every 0.1 second, but this lock is adding 2 more manual steps to stop watching and then to restart watching after running a test.

(BTW: I know I can use watching to also run tests, however I may have a larger set of tests that is slow and consuming lot of resources, so I don't want my tests running when I'm working with the code, until I finish).

@jvican jvican changed the title Can't run a test when backround compilation is watching Unexpected lock when two CLI clients are active Jul 29, 2019
@jvican jvican added cli ergonomics Any change that affects developer ergonomics and the easiness of use of bloop. feature task / compile task / test labels Jul 29, 2019
@jvican
Copy link
Contributor

jvican commented Jul 29, 2019

At the moment, this use case is not supported on purpose. You can have multiple BSP clients per build but you can only have one CLI. The reason for this is because a CLI has a stable set of compile directories and stable directories can't be properly isolated across concurrent clients.

This is likely to change with some changes I'll work on soon, where a CLI won't use nailgun anymore and instead will act as a BSP client. However, I need to experiment with that a bit because that solution also has some drawbacks. There are other solutions we could implement to support this behavior too, let's discuss those when I provide feedback after running this experiment.

Note how asking to run a test from a BSP client such as Metals or IntelliJ works (at least when IntelliJ fixes the issue with testing).

@tindzk
Copy link
Contributor

tindzk commented Aug 28, 2019

I hit the same problem in several projects. It is a common use case in client-server applications.

In previous Bloop versions, I could open two terminals with:

bloop link client --watch   # Scala.js frontend
bloop run server --watch    # Scala JVM backend

But with Bloop v1.3.2, the second command will wait for the lock to be released. The lock is actually not needed since client and server do not have any overlapping .class files.

If BSP is not affected by this limitation, I could adapt Seed to use a BSP connection instead of the Nailgun client. I had a look at the latest protocol definition, but it does not seem to be capable of linking modules yet.

@jvican
Copy link
Contributor

jvican commented Aug 28, 2019

@tindzk All build tool integrations should exist over BSP instead of CLI. Once a BSP integration is in place, you'll be able to run two seed commands where each of it uses a different part of the build graph.

@jvican
Copy link
Contributor

jvican commented Oct 5, 2019

I'm closing this ticket because this is a design decision I'm not planning to revisit any time soon. I might experiment with productionizing a prototype to make the CLI a bsp client down the road, but for now it's not a priority. Thanks for reporting, I think it's great there's some trace of this behavior in the issue tracker as well as a reiteration that build tool integrations should always prefer BSP-based instead of CLI-based integrations.

@jvican jvican closed this as completed Oct 5, 2019
tindzk added a commit to tindzk/seed that referenced this issue Oct 10, 2019
Build and run modules with the Build Server Protocol (BSP).
Previously, we were using Bloop's CLI which has the following
limitations:

- `bloop run --watch` does not compile the changed sources if the
  process is running already:
  scalacenter/bloop#558
- For each Bloop build, only a single command can be run at a time:
  scalacenter/bloop#982
- No detailed build progress information is available

Using BSP allows us to integrate more tightly with Bloop and avoid
these limitations. When the user now executes a Seed command, the
following steps are performed:

First, `bloop bsp` is run which will start a BSP server in the
background and listen on a UNIX socket. Then, bsp4j is used to
establish a connection using ZIO's exponential retry policy. Once
the connection is active, the corresponding BSP tasks are executed
sequentially. The BSP server responds asynchronously with the
results of the running tasks (e.g. compilation diagnostics or
progress updates). Finally, the BSP server can be gracefully shut
down and the socket file removed.

BSP reports detailed information on the compilation progress which
allows us to render a separate progress bar for each module.
Compilation messages (warnings or errors) do not interfere with the
progress bar since messages are printed above it. Progress bars are
disabled in the Docker image to avoid verbose CI logs.

A new command for running modules was introduced, called `run`. It
is currently limited to one module since programs can be
non-terminating.

The `package` command now builds the supplied module prior to
packaging it. This change was needed since BSP uses a different
target path for class files compared to Bloop's CLI command.

The `link` command continues to use the Bloop CLI since this
operation is not exposed via BSP yet.

A bug was fixed in `server` that resulted in the `optimise`
parameter being ignored when sending a link request.

The commands `build`, `run` and `link` support a watch mode that can
be enabled by passing in `--watch`. It was implemented based on
EPFL's fork of the directory-watcher library which supports watching
file paths. The watch events are then exposed as a ZIO stream.

With these changes, the user can run operations in parallel on
multiple modules defined in the same build. We can also
automatically restart JVM processes whenever source files are
changed. As an example, this enables the following workflow for a
client/server application:

```bash
seed link client --watch  # Terminal 1: Link Scala.js frontend
seed run server --watch   # Terminal 2: Run Scala JVM backend
```

Closes #58.
@jvican
Copy link
Contributor

jvican commented Oct 22, 2019

Note for casual readers: the workaround to this problem is to run tests/application from other Bloop clients than the CLI client. For example, you can run your application once from your CLI and from your editor concurrently (IntelliJ/VS Code/vim, etc). The problem discussed here only affects concurrent CLI clients.

@jypma
Copy link

jypma commented Oct 22, 2019

Interesting. Why would CLI clients require a different kind of locking from editor clients?

@jvican
Copy link
Contributor

jvican commented Oct 22, 2019

@jypma Because BSP clients are isolated (every BSP client has its own unique set of classes directories granted upon their connection) while the CLI classes directories are stable (at least for now). You can learn more about this in my Scala World talk https://www.youtube.com/watch?v=PgQyCnqm4QQ

@jvican
Copy link
Contributor

jvican commented Nov 2, 2019

Hi all! I found a sensible way to guarantee the isolation of CLI clients so the next release will remove the CLI lock and allow users to use as many concurrent CLI clients as they want.

jvican added a commit that referenced this issue Nov 2, 2019
Before this change, CLI clients were always using the directories coming
from the build tools and only one CLI client could run in a workspace
(precisely because of the same reason -- the actions of multiple, active
CLI sessions were by nature not isolated from each other).

This commit changes that. Given that now the build tool does own the
generic classes directories for every project, the CLI can no longer
used the same set of directories if it wants to run at the same time as
the build tool. We fix that by assigning a stable classes directory per
project for a CLI client. When more than one CLI client is detected,
bloop creates a temporary set of classes directories where all of the
compilation outputs are added. These directories only exist while the
client is connnected. When it exists, Bloop deletes these temporary
directories.

To make sure we neve rleak any additional CLI temporary directories, we
add some logic after the exit of every client to check if there are some
outdated/orphan temporary classes directories from previous CLI
sessions. These orphan directories are usually from previous CLI
sessions whose deletion logic didn't run because, for example, the
server was shut down with a SIGKILL.

All in all, this commit fixes #982
tindzk added a commit to tindzk/seed that referenced this issue Nov 5, 2019
Build and run modules with the Build Server Protocol (BSP).
Previously, we were using Bloop's CLI which has the following
limitations:

- `bloop run --watch` does not compile the changed sources if the
  process is running already:
  scalacenter/bloop#558
- For each Bloop build, only a single command can be run at a time:
  scalacenter/bloop#982
- No detailed build progress information is available

Using BSP allows us to integrate more tightly with Bloop and avoid
these limitations. When the user now executes a Seed command, the
following steps are performed:

First, `bloop bsp` is run which will start a BSP server in the
background and listen on a UNIX socket. Then, bsp4j is used to
establish a connection using ZIO's exponential retry policy. Once
the connection is active, the corresponding BSP tasks are executed
sequentially. The BSP server responds asynchronously with the
results of the running tasks (e.g. compilation diagnostics or
progress updates). Finally, the BSP server can be gracefully shut
down and the socket file removed.

BSP reports detailed information on the compilation progress which
allows us to render a separate progress bar for each module.
Compilation messages (warnings or errors) do not interfere with the
progress bar since messages are printed above it. Progress bars are
disabled in the Docker image to avoid verbose CI logs.

A new command for running modules was introduced, called `run`. It
is currently limited to one module since programs can be
non-terminating.

The `package` command now builds the supplied module prior to
packaging it. This change was needed since BSP uses a different
target path for class files compared to Bloop's CLI command.

The `link` command continues to use the Bloop CLI since this
operation is not exposed via BSP yet.

A bug was fixed in `server` that resulted in the `optimise`
parameter being ignored when sending a link request.

The commands `build`, `run` and `link` support a watch mode that can
be enabled by passing in `--watch`. It was implemented based on
EPFL's fork of the directory-watcher library which supports watching
file paths. The watch events are then exposed as a ZIO stream.

With these changes, the user can run operations in parallel on
multiple modules defined in the same build. We can also
automatically restart JVM processes whenever source files are
changed. As an example, this enables the following workflow for a
client/server application:

```bash
seed link client --watch  # Terminal 1: Link Scala.js frontend
seed run server --watch   # Terminal 2: Run Scala JVM backend
```

Closes #58.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli ergonomics Any change that affects developer ergonomics and the easiness of use of bloop. feature task / compile task / test
Projects
None yet
Development

No branches or pull requests

4 participants