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

Differentiate by Mill version when selecting a server worker #2639

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

lefou
Copy link
Member

@lefou lefou commented Jun 27, 2023

In older Mill versions, we tried to detect when the Mill version changed. In that case, we stopped the server process and started a new one. This server stopping worked unrelieble and is currently not working at all.

This pragmatic change simply does not try to re-use a mill server process when the Mill version does not match. Instead, we only choose between instances with the exact version or start a new instance. This is identical to the beavior, when we run with a differnet JVM.

Resource wise, we potentially increase the amount of running processes, but we do this anyways in other situations. Also, idle Mill servers stop themselves after a some time span.

In older Mill versions, we tried to detect when the Mill version changed. In that case, we stopped the server process and started a new one. This server stopping worked unrelieble and is currently not working at all.

This pragmatic change simply does not try to re-use a mill server process when the Mill version does not match. Instead, we only choose between instances with the exact version or start a new instance. This is identical to the beavior, when we run with a differnet JVM.

Resource wise, we potentially increate the amount of running processes, but we do this anyways in other situations. Also, idle Mill servers stop themselves after a some time span.
Copy link
Member

@lihaoyi lihaoyi left a comment

Choose a reason for hiding this comment

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

Looks fine. Should we leave the mill version un-hashes? Might make it easier to inspect and see what's going on if the mill version was in plaintext as part lf the file path

@lefou
Copy link
Member Author

lefou commented Jun 27, 2023

Looks fine. Should we leave the mill version un-hashes? Might make it easier to inspect and see what's going on if the mill version was in plaintext as part lf the file path

Would be fine by me. That was actually my first approach, but the resulting name was rather long. For more insights, we could also leave some info as file in the directory, e.g. start time, Mill version, Java details, maybe even a history of executed tasks.

@lefou
Copy link
Member Author

lefou commented Jun 27, 2023

For some reasons, I always get an exception, when I include the plain Mill version in the Mill worker directory:

-        String versionAndJvmHomeEncoding = Util.sha1Hash(BuildInfo.millVersion + System.getProperty("java.home"));
+        final String versionAndJvmHomeEncoding = BuildInfo.millVersion + "-" + Util.sha1Hash(System.getProperty("java.home"));
+        System.err.println("==> " + versionAndJvmHomeEncoding);
MILL_VERSION=0.11.1-3-d55c29-DIRTY34f18ee7 mill resolve _
==> 0.11.1-3-d55c29-DIRTY34f18ee7-jI8OnFgMTiLMP+r5K66NDMQ65lc=
Exception in thread "main" java.net.SocketException: Datei oder Verzeichnis nicht gefunden
        at org.newsclub.net.unix.NativeUnixSocket.bind(Native Method)
        at org.newsclub.net.unix.AFSocketImpl.bind(AFSocketImpl.java:336)
        at org.newsclub.net.unix.AFServerSocket.bind(AFServerSocket.java:257)
        at java.base/java.net.ServerSocket.bind(ServerSocket.java:349)
        at org.newsclub.net.unix.AFServerSocket.bindOn(AFServerSocket.java:176)
        at org.newsclub.net.unix.AFUNIXServerSocket.bindOn(AFUNIXServerSocket.java:87)
        at mill.runner.Server.$anonfun$run$2(MillServerMain.scala:107)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
        at mill.runner.Server$.lockBlock(MillServerMain.scala:231)
        at mill.runner.Server.$anonfun$run$1(MillServerMain.scala:102)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
        at mill.runner.Server$.tryLockBlock(MillServerMain.scala:239)
        at mill.runner.Server.run(MillServerMain.scala:99)
        at mill.runner.MillServerMain$.main(MillServerMain.scala:60)
        at mill.runner.MillServerMain.main(MillServerMain.scala)
Exception in thread "main" java.lang.Exception: Failed to connect to server
        at mill.main.client.MillClientMain.run(MillClientMain.java:197)
        at mill.main.client.MillClientMain.main0(MillClientMain.java:123)
        at mill.main.client.MillClientMain.main(MillClientMain.java:71)
Caused by: java.net.SocketException: 
        at org.newsclub.net.unix.NativeUnixSocket.connect(Native Method)
        at org.newsclub.net.unix.AFSocketImpl.connect0(AFSocketImpl.java:393)
        at org.newsclub.net.unix.AFSocket.connect0(AFSocket.java:267)
        at org.newsclub.net.unix.AFSocket.connect(AFSocket.java:236)
        at org.newsclub.net.unix.AFSocket.connect(AFSocket.java:231)
        at org.newsclub.net.unix.AFSocket.connectTo(AFSocket.java:179)
        at org.newsclub.net.unix.AFUNIXSocket.connectTo(AFUNIXSocket.java:109)
        at mill.main.client.MillClientMain.run(MillClientMain.java:189)
        ... 2 more

Maybe the dots. I don't know. To keep things simple, I will just keep it in the hashed form, which works as expected.

@lefou lefou merged commit 916a5cd into main Jun 27, 2023
@lefou lefou deleted the server-version-change branch June 27, 2023 20:22
@lefou lefou added this to the 0.11.2 milestone Jun 27, 2023
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.

2 participants