From 97e20bc579c16b5f08d8f29eb4054192c8187d8a Mon Sep 17 00:00:00 2001 From: paul cannon Date: Fri, 10 Mar 2023 13:45:01 -0600 Subject: [PATCH] scripts/tests: fix rollingupgrade test even more This might be pretty awful, but at least it is a complete and non-flaky solution. **Only when using the rollingupgrade test** (which implies a throwaway satellite and also a PostgreSQL backend), create a trigger on the nodes table which forces last_net to be equal to last_ip_port always. Change-Id: I8448cf131e46576d96a414d06780270c7b2b1892 --- satellite/satellitedb/overlaycache.go | 29 +++++++++++++++---- .../test-sim-rolling-upgrade.sh | 3 +- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/satellite/satellitedb/overlaycache.go b/satellite/satellitedb/overlaycache.go index bf307b42c..e1b4dc460 100644 --- a/satellite/satellitedb/overlaycache.go +++ b/satellite/satellitedb/overlaycache.go @@ -1685,11 +1685,30 @@ func (n *noiseScanner) Convert() *pb.NoiseInfo { // OneTimeFixLastNets updates the last_net values for all node records to be equal to their // last_ip_port values. // -// This is only appropriate to do when the satellite has DistinctIP=false and has been upgraded from -// before changeset I0e7e92498c3da768df5b4d5fb213dcd2d4862924. It is only necessary to run this -// once, after all satellite peers are upgraded (otherwise some nodes may be inserted with truncated -// last_net values, and those nodes could be unfairly disadvantaged in node selection). +// This is only appropriate to do when the satellite has DistinctIP=false, the satelliteDB is +// running on PostgreSQL (not CockroachDB), and has been upgraded from before changeset +// I0e7e92498c3da768df5b4d5fb213dcd2d4862924. These are not common circumstances, but they do +// exist. It is only necessary to run this once. When all satellite peer processes are upgraded, +// the function and trigger can be dropped if desired. func (cache *overlaycache) OneTimeFixLastNets(ctx context.Context) error { - _, err := cache.db.ExecContext(ctx, "UPDATE nodes SET last_net = last_ip_port") + _, err := cache.db.ExecContext(ctx, ` + CREATE OR REPLACE FUNCTION fix_last_nets() + RETURNS TRIGGER + LANGUAGE plpgsql AS $$ + BEGIN + NEW.last_net = NEW.last_ip_port; + RETURN NEW; + END + $$; + + CREATE TRIGGER nodes_fix_last_nets + BEFORE INSERT OR UPDATE ON nodes + FOR EACH ROW + EXECUTE PROCEDURE fix_last_nets(); + `) + if err != nil { + return Error.Wrap(err) + } + _, err = cache.db.ExecContext(ctx, "UPDATE nodes SET last_net = last_ip_port") return Error.Wrap(err) } diff --git a/scripts/tests/rollingupgrade/test-sim-rolling-upgrade.sh b/scripts/tests/rollingupgrade/test-sim-rolling-upgrade.sh index bad776365..263703cfe 100755 --- a/scripts/tests/rollingupgrade/test-sim-rolling-upgrade.sh +++ b/scripts/tests/rollingupgrade/test-sim-rolling-upgrade.sh @@ -288,6 +288,7 @@ echo -e "\nRunning stage 2." fix_last_nets() { $(version_dir ${stage2_sat_version})/bin/satellite --config-dir "${test_dir}/local-network/satellite/0" fix-last-nets } +fix_last_nets # Starting old satellite api in the background has_marketing_server=$(echo $stage1_sat_version | awk 'BEGIN{FS="[v.]"} ($2 == 1 && $3 <= 22) || $2 == 0 {print $0}') @@ -314,12 +315,10 @@ for ul_version in ${stage2_uplink_versions}; do echo "Stage 2 uplink version: ${ul_version}" src_ul_version_dir=$(version_dir ${ul_version}) ln -f ${src_ul_version_dir}/bin/uplink $test_dir/bin/uplink - fix_last_nets PATH=$test_dir/bin:$PATH storj-sim -x --host "${STORJ_NETWORK_HOST4}" --config-dir "${test_dir}/local-network" network test bash "${scriptdir}/test-rolling-upgrade.sh" "${test_dir}/local-network" "${stage1_uplink_version}" "$update_access_script_path" if [[ $ul_version == $current_commit ]];then echo "Running final upload/download test on $current_commit" - fix_last_nets PATH=$test_dir/bin:$PATH storj-sim -x --host "${STORJ_NETWORK_HOST4}" --config-dir "${test_dir}/local-network" network test bash "${scriptdir}/test-rolling-upgrade-final-upload.sh" "${test_dir}/local-network" fi done