basho / riak

Riak is a decentralized datastore from Basho Technologies.
http://docs.basho.com
Apache License 2.0
3.92k stars 534 forks source link

WIP Upgrade to rebar3 3.18, with improvements in packaging and release assembly #1097

Closed hmmr closed 2 years ago

hmmr commented 2 years ago

This is a patchset inspired by the work I have been doing for some months on Riak CS/Stanchion 3.0. It includes:

There are no changes in Erlang code.

Testing was done as follows:

martinsumner commented 2 years ago

This seems to be failing for me to do both make rel and make devrel.

e.g.

make rel
rel/riak/bin/riak start
rel/riak/bin/riak ping

Returns no pong. Starting with riak console is OK.

hmmr commented 2 years ago

@martinsumner Indeed, riak doesn't start on the very first run (it somehow mends itself the second time and starts just fine on subsequent attempts though). Something to investigate -- thanks for alerting me to this.

In the meantime, can you add a tag (2.1.1 or maybe 2.2.0) somewhere in basho/cuttlefish post https://github.com/basho/cuttlefish/pull/249? This will help to get rid of non-basho dependencies in this PR.

martinsumner commented 2 years ago

Could you use the branch develop-3.0 reference on basho/cuttlefish in rebar.config? As a general rule, we try and use branch names on dependencies until the point we're cutting releases, then we go through and tag everything in one sweep at that point.

You're right that riak post make rel does work on the second start.

From a timeline perspective, I'm pulling together release 3.0.10 at the moment, just doing some specific performance tests required for the release. If we can solve any outstanding issues soon, I will check to see if we can add this PR in. The issue is finding someone to review/approve, as I don't feel confident I understand the changes well enough to OK it myself.

martinsumner commented 2 years ago

Some other issues:

Bob-The-Marauder commented 2 years ago

Sorry for being late to the party but regarding the hyphenated/non-hyphenated command issue, having both options working in at least 3.0 is strongly beneficiary for upgrading users who use automated scripts. If the hyphenated commands work then their scripts just continue working as before and there is nothing for them to worry about despite the scariness of a v2 to v3 upgrade. As Martin Sumner will probably verify, one of the common 3.0.x issues found in the Post Riak Slack channel is people trying to use the hyphenated commands in KV 3.0 and getting unexpected errors.

Despite the fact we are about to hit 3.0.10 (at time of writing), there are a lot of people currently on early 2.9.x releases who are currently considering upgrading to 3.0.x within the next 6 months to 18 months. Giving them the smoothest possible transition is probably the best way to gain continued loyalty to the platform.

If there is a really strong need to move exclusively to the non-hyphenated commands, I suggest we put a note in the version 3.0 documentation (currently being written) saying that the hyphenated commands will be deprecated and that from version 3.x.0 e.g. 3.1.0 or 3.2.0 that they will no longer be supported.

martincox commented 2 years ago

It's not that there's a strong need to remove the hyphenated scripts. More of a case that they have already been removed and the changes absorbed by two orgs with the largest deployments of riak.

Adding them back adds a bunch of extra boilerplate to scripts and counters the usefulness of start script extensions - there's no point in maintaining both together.

Perhaps if we had honoured both when the change was made, and eventually deprecated hyphenated versions, it might have been more preferred. But re-introducing several releases later makes little sense to me, and I'd strongly recommend we don't.

hmmr commented 2 years ago

To clarify:

I personally have no strong opinions here; it does seem to me, however, that maintaining hyphenation equivalence is good for 'pure' customers (those who install riak from packages and don't build riak themselves), and we should continue to offer the equivalent forms -- especially given that it's already there and comes at no cost.

martincox commented 2 years ago

If it is absolutely necessary to bridge the gap of hyphenated forms being deprecated, why not take a less invasive route and create proxy scripts for the hyphenated forms that live in /usr/sbin/ with something in the form of:

#!/bin/sh
echo "riak-admin command is now deprecated, use riak admin bla bla"
riak admin $@

It saves trying to shoehorn the hyphenated forms back into the release config, which will ultimately be removed later anyway. As I mentioned before, I see little point in maintaining both. An even simpler workaround might be to use aliases.

With regards to the PID writing - does that not work? It should be handled as part of the start script. I mean, I've just checked over a few clusters (3.*) and all have PIDs in the expected directory.

martincox commented 2 years ago

Also, just for my own clarity, what branch are we targeting with this PR? It's set to develop-3.0, but I'm not sure if is because that happened to be the default branch at the time it was created.

hmmr commented 2 years ago

This seems to be failing for me to do both make rel and make devrel.

e.g.

make rel
rel/riak/bin/riak start
rel/riak/bin/riak ping

Returns no pong. Starting with riak console is OK.

Fixed in https://github.com/basho/riak/pull/1097/commits/e6f64bfadf128d86c14a7ee1b68d4e0adb42d087

hmmr commented 2 years ago

@martincox Re:

If it is absolutely necessary to bridge the gap of hyphenated forms being deprecated, why not take a less invasive route and create proxy scripts for the hyphenated forms that live in /usr/sbin/ with something in the form of:

With https://github.com/basho/riak/pull/1097/commits/1ce026214e75470afb87f47929b45b5bd05ab51b, when installed from a package on a debian11 system, we now have:

root@debian:~# riak admin
Usage: riak-admin { cluster | join | leave | backup | restore | test | 
                    reip | erl-reload | wait-for-service | 
...
root@debian:~# riak-admin
The hyphenated form of this command is deprecated. Please fix your scripts to call `riak admin` instead.

Usage: riak-admin { cluster | join | leave | backup | restore | test | 
                    reip | erl-reload | wait-for-service | 
                    ringready | transfers | force-remove | down |

With regards to the PID writing - does that not work? It should be handled as part of the start script. I mean, I've just checked over a few clusters (3.*) and all have PIDs in the expected directory.

It's another possible regression. I have the extended_start_script_hooks stanza in relx/deb section in rebar.config, unchanged since 3.0.9, but if I delete this line, the pid file is not written. riak pid still works (pid is obtained by calling os:getpid/0, so no wonder). I will investigate.

martinsumner commented 2 years ago

@hmmr wanted to try test this today, but had issues fetching the referred to tag in riak_repl. Is the TI-Tokyo riak_repl repo public?

hmmr commented 2 years ago

@martinsumner It is indeed private. The change I pushed in TI-Tokyo repo is just s|git://|https://| on top of tag riak_kv-3.0.9. I guess you can do this change manually in the official repo, tag it riak_kv-3.0.10, and I will then re-point riak_repl to the new tag accordingly in this PR.

I have traced the pid file regression, which was in rebar3_cuttlefish plugin (it was introduced by vernemq people in their more recent changes, and adopted by too eager me). I have reverted to version 0.2.0, tweaking it to work with rebar3.18 (and noticed that @martincox has already done the same). I have just re-pointed rebar3_cuttlefish to basho/develop branch, and it is great again.

There are still issues with https://github.com/basho/riak/pull/1097/commits/f75df62e2e042f2313a387f1580c31eb3bd4b270, which I am trying to fix, but you should be able to build it locally with make rel.

hmmr commented 2 years ago

Superseded by https://github.com/basho/riak/pull/1108.