You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/01/20 23:13:59 UTC

[GitHub] [couchdb] kocolosk opened a new pull request #3337: Simplify and speedup dev node startup

kocolosk opened a new pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337


   ## Overview
   
   This patch introduces an escript that generates an Erlang .boot script to start CouchDB using the in-place .beam files produced by the compile phase of the build. This allows us to radically simplify the boot process for dev nodes as Erlang computes the optimal order for loading the necessary modules, and the previous logic in `boot_node` was, well, _not_ optimal 😄 
       
   In addition to the simplification this approach offers a nice startup time speedup, at least when working inside a container environment. In my test with the stock .devcontainer it reduces startup time from about 75 seconds down to under 5 seconds 🚀 
   
   I had initially introduced a new `devnode` target in the Makefile but figured just invoking the script if the `devnode.boot` file is missing would be sufficient. Note that this file can become outdated on the following conditions:
   
   - Change in Erlang version 
   - A new application is introduced to the source tree
   - Modules are removed from existing applications
   
   In each case running `make clean` or simply deleting the `devnode.boot` file will fix it. We could generate it every time but I figured it was maybe overkill for infrequent occurrences, and `make clean` is an easy fix.
   
   ## Testing recommendations
   
   Start a dev node with `./dev/run`. Try doing this after a `make clean; make` cycle and you should see a ~5 second delay during "Ensure Erlang boot script exists". Subsequent runs should be instantaneous at that step.
   
   ## Related Issues or Pull Requests
   
   <!-- If your changes affects multiple components in different
        repositories please put links to those issues or pull requests here.  -->
   
   ## Checklist
   
   - [x] Code is written and works correctly
   - [ ] Changes are covered by tests
   - [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
   - [ ] A PR for documentation changes has been made in https://github.com/apache/couchdb-documentation
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on a change in pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
kocolosk commented on a change in pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#discussion_r561370587



##########
File path: dev/run
##########
@@ -592,10 +599,9 @@ def set_boot_env(ctx):
 
 @log("Start node {node}")
 def boot_node(ctx, node):
-    erl_libs = os.path.join(ctx["rootdir"], "src")
     set_boot_env(ctx)
     env = os.environ.copy()
-    env["ERL_LIBS"] = os.pathsep.join([erl_libs])
+    env["ERL_LIBS"] = os.path.join(ctx["rootdir"], "src")

Review comment:
       I wasn't sure what was going on here with the `os.pathsep.join` piece, since there's only one path. It felt like a vestigial thing. I cleaned it up to match the same dance being done in the `check_boot_script` method.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] davisp commented on pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
davisp commented on pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#issuecomment-764744735


   I can push that commit if you like.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk merged pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
kocolosk merged pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] rnewson commented on pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
rnewson commented on pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#issuecomment-764791865


   +1


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk merged pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
kocolosk merged pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on a change in pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
kocolosk commented on a change in pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#discussion_r561369979



##########
File path: dev/make_boot_script
##########
@@ -0,0 +1,9 @@
+#!/usr/bin/env escript
+
+main(_) ->
+    {ok, Server} = reltool:start_server([
+        {config, "../rel/reltool.config"}
+    ]),
+    {ok, Release} = reltool:get_rel(Server, "couchdb"),
+    ok = file:write_file("devnode.rel", io_lib:format("~p.~n", [Release])),
+    ok = systools:make_script("devnode", [local]).

Review comment:
       This is really where the magic happens -- we take the existing `reltool.config` file and generate a .boot script with the `local` option, which automatically uses the existing paths for all the modules instead of relocatable ones like we have in the actual release.

##########
File path: dev/run
##########
@@ -592,10 +599,9 @@ def set_boot_env(ctx):
 
 @log("Start node {node}")
 def boot_node(ctx, node):
-    erl_libs = os.path.join(ctx["rootdir"], "src")
     set_boot_env(ctx)
     env = os.environ.copy()
-    env["ERL_LIBS"] = os.pathsep.join([erl_libs])
+    env["ERL_LIBS"] = os.path.join(ctx["rootdir"], "src")

Review comment:
       I wasn't sure what was going on here with the `os.pathsep.join` piece, since there's only one path. It felt like a vestigial thing. I cleaned it up to match the same dance being done in the `check_boot_script` method.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] rnewson commented on pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
rnewson commented on pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#issuecomment-764748069






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] rnewson commented on pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
rnewson commented on pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#issuecomment-764748069


   @davisp go ahead.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] davisp commented on pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
davisp commented on pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#issuecomment-764767483


   Also done for main as well.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] kocolosk commented on a change in pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
kocolosk commented on a change in pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#discussion_r561369979



##########
File path: dev/make_boot_script
##########
@@ -0,0 +1,9 @@
+#!/usr/bin/env escript
+
+main(_) ->
+    {ok, Server} = reltool:start_server([
+        {config, "../rel/reltool.config"}
+    ]),
+    {ok, Release} = reltool:get_rel(Server, "couchdb"),
+    ok = file:write_file("devnode.rel", io_lib:format("~p.~n", [Release])),
+    ok = systools:make_script("devnode", [local]).

Review comment:
       This is really where the magic happens -- we take the existing `reltool.config` file and generate a .boot script with the `local` option, which automatically uses the existing paths for all the modules instead of relocatable ones like we have in the actual release.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] davisp commented on pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
davisp commented on pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#issuecomment-764744445


   It looks like this is the diff that black (the python formatter/linter) is suggesting:
   
   ```diff
   diff --git a/dev/run b/dev/run
   index 476ec3682..7af718afa 100755
   --- a/dev/run
   +++ b/dev/run
   @@ -190,7 +190,7 @@ def get_args_parser():
            "--erlang-config",
            dest="erlang_config",
            default="rel/files/sys.config",
   -        help="Specify an alternative Erlang application configuration"
   +        help="Specify an alternative Erlang application configuration",
        )
        parser.add_option(
            "--degrade-cluster",
   @@ -275,6 +275,7 @@ def check_beams(ctx):
        for fname in glob.glob(os.path.join(ctx["devdir"], "*.erl")):
            sp.check_call(["erlc", "-o", ctx["devdir"] + os.sep, fname])
    
   +
    @log("Ensure Erlang boot script exists")
    def check_boot_script(ctx):
        if not os.path.exists(os.path.join(ctx["devdir"], "devnode.boot")):
   @@ -282,6 +283,7 @@ def check_boot_script(ctx):
            env["ERL_LIBS"] = os.path.join(ctx["rootdir"], "src")
            sp.check_call(["escript", "make_boot_script"], env=env, cwd=ctx["devdir"])
    
   +
    @log("Prepare configuration files")
    def setup_configs(ctx):
        for idx, node in enumerate(ctx["nodes"]):
   @@ -631,7 +633,7 @@ def boot_node(ctx, node):
            os.path.join(ctx["devdir"], "devnode"),
            "-pa",
            ctx["devdir"],
   -        "-s monitor_parent"
   +        "-s monitor_parent",
        ]
        if ctx["reset_logs"]:
            mode = "wb"
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] davisp commented on pull request #3337: Simplify and speedup dev node startup

Posted by GitBox <gi...@apache.org>.
davisp commented on pull request #3337:
URL: https://github.com/apache/couchdb/pull/3337#issuecomment-764744445






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org