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

ERL-1410: relup remove_application is ordered before apps depending on it #4022

Closed
OTP-Maintainer opened this issue Nov 18, 2020 · 3 comments
Assignees
Labels
bug Issue is reported as a bug priority:medium team:VM Assigned to OTP team VM
Milestone

Comments

@OTP-Maintainer
Copy link

Original reporter: JIRAUSER16906
Affected version: Not Specified
Fixed in version: OTP-23.3
Component: sasl
Migrated from: https://bugs.erlang.org/browse/ERL-1410


While using rebar3 relup to generate relup file, 
 we noticed if an app is removed from the release,
 the app stop and code purge instructions are ordered before apps which depend on it.

In our case, we have app1 depending on app2,
 in the release which have app2 replaced with app3,
 in the generated relup it instructs to remove app2 before app1 is restarted.
 i.e. app2 is stopped, have all its modules purged, and application unloaded.
 This causes app1 to fail at its stop step because of the dependency.

I am not entirely sure if this is intended or by accident:
 maybe app removals should be ordered the last in the generated instructions?
 i.e. maybe it should be `RU0s ++ RUs` here: 
[https://github.com/erlang/otp/blob/f1ee84f8bb7b642cb5b6b5fd8f96119676c6ffa3/lib/sasl/src/systools_relup.erl#L462]


{code:java}
create_remove_app_scripts(FromRel, ToRel, RU0s, W0s) ->
    RemovedNs = [N || {N, _V, _T} <- FromRel#release.applications,
              not lists:keymember(N, 1, ToRel#release.applications)],
    %% io:format("Removed apps: ~p~n", [RemovedNs]),
    RUs = [[{remove_application, N}] || N <- RemovedNs],
    {RUs ++ RU0s, W0s}.
{code}
edit: updated with a permalink to the line.
@OTP-Maintainer
Copy link
Author

lukas said:

I have a vague recollection of reading somewhere that it should not be possible to replace an application in an upgrade, instead, you have to first add the new dependency and then in a later upgrade remove the old dependency. However, I cannot find anything in the docs about that, so not sure where I got that from.

Doing what you suggest seems to be a good solution to the problem so I've opened up a PR with that change: https://github.com/erlang/otp/pull/2882

@OTP-Maintainer
Copy link
Author

lukas said:

It turned out to be a lot harder to fix this problem than I initially thought. Your small fix solves one specific problem, but in order to properly replace an application dependency more work is needed in systools.

I think for now you will have to do this type of thing by doing a stages upgrade, i.e. first upgrade to a version where the application depends on both and then remove the extra dependency in the next upgrade. Or something similar to that.

@OTP-Maintainer
Copy link
Author

lukas said:

PR merged

@OTP-Maintainer OTP-Maintainer added bug Issue is reported as a bug team:VM Assigned to OTP team VM priority:medium labels Feb 10, 2021
@OTP-Maintainer OTP-Maintainer added this to the OTP-23.3 milestone Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug priority:medium team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

2 participants