Merge pull request #93 from seriyps/code-change

Add `code_change` implementation and `appup` and hot upgrade test

.github/workflows/hot_upgrade.yml

@@ -0,0 +1,35 @@
+name: Hot upgrade
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+jobs:
+  ci:
+    name: Hot code upgrade from ${{ matrix.from_version }} on OTP-${{ matrix.otp }}
+    runs-on: ${{matrix.os}}
+
+    strategy:
+      fail-fast: true
+      matrix:
+        os: ["ubuntu-20.04"]
+        otp: ["23.3"]
+        rebar3: ["3.17.0"]
+        from_version:
+            - "1.5.2"
+            - "9c28fb479f9329e2a1644565a632bc222780f1b7"
+
+    steps:
+      - uses: actions/checkout@v3
+        with:
+            fetch-depth: 0
+
+      - uses: erlef/setup-beam@v1
+        with:
+          otp-version: ${{ matrix.otp }}
+          rebar3-version: ${{ matrix.rebar3 }}
+
+      - name: Hot-upgrade
+        run: make hotupgrade_setup BASE_REV=${{ matrix.from_version }} hotupgrade_check

Makefile

@@ -42,6 +42,12 @@ clean: $(REBAR)
 dialyzer: $(REBAR)
 	$(REBAR) dialyzer
 
+hotupgrade_setup: $(REBAR)
+	./test/hotupgrade_test.sh setup $(BASE_REV)
+
+hotupgrade_check:
+	./test/hotupgrade_test.sh check
+
 # Get rebar3 if it doesn't exist. If rebar3 was found on PATH, the
 # $(REBAR) dep will be satisfied since the file will exist.
 

README.org

@@ -347,6 +347,20 @@ Please attach the output of ~rebar3 bench --baseline master~ after your changes
 in order to prove that there were no performance regressions. Please attach the OTP version you run the
 benchmarks on.
 
+*** New release
+
+Our goal is to allow the hot code upgrade of ~pooler~, so it is shipped with ~.appup~ file and hot upgrade
+procedure is tested in CI.
+
+To cut a new release, do the following steps:
+
+1. In ~src/pooler.app.src~: update the ~vsn~
+2. In ~src/pooler.appup.src~: replace the contents with upgrade instructions for a new release
+3. In ~test/relx-base.config~: update the ~pooler~'s app version to a previous release (or leave it without version)
+4. In ~test/relx-current.config~: update the ~pooler~'s app version to a new one
+5. In ~.github/workflows/hot_upgrade.yml~: update ~from_version~ to a previous release, maybe bump OTP version as well
+6. Push, wait for the green build, tag
+
 ** License
 Pooler is licensed under the Apache License Version 2.0.  See the
 [[file:LICENSE][LICENSE]] file for details.

src/pooler.app.src

@@ -1,11 +1,13 @@
 {application, pooler, [
     {description, "An OTP Process Pool Application"},
-    {vsn, git},
-    {registered, []},
+    {vsn, "1.6.0"},
+    {registered, [pooler_sup, pooler_starter_sup]},
     {applications, [
         kernel,
         stdlib
     ]},
     {mod, {pooler_app, []}},
-    {env, []}
+    {env, []},
+    {licenses, ["Apache License 2.0"]},
+    {links, [{"Github", "https://github.com/epgsql/pooler"}]}
 ]}.

src/pooler.appup.src

@@ -0,0 +1,24 @@
+% -*- mode: erlang -*-
+{"1.6.0",
+  [{<<"1\\.5\\.2.*">>,
+    [{update, pooler, {advanced, []}},
+     {load_module, pooler_starter},
+     {load_module, pooler_app},
+     {update, pooler_sup, supervisor},
+     {update, pooler_pool_sup, supervisor},
+     {update, pooler_starter_sup, supervisor},
+     {update, pooler_pooled_worker_sup, supervisor},
+     {delete_module, pooler_config}]
+   },
+   {<<"1\\.5\\.3.*">>,
+    [{update, pooler, {advanced, []}},
+     {load_module, pooler_starter},
+     {load_module, pooler_app},
+     {update, pooler_sup, supervisor},
+     {update, pooler_pool_sup, supervisor},
+     {update, pooler_starter_sup, supervisor},
+     {update, pooler_pooled_worker_sup, supervisor},
+     {delete_module, pooler_config}]
+   }
+   ],
+  [{<<".*">>, []}]}.

src/pooler.erl

@@ -43,7 +43,7 @@
     call_free_members/2,
     call_free_members/3
 ]).
--export([create_group_table/0, config_as_map/1]).
+-export([create_group_table/0, config_as_map/1, to_map/1]).
 
 %% ------------------------------------------------------------------
 %% gen_server Function Exports
@@ -59,6 +59,9 @@
     code_change/3
 ]).
 
+-vsn(2).
+%% Bump this value and add a new clause to `code_change', if the format of `#pool{}' record changed
+
 %% ------------------------------------------------------------------
 %% Types
 %% ------------------------------------------------------------------
@@ -606,7 +609,40 @@ terminate(_Reason, _State) ->
     ok.
 
 -spec code_change(_, _, _) -> {'ok', _}.
-code_change(_OldVsn, State, _Extra) ->
+code_change(
+    _OldVsn,
+    {pool, Name, Group, MaxCount, InitCount, StartMFA, FreePids, InUseCount, FreeCount, AddMemberRetry, CullInterval,
+        MaxAge, MemberSup, StarterSup, AllMembers, ConsumerToPid, StartingMembers, MemberStartTimeout,
+        AutoGrowThreshold, StopMFA, MetricsMod, MetricsAPI, QueuedRequestors, QueueMax},
+    _Extra
+) ->
+    {ok, #pool{
+        cull_timer = undefined,
+        all_members = maps:from_list(dict:to_list(AllMembers)),
+        consumer_to_pid = maps:from_list(dict:to_list(ConsumerToPid)),
+        name = Name,
+        group = Group,
+        max_count = MaxCount,
+        init_count = InitCount,
+        start_mfa = StartMFA,
+        free_pids = FreePids,
+        in_use_count = InUseCount,
+        free_count = FreeCount,
+        add_member_retry = AddMemberRetry,
+        cull_interval = CullInterval,
+        max_age = MaxAge,
+        member_sup = MemberSup,
+        starter_sup = StarterSup,
+        starting_members = StartingMembers,
+        member_start_timeout = MemberStartTimeout,
+        auto_grow_threshold = AutoGrowThreshold,
+        stop_mfa = StopMFA,
+        metrics_mod = MetricsMod,
+        metrics_api = MetricsAPI,
+        queued_requestors = QueuedRequestors,
+        queue_max = QueueMax
+    }};
+code_change(_, State, _Extra) ->
     {ok, State}.
 
 %% ------------------------------------------------------------------
@@ -1311,6 +1347,7 @@ do_call_free_member(Fun, Pid) ->
             {error, Reason}
     end.
 
+%% @private
 to_map(#pool{} = Pool) ->
     [Name | Values] = tuple_to_list(Pool),
     maps:from_list(

src/pooler_pool_sup.erl

@@ -15,17 +15,57 @@ start_link(PoolConf) ->
     SupName = pool_sup_name(PoolConf),
     supervisor:start_link({local, SupName}, ?MODULE, PoolConf).
 
-init(PoolConf) ->
-    PoolerSpec = {pooler, {pooler, start_link, [PoolConf]}, transient, 5000, worker, [pooler]},
+init(PoolConf) when is_map(PoolConf) ->
+    PoolerSpec = #{
+        id => pooler,
+        start => {pooler, start_link, [PoolConf]},
+        restart => transient,
+        shutdown => 5000,
+        type => worker,
+        modules => [pooler]
+    },
     MemberSupName = member_sup_name(PoolConf),
     MemberSupSpec =
-        {MemberSupName, {pooler_pooled_worker_sup, start_link, [PoolConf]}, transient, 5000, supervisor, [
-            pooler_pooled_worker_sup
-        ]},
+        #{
+            id => MemberSupName,
+            start => {pooler_pooled_worker_sup, start_link, [PoolConf]},
+            restart => transient,
+            shutdown => 5000,
+            type => supervisor,
+            modules => [pooler_pooled_worker_sup]
+        },
 
     %% five restarts in 60 seconds, then shutdown
-    Restart = {one_for_all, 5, 60},
-    {ok, {Restart, [MemberSupSpec, PoolerSpec]}}.
+    Restart = #{strategy => one_for_all, intensity => 5, period => 60},
+    {ok, {Restart, [MemberSupSpec, PoolerSpec]}};
+init(PoolRecord) when is_tuple(PoolRecord), element(1, PoolRecord) =:= pool ->
+    %% This clause is for the hot code upgrade from pre-1.6.0;
+    %% can be removed when "upgrade-from-version" below 1.6.0 are removed from `pooler.appup.src'
+    {ok, PoolRecord1} = pooler:code_change(0, PoolRecord, []),
+    AsMap = pooler:to_map(PoolRecord1),
+    init(
+        maps:with(
+            [
+                name,
+                init_count,
+                max_count,
+                start_mfa,
+                group,
+                cull_interval,
+                max_age,
+                member_start_timeout,
+                queue_max,
+                metrics_api,
+                metrics_mod,
+                stop_mfa,
+                auto_grow_threshold,
+                add_member_retry,
+                metrics_mod,
+                metrics_api
+            ],
+            AsMap
+        )
+    ).
 
 -spec member_sup_name(pooler:pool_config()) -> atom().
 member_sup_name(#{name := Name}) ->

src/pooler_pooled_worker_sup.erl

@@ -10,7 +10,14 @@ start_link(#{start_mfa := MFA} = PoolConf) ->
     supervisor:start_link({local, SupName}, ?MODULE, MFA).
 
 init({Mod, Fun, Args}) ->
-    Worker = {Mod, {Mod, Fun, Args}, temporary, brutal_kill, worker, [Mod]},
+    Worker = #{
+        id => Mod,
+        start => {Mod, Fun, Args},
+        restart => temporary,
+        shutdown => brutal_kill,
+        type => worker,
+        modules => [Mod]
+    },
     Specs = [Worker],
-    Restart = {simple_one_for_one, 1, 1},
+    Restart = #{strategy => simple_one_for_one, intensity => 1, period => 1},
     {ok, {Restart, Specs}}.

src/pooler_starter_sup.erl

@@ -20,7 +20,14 @@ start_link() ->
     supervisor:start_link({local, ?MODULE}, ?MODULE, []).
 
 init([]) ->
-    Worker = {pooler_starter, {pooler_starter, start_link, []}, temporary, brutal_kill, worker, [pooler_starter]},
+    Worker = #{
+        id => pooler_starter,
+        start => {pooler_starter, start_link, []},
+        restart => temporary,
+        shutdown => brutal_kill,
+        type => worker,
+        modules => [pooler_starter]
+    },
     Specs = [Worker],
-    Restart = {simple_one_for_one, 1, 1},
+    Restart = #{strategy => simple_one_for_one, intensity => 1, period => 1},
     {ok, {Restart, Specs}}.

src/pooler_sup.erl

@@ -10,6 +10,8 @@
     start_link/0
 ]).
 
+-include_lib("kernel/include/logger.hrl").
+
 start_link() ->
     supervisor:start_link({local, ?MODULE}, ?MODULE, []).
 
@@ -30,8 +32,20 @@ init([]) ->
         })
      || Config <- Configs
     ],
-    pooler:create_group_table(),
-    {ok, {{one_for_one, 5, 60}, [starter_sup_spec() | PoolSupSpecs]}}.
+    try
+        pooler:create_group_table()
+    catch
+        error:badarg:Stack ->
+            ?LOG_ERROR(
+                #{
+                    label => "Failed to start pool groups ETS table",
+                    reason => badarg,
+                    stack => Stack
+                },
+                #{domain => [pooler]}
+            )
+    end,
+    {ok, {#{strategy => one_for_one, intensity => 5, period => 60}, [starter_sup_spec() | PoolSupSpecs]}}.
 
 %% @doc Create a new pool from proplist pool config `PoolConfig'. The
 %% public API for this functionality is {@link pooler:new_pool/1}.
@@ -61,11 +75,25 @@ rm_pool(Name) ->
     end.
 
 starter_sup_spec() ->
-    {pooler_starter_sup, {pooler_starter_sup, start_link, []}, transient, 5000, supervisor, [pooler_starter_sup]}.
+    #{
+        id => pooler_starter_sup,
+        start => {pooler_starter_sup, start_link, []},
+        restart => transient,
+        shutdown => 5000,
+        type => supervisor,
+        modules => [pooler_starter_sup]
+    }.
 
 pool_sup_spec(#{name := Name} = PoolConfig) ->
     SupName = pool_sup_name(Name),
-    {SupName, {pooler_pool_sup, start_link, [PoolConfig]}, transient, 5000, supervisor, [pooler_pool_sup]}.
+    #{
+        id => SupName,
+        start => {pooler_pool_sup, start_link, [PoolConfig]},
+        restart => transient,
+        shutdown => 5000,
+        type => supervisor,
+        modules => [pooler_pool_sup]
+    }.
 
 pool_sup_name(Name) ->
     list_to_atom("pooler_" ++ atom_to_list(Name) ++ "_pool_sup").

test/hotupgrade_test.sh

@@ -0,0 +1,61 @@
+#!/bin/sh
+#
+# Script to test release hot code upgrade
+#
+# Usage:
+#   # to generate and launch previous release, then generate a new release and upgrade files
+#   ./test/hotupgrade_test.sh setup <revision-to-upgrade-from>
+#   # to upgrade the release and perform sanity checks
+#   ./test/hotupgrade_test.sh check
+
+set -e
+set -x
+
+REBAR=`which rebar3 || echo ./rebar3`
+COMMAND=$1
+BASE_REV=$2
+
+do_setup() {
+    CUR_VERSION=`git log -1 --format='%H'`   #`git rev-parse --abbrev-ref HEAD`
+    # Building the release for the OLD version
+    if [ ! -d ebin ]; then mkdir ebin; fi
+    cp test/relx-base.config ./relx.config
+    cp src/pooler.appup.src ebin/pooler.appup
+    git checkout $BASE_REV
+    if [ ! -e src/pooled_gs.erl ]; then cp test/pooled_gs.erl src/; fi
+    $REBAR as test release
+
+    ./_build/test/rel/pooler_test/bin/pooler_test daemon
+
+    git clean -df
+
+    # Building the release and relup file with the NEW version
+    git checkout $CUR_VERSION
+    if [ ! -e src/pooled_gs.erl ]; then cp test/pooled_gs.erl src/; fi
+    $REBAR as test release --config test/relx-current.config
+    $REBAR as test relup   --config test/relx-current.config --relname pooler_test --relvsn=2.0.0 --upfrom=1.0.0
+    $REBAR as test tar     --config test/relx-current.config
+    cp _build/test/rel/pooler_test/pooler_test-2.0.0.tar.gz _build/test/rel/pooler_test/releases/
+    ./_build/test/rel/pooler_test/bin/pooler_test-1.0.0 unpack 2.0.0
+}
+
+TEST='Taken=[begin P = pooler:take_member(pool1, 1000), is_pid(P) orelse error(P), P end || _ <- lists:seq(1, 5)], [pooler:return_member(pool1, P, fail) || P <- Taken], ok.'
+
+do_check() {
+    RES1=`./_build/test/rel/pooler_test/bin/pooler_test-1.0.0 eval "${TEST}"`
+    if [ "$RES1" != "ok" ]; then
+        echo "Before upgrade checkout failed" >&2
+        echo $RES1 >&2
+        exit 1
+    fi
+    ./_build/test/rel/pooler_test/bin/pooler_test-1.0.0 upgrade 2.0.0
+    RES2=`./_build/test/rel/pooler_test/bin/pooler_test eval "${TEST}"`
+    if [ "$RES2" != "ok" ]; then
+        echo "After upgrade checkout failed" >&2
+        echo $RES2 >&2
+        exit 1
+    fi
+    ./_build/test/rel/pooler_test/bin/pooler_test stop
+}
+
+"do_$COMMAND"

test/relx-base.config

@@ -0,0 +1,4 @@
+{release, {pooler_test, "1.0.0"}, [sasl, pooler, runtime_tools]}.
+{sys_config, "config/demo.config"}.
+{extended_start_script, true}.
+{include_erts, false}.

test/relx-current.config

@@ -0,0 +1,4 @@
+{release, {pooler_test, "2.0.0"}, [sasl, {pooler, "1.6.0"}, runtime_tools]}.
+{sys_config, "config/demo.config"}.
+{extended_start_script, true}.
+{include_erts, false}.