streamer, tests: migrate to non-overlapping ports

[puhtaytow]

Problem

A bunch of tests still relies on either hardcoded ports or bind_to_localhost and bind_to_unspecified. This creates flaky tests in cases where unique port ranges are needed.

Related to #6886 #7055

Summary of Changes

Migrated tests to non-overlapping ports.
This PR should close migration for the Streamer package.

[0xbrw]

@alexpyattaev or @apfitzge when you get a chance, can I have you take a look?

[apfitzge]

It looks correct to me on skim, but it's a bit verbose to do this everywhere. Can we not have a bind_to_localhost_for_tests that just accesses some static behind a mutex that is lazily initialized with this iterator? That'd make a lot of these simple function changes instead of having like 4-5 extra lines per change.

[puhtaytow]

It looks correct to me on skim, but it's a bit verbose to do this everywhere. Can we not have a bind_to_localhost_for_tests that just accesses some static behind a mutex that is lazily initialized with this iterator? That'd make a lot of these simple function changes instead of having like 4-5 extra lines per change.

I looked at the https://github.com/anza-xyz/agave/blob/master/net-utils/src/sockets.rs#L27-L45 and to me, the function does utilize the nextest env variable to coordinate the tests globally, there is also the atomic SLICE.. it looks like we could skip mutex in this case.

Obviously, correct me if im wrong 🙏

pub fn localhost_port_range_for_tests() -> (u16, u16) {
    static SLICE: AtomicU16 = AtomicU16::new(0);
    let offset = SLICE.fetch_add(20, Ordering::Relaxed);
    let start = offset
        + match std::env::var("NEXTEST_TEST_GLOBAL_SLOT") {
            Ok(slot) => {
                let slot: u16 = slot.parse().unwrap();
                assert!(
                    offset < SLICE_PER_PROCESS,
                    "Overrunning into the port range of another test! Consider using fewer ports per test."
                );
                BASE_PORT + slot * SLICE_PER_PROCESS
            }
            Err(_) => BASE_PORT,
        };
    assert!(start < u16::MAX - 20, "ran out of port numbers!");
    (start, start + 20)
}

To address the unnecessary verbosity, i introduced the helper function in net-utils as per your suggestion. https://github.com/anza-xyz/agave/compare/master...puhtaytow:agave:streamer-non-overlapping-socket-bind-api-1?expand=1

Please tell me if that is the correct place for it and if so then how would you like to merge the changes to net-utils, as separate PR?

Thank you for the suggestion 🙏 i do appreciate a lot any constructive input.

[alexpyattaev]

Please do not fix what is not broken =) localhost_port_range_for_tests uses both nextest env variables AND local atomics to coordinate port allocation because we do not know if it will run under cargo test or cargo nextest. cargo test uses 1 process for all tests => atomics work. cargo nextest uses process-per-test model, and thus we need to use env variables. One possible improvement for that function was suggested by t-nelson that would use shared memory instead of env variables for inter-process coordination, as that would allow the same mechanism to be used under both test runners.

[alexpyattaev]

Let us not create mutable iterators where direct access would achieve the same result=)

[puhtaytow]

@alexpyattaev thank you for pointing these out / fixed 🙏

[alexpyattaev]

LGTM thank you!

[puhtaytow]

LGTM thank you!

Thank you! 🙏

[alexpyattaev]

This is currently blocked by #6957

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.2%. Comparing base (7baf884) to head (11c04b2).
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #6895     +/-   ##
=========================================
- Coverage    83.2%    83.2%   -0.1%     
=========================================
  Files         852      852             
  Lines      373846   373845      -1     
=========================================
- Hits       311408   311353     -55     
- Misses      62438    62492     +54     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[alexpyattaev]

LGTM

[puhtaytow]

Thank you!