Graceful shutdown for mining-proxy #1021

johnnyasantoss · 2024-06-30T01:57:33Z

Add graceful shutdown for mining proxy fixing #484

PS: I'm not sure if I should merge against dev or main

github-actions · 2024-06-30T02:01:41Z

Bencher

Report	Wed, July 24, 2024 at 14:23:52 UTC
Project	Stratum v2 (SRI)
Branch	1021/merge
Testbed	sv1

Click to view all benchmark results

Benchmark	Latency	Latency Results nanoseconds (ns) \| (Δ%)	Latency Upper Boundary nanoseconds (ns) \| (%)
client-submit-serialize	✅ (view plot)	6,470.10 (-6.00%)	7,377.54 (87.70%)
client-submit-serialize-deserialize	✅ (view plot)	7,504.80 (-3.82%)	8,348.93 (89.89%)
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle	✅ (view plot)	8,044.30 (-3.98%)	8,851.37 (90.88%)
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle	✅ (view plot)	900.40 (-0.05%)	941.43 (95.64%)
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize	✅ (view plot)	699.22 (-0.18%)	738.79 (94.64%)
client-sv1-authorize-serialize/client-sv1-authorize-serialize	✅ (view plot)	247.49 (-0.40%)	257.83 (95.99%)
client-sv1-get-authorize/client-sv1-get-authorize	✅ (view plot)	159.07 (+0.66%)	170.50 (93.29%)
client-sv1-get-submit	✅ (view plot)	6,343.50 (-4.73%)	7,139.08 (88.86%)
client-sv1-get-subscribe/client-sv1-get-subscribe	✅ (view plot)	278.58 (-0.36%)	296.29 (94.02%)
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle	✅ (view plot)	770.45 (+2.66%)	786.78 (97.92%)
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize	✅ (view plot)	618.26 (+0.37%)	644.44 (95.94%)
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize	✅ (view plot)	207.21 (+0.21%)	219.53 (94.39%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

github-actions · 2024-06-30T02:01:41Z

Bencher

Report	Wed, July 24, 2024 at 14:23:48 UTC
Project	Stratum v2 (SRI)
Branch	johnny/issue/484
Testbed	sv2

Click to view all benchmark results

Benchmark	Latency	Latency Results nanoseconds (ns) \| (Δ%)	Latency Upper Boundary nanoseconds (ns) \| (%)
client_sv2_handle_message_common	✅ (view plot)	44.48 (-0.19%)	45.22 (98.35%)
client_sv2_handle_message_mining	✅ (view plot)	74.75 (+2.54%)	80.18 (93.24%)
client_sv2_mining_message_submit_standard	✅ (view plot)	14.65 (-0.04%)	14.69 (99.69%)
client_sv2_mining_message_submit_standard_serialize	✅ (view plot)	259.97 (-1.60%)	283.36 (91.75%)
client_sv2_mining_message_submit_standard_serialize_deserialize	✅ (view plot)	600.02 (+0.88%)	627.62 (95.60%)
client_sv2_open_channel	✅ (view plot)	174.27 (+4.78%)	174.33 (99.96%)
client_sv2_open_channel_serialize	✅ (view plot)	281.67 (-0.19%)	294.26 (95.72%)
client_sv2_open_channel_serialize_deserialize	✅ (view plot)	371.40 (-1.79%)	422.19 (87.97%)
client_sv2_setup_connection	✅ (view plot)	158.60 (-3.43%)	174.93 (90.66%)
client_sv2_setup_connection_serialize	✅ (view plot)	460.27 (-2.57%)	506.28 (90.91%)
client_sv2_setup_connection_serialize_deserialize	✅ (view plot)	988.42 (+1.51%)	1,043.87 (94.69%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

github-actions · 2024-06-30T02:01:42Z

Bencher

Report	Wed, July 24, 2024 at 14:23:48 UTC
Project	Stratum v2 (SRI)
Branch	johnny/issue/484
Testbed	sv1

🚨 2 ALERTS: Threshold Boundary Limits exceeded!

Benchmark	Measure (units)	View	Value	Lower Boundary	Upper Boundary
serialize_deserialize_authorize	RAM Accesses (accesses)	🚨 (view plot \| view alert)	299.00 (+1.32%)		298.60 (100.13%)
serialize_deserialize_handle_authorize	RAM Accesses (accesses)	🚨 (view plot \| view alert)	369.00 (+1.31%)		368.45 (100.15%)

Click to view all benchmark results

Benchmark	Estimated Cycles	Estimated Cycles Results estimated cycles \| (Δ%)	Estimated Cycles Upper Boundary estimated cycles \| (%)	Instructions	Instructions Results instructions \| (Δ%)	Instructions Upper Boundary instructions \| (%)	L1 Accesses	L1 Accesses Results accesses \| (Δ%)	L1 Accesses Upper Boundary accesses \| (%)	L2 Accesses	L2 Accesses Results accesses \| (Δ%)	L2 Accesses Upper Boundary accesses \| (%)	RAM Accesses	RAM Accesses Results accesses \| (Δ%)	RAM Accesses Upper Boundary accesses \| (%)
get_authorize	✅ (view plot)	8,526.00 (+1.00%)	8,724.03 (97.73%)	✅ (view plot)	3,738.00 (-0.07%)	3,851.74 (97.05%)	✅ (view plot)	5,236.00 (-0.17%)	5,396.38 (97.03%)	✅ (view plot)	7.00 (-11.18%)	10.30 (67.96%)	✅ (view plot)	93.00 (+3.10%)	94.29 (98.63%)
get_submit	✅ (view plot)	95,497.00 (-0.06%)	96,118.98 (99.35%)	✅ (view plot)	59,431.00 (-0.06%)	59,766.70 (99.44%)	✅ (view plot)	85,347.00 (-0.06%)	85,819.81 (99.45%)	✅ (view plot)	49.00 (-10.41%)	62.59 (78.29%)	✅ (view plot)	283.00 (+0.31%)	287.56 (98.41%)
get_subscribe	✅ (view plot)	7,979.00 (+0.03%)	8,260.56 (96.59%)	✅ (view plot)	2,833.00 (+0.09%)	2,936.79 (96.47%)	✅ (view plot)	3,959.00 (+0.11%)	4,095.75 (96.66%)	✅ (view plot)	13.00 (-18.58%)	19.85 (65.48%)	✅ (view plot)	113.00 (+0.33%)	116.81 (96.74%)
serialize_authorize	✅ (view plot)	12,279.00 (+0.56%)	12,499.68 (98.23%)	✅ (view plot)	5,309.00 (-0.05%)	5,422.74 (97.90%)	✅ (view plot)	7,399.00 (-0.11%)	7,559.72 (97.87%)	✅ (view plot)	10.00 (-6.94%)	13.27 (75.36%)	✅ (view plot)	138.00 (+1.70%)	140.34 (98.33%)
serialize_deserialize_authorize	✅ (view plot)	24,576.00 (+0.41%)	24,713.25 (99.44%)	✅ (view plot)	9,890.00 (-0.10%)	10,021.80 (98.68%)	✅ (view plot)	13,946.00 (-0.14%)	14,144.36 (98.60%)	✅ (view plot)	33.00 (-9.19%)	41.55 (79.42%)	🚨 (view plot \| view alert)	299.00 (+1.32%)	298.60 (100.13%)
serialize_deserialize_handle_authorize	✅ (view plot)	30,315.00 (+0.52%)	30,373.30 (99.81%)	✅ (view plot)	12,093.00 (-0.02%)	12,206.74 (99.07%)	✅ (view plot)	17,105.00 (-0.07%)	17,274.31 (99.02%)	✅ (view plot)	59.00 (+0.23%)	64.63 (91.28%)	🚨 (view plot \| view alert)	369.00 (+1.31%)	368.45 (100.15%)
serialize_deserialize_handle_submit	✅ (view plot)	126,372.00 (-0.03%)	127,009.42 (99.50%)	✅ (view plot)	73,224.00 (-0.03%)	73,606.64 (99.48%)	✅ (view plot)	104,947.00 (-0.04%)	105,492.52 (99.48%)	✅ (view plot)	120.00 (-0.39%)	130.89 (91.68%)	✅ (view plot)	595.00 (+0.02%)	599.12 (99.31%)
serialize_deserialize_handle_subscribe	✅ (view plot)	27,479.00 (+0.08%)	27,601.86 (99.55%)	✅ (view plot)	9,635.00 (+0.03%)	9,738.79 (98.93%)	✅ (view plot)	13,629.00 (+0.04%)	13,771.37 (98.97%)	✅ (view plot)	61.00 (-7.07%)	73.58 (82.90%)	✅ (view plot)	387.00 (+0.29%)	388.67 (99.57%)
serialize_deserialize_submit	✅ (view plot)	114,999.00 (-0.07%)	115,630.58 (99.45%)	✅ (view plot)	68,001.00 (-0.08%)	68,389.11 (99.43%)	✅ (view plot)	97,559.00 (-0.09%)	98,130.66 (99.42%)	✅ (view plot)	65.00 (-5.98%)	75.26 (86.37%)	✅ (view plot)	489.00 (+0.17%)	492.32 (99.33%)
serialize_deserialize_subscribe	✅ (view plot)	22,910.00 (+0.15%)	23,105.97 (99.15%)	✅ (view plot)	8,187.00 (+0.01%)	8,294.29 (98.71%)	✅ (view plot)	11,530.00 (+0.00%)	11,678.40 (98.73%)	✅ (view plot)	36.00 (-8.09%)	44.03 (81.75%)	✅ (view plot)	320.00 (+0.44%)	321.54 (99.52%)
serialize_submit	✅ (view plot)	99,814.00 (-0.07%)	100,447.03 (99.37%)	✅ (view plot)	61,475.00 (-0.06%)	61,815.90 (99.45%)	✅ (view plot)	88,194.00 (-0.06%)	88,673.12 (99.46%)	✅ (view plot)	49.00 (-11.32%)	62.53 (78.36%)	✅ (view plot)	325.00 (+0.10%)	329.05 (98.77%)
serialize_subscribe	✅ (view plot)	11,316.00 (-0.08%)	11,596.31 (97.58%)	✅ (view plot)	4,180.00 (+0.06%)	4,283.79 (97.58%)	✅ (view plot)	5,816.00 (+0.06%)	5,954.83 (97.67%)	✅ (view plot)	15.00 (-7.81%)	18.89 (79.41%)	✅ (view plot)	155.00 (-0.11%)	159.60 (97.12%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

github-actions · 2024-06-30T02:01:46Z

Bencher

Report	Wed, July 24, 2024 at 14:23:48 UTC
Project	Stratum v2 (SRI)
Branch	johnny/issue/484
Testbed	sv2

🚨 8 ALERTS: Threshold Boundary Limits exceeded!

Benchmark	Measure (units)	View	Value	Upper Boundary
client_sv2_mining_message_submit_standard	L2 Accesses (accesses)	🚨 (view plot \| view alert)	26.00 (+42.53%)	25.37 (102.50%)
client_sv2_mining_message_submit_standard_serialize	L2 Accesses (accesses)	🚨 (view plot \| view alert)	55.00 (+14.71%)	54.50 (100.93%)
client_sv2_mining_message_submit_standard_serialize_deserialize	Instructions (instructions)	🚨 (view plot \| view alert)	10,585.00 (+0.37%)	10,578.26 (100.06%)
client_sv2_mining_message_submit_standard_serialize_deserialize	L1 Accesses (accesses)	🚨 (view plot \| view alert)	15,401.00 (+0.36%)	15,390.03 (100.07%)
client_sv2_open_channel_serialize_deserialize	Instructions (instructions)	🚨 (view plot \| view alert)	8,027.00 (+0.49%)	8,020.59 (100.08%)
client_sv2_open_channel_serialize_deserialize	L1 Accesses (accesses)	🚨 (view plot \| view alert)	11,674.00 (+0.47%)	11,663.99 (100.09%)
client_sv2_setup_connection_serialize_deserialize	Instructions (instructions)	🚨 (view plot \| view alert)	14,855.00 (+0.27%)	14,848.24 (100.05%)
client_sv2_setup_connection_serialize_deserialize	L1 Accesses (accesses)	🚨 (view plot \| view alert)	21,813.00 (+0.26%)	21,801.89 (100.05%)

Click to view all benchmark results

Benchmark	Estimated Cycles	Estimated Cycles Results estimated cycles \| (Δ%)	Estimated Cycles Upper Boundary estimated cycles \| (%)	Instructions	Instructions Results instructions \| (Δ%)	Instructions Upper Boundary instructions \| (%)	L1 Accesses	L1 Accesses Results accesses \| (Δ%)	L1 Accesses Upper Boundary accesses \| (%)	L2 Accesses	L2 Accesses Results accesses \| (Δ%)	L2 Accesses Upper Boundary accesses \| (%)	RAM Accesses	RAM Accesses Results accesses \| (Δ%)	RAM Accesses Upper Boundary accesses \| (%)
client_sv2_handle_message_common	✅ (view plot)	2,043.00 (-0.31%)	2,124.24 (96.18%)	✅ (view plot)	473.00 (+0.45%)	486.03 (97.32%)	✅ (view plot)	733.00 (+0.11%)	754.43 (97.16%)	✅ (view plot)	10.00 (+38.42%)	11.36 (88.06%)	✅ (view plot)	36.00 (-1.65%)	38.47 (93.57%)
client_sv2_handle_message_mining	✅ (view plot)	8,219.00 (+0.25%)	8,333.65 (98.62%)	✅ (view plot)	2,137.00 (+0.43%)	2,170.73 (98.45%)	✅ (view plot)	3,159.00 (+0.43%)	3,214.88 (98.26%)	✅ (view plot)	39.00 (+1.07%)	43.49 (89.67%)	✅ (view plot)	139.00 (+0.10%)	141.87 (97.98%)
client_sv2_mining_message_submit_standard	✅ (view plot)	6,248.00 (-0.41%)	6,383.48 (97.88%)	✅ (view plot)	1,750.00 (+0.03%)	1,762.72 (99.28%)	✅ (view plot)	2,548.00 (-0.21%)	2,574.92 (98.95%)	🚨 (view plot \| view alert)	26.00 (+42.53%)	25.37 (102.50%)	✅ (view plot)	102.00 (-1.63%)	106.82 (95.49%)
client_sv2_mining_message_submit_standard_serialize	✅ (view plot)	14,589.00 (-1.16%)	15,030.29 (97.06%)	✅ (view plot)	4,694.00 (+0.01%)	4,706.72 (99.73%)	✅ (view plot)	6,754.00 (-0.01%)	6,774.47 (99.70%)	🚨 (view plot \| view alert)	55.00 (+14.71%)	54.50 (100.93%)	✅ (view plot)	216.00 (-2.66%)	230.02 (93.91%)
client_sv2_mining_message_submit_standard_serialize_deserialize	✅ (view plot)	27,401.00 (-0.26%)	27,827.65 (98.47%)	🚨 (view plot \| view alert)	10,585.00 (+0.37%)	10,578.26 (100.06%)	🚨 (view plot \| view alert)	15,401.00 (+0.36%)	15,390.03 (100.07%)	✅ (view plot)	90.00 (+6.81%)	90.82 (99.10%)	✅ (view plot)	330.00 (-1.33%)	344.90 (95.68%)
client_sv2_open_channel	✅ (view plot)	4,383.00 (-2.24%)	4,612.90 (95.02%)	✅ (view plot)	1,461.00 (+0.05%)	1,473.99 (99.12%)	✅ (view plot)	2,158.00 (+0.23%)	2,172.85 (99.32%)	✅ (view plot)	11.00 (-7.54%)	15.08 (72.93%)	✅ (view plot)	62.00 (-4.44%)	68.28 (90.80%)
client_sv2_open_channel_serialize	✅ (view plot)	13,972.00 (-1.58%)	14,470.71 (96.55%)	✅ (view plot)	5,064.00 (+0.01%)	5,076.99 (99.74%)	✅ (view plot)	7,322.00 (+0.05%)	7,338.94 (99.77%)	✅ (view plot)	42.00 (+11.44%)	43.10 (97.45%)	✅ (view plot)	184.00 (-3.74%)	199.33 (92.31%)
client_sv2_open_channel_serialize_deserialize	✅ (view plot)	22,489.00 (-0.62%)	23,007.73 (97.75%)	🚨 (view plot \| view alert)	8,027.00 (+0.49%)	8,020.59 (100.08%)	🚨 (view plot \| view alert)	11,674.00 (+0.47%)	11,663.99 (100.09%)	✅ (view plot)	84.00 (+13.14%)	84.69 (99.19%)	✅ (view plot)	297.00 (-2.29%)	314.87 (94.33%)
client_sv2_setup_connection	✅ (view plot)	4,625.00 (-1.44%)	4,768.15 (97.00%)	✅ (view plot)	1,502.00 (+0.05%)	1,514.99 (99.14%)	✅ (view plot)	2,275.00 (-0.07%)	2,298.75 (98.97%)	✅ (view plot)	15.00 (+56.47%)	15.13 (99.12%)	✅ (view plot)	65.00 (-3.92%)	69.98 (92.88%)
client_sv2_setup_connection_serialize	✅ (view plot)	15,908.00 (-2.01%)	16,535.64 (96.20%)	✅ (view plot)	5,963.00 (+0.01%)	5,975.99 (99.78%)	✅ (view plot)	8,663.00 (+0.08%)	8,677.86 (99.83%)	✅ (view plot)	49.00 (+8.85%)	49.99 (98.01%)	✅ (view plot)	200.00 (-4.82%)	219.07 (91.29%)
client_sv2_setup_connection_serialize_deserialize	✅ (view plot)	35,348.00 (-0.48%)	35,749.36 (98.88%)	🚨 (view plot \| view alert)	14,855.00 (+0.27%)	14,848.24 (100.05%)	🚨 (view plot \| view alert)	21,813.00 (+0.26%)	21,801.89 (100.05%)	✅ (view plot)	110.00 (+9.43%)	114.51 (96.06%)	✅ (view plot)	371.00 (-2.08%)	385.73 (96.18%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

jbesraa

Thanks for looking into this, added few small comments

roles/mining-proxy/src/lib/downstream_mining.rs

roles/mining-proxy/src/main.rs

plebhash · 2024-07-01T15:33:21Z

@johnnyasantoss can you write some instructions on how to reproduce the original issue?

I still haven't been able to reproduce it

johnnyasantoss · 2024-07-01T16:38:25Z

@johnnyasantoss can you write some instructions on how to reproduce the original issue?

I still haven't been able to reproduce it

Try cargo run -- -c config-examples/proxy-config-example.toml followed by echo $?, and ctrl-c on both dev and on this branch

Ideally it should be 0 and not 130 so that's easier to script

johnnyasantoss · 2024-07-03T13:39:20Z

Should I keep rebasing or only rebase once there are enough reviews?

plebhash · 2024-07-03T15:59:09Z

Should I keep rebasing or only rebase once there are enough reviews?

this is tricky, we discussed it a bit on the last minutes of our call yesterday

basically, every PR that gets merged before this would mean you need to rebase it, which is very boring and doesn't really scale well

we established the following convention:

if PR author needs to update the PR, or solve some conflict, they should obviously rebase it
if PR is ready to be merged but it's not urgent, we wait a few days for PR author to rebase
if PR is ready to be merged and it's urgent, some core dev with merging rights rebases it, leaves a comment for context (since now the core dev will co-author all commits in the PR), and merges it

pavlenex · 2024-07-03T16:03:27Z

Should I keep rebasing or only rebase once there are enough reviews?

this is tricky, we discussed it a bit on the last minutes of our call yesterday

basically, every PR that gets merged before this would mean you need to rebase it, which is very boring and doesn't really scale well

we established the following convention:

if PR author needs to update the PR, or solve some conflict, they should obviously rebase it

if PR is ready to be merged but it's not urgent, we wait a few days for PR author to rebase

if PR is ready to be merged and it's urgent, some core dev with merging rights rebases it, leaves a comment for context (since now the core dev will co-author all commits in the PR), and merges it

Should we add this to contributing.md ?

johnnyasantoss · 2024-07-03T16:51:52Z

Should I keep rebasing or only rebase once there are enough reviews?

this is tricky, we discussed it a bit on the last minutes of our call yesterday

basically, every PR that gets merged before this would mean you need to rebase it, which is very boring and doesn't really scale well

we established the following convention:
* if PR author needs to update the PR, or solve some conflict, they should obviously rebase it

* if PR is ready to be merged but it's not urgent, we wait a few days for PR author to rebase

* if PR is ready to be merged and it's urgent, some core dev with merging rights rebases it, leaves a comment for context (since now the core dev will co-author all commits in the PR), and merges it

I would also add about the commit strategy. Should we use merge commits + squash merge PR or rebase + merge commit PR?
Also, the PR reviewer can perform minor updates (like clicking in update branch here on GH) to the PR, in my humble opinion. That would speed up the maintainer's task of closing PRs.

jbesraa

I have been looking into the code a bit and it would be good to do a couple of things here:
It would be very nice if you could add unit test to test function start_mining_proxy, that I think can look something like this:

async fn start_mining_proxy(config: Config) {
    let group_id = Arc::new(Mutex::new(GroupId::new()));
    lib::ROUTING_LOGIC
        .set(Mutex::new(
            lib::initialize_r_logic(&config.upstreams, group_id, config.clone()).await,
        ))
        .expect("BUG: Failed to set ROUTING_LOGIC");
    info!("PROXY INITIALIZING");
    lib::initialize_upstreams(config.min_supported_version, config.max_supported_version).await;

    // Wait for downstream connection
    let socket = SocketAddr::new(
        config.listen_address.parse().unwrap(),
        config.listen_mining_port,
    );


    let (shutdown_tx, mut shutdown_rx) = oneshot::channel();
    let listener = TcpListener::bind(socket).await.unwrap();
    info!("Listening on {}", socket.to_string());

    tokio::select! {
        _ = lib::downstream_mining::listen_for_downstream_mining(listener) => {
            warn!("Downstream mining listener exited prematurely");
        },
        _ = &mut shutdown_rx => {
            info!("Closing listener on {}", socket.to_string());
            let _ = shutdown_tx.send(());
        }
        _ = tokio::signal::ctrl_c() => {
            let _ = shutdown_tx.send(());
            info!("Interrupt received");
        }
    }

    info!("Shutdown done");

}

Notice how I changed the tokio::select but I dont have strong opinion about it, but I do think we should try to avoid nested select's.

This would also require to change listen_for_downstream_mining signature to accept TcpListener rather than SocketAddr.

This way we separate the binary stuff(getting the config), and the actual service initiating function and we can test it more easily to verify your solution.

If you think this is too much, I am happy to pick it up in a later PR.

Here is a commit with a bit of changes jbesraa@8753830

plebhash · 2024-07-04T18:07:36Z

Should I keep rebasing or only rebase once there are enough reviews?

this is tricky, we discussed it a bit on the last minutes of our call yesterday
basically, every PR that gets merged before this would mean you need to rebase it, which is very boring and doesn't really scale well
we established the following convention:
* if PR author needs to update the PR, or solve some conflict, they should obviously rebase it

* if PR is ready to be merged but it's not urgent, we wait a few days for PR author to rebase

* if PR is ready to be merged and it's urgent, some core dev with merging rights rebases it, leaves a comment for context (since now the core dev will co-author all commits in the PR), and merges it
I would also add about the commit strategy. Should we use merge commits + squash merge PR or rebase + merge commit PR? Also, the PR reviewer can perform minor updates (like clicking in update branch here on GH) to the PR, in my humble opinion. That would speed up the maintainer's task of closing PRs.

@marathon-gary started working on #1030 so we should move this discussion to that PR

jbesraa

cool! this looks much better. Ill create a followup pr to cover another couple of things in mining-proxy (like remove the dead_code, adding tests..)

could you please tide up the commit message?

johnnyasantoss · 2024-07-09T11:56:47Z

cool! this looks much better. Ill create a followup pr to cover another couple of things in mining-proxy (like remove the dead_code, adding tests..)

could you please tide up the commit message?

@jbesraa tide up? Is there a pattern that we must follow? Something like conventional commits? Asking because I didn't see any docs about this and this is my first PR here.

I still need to reformat and add the tests you mentioned. I was fighting tooling yesterday. Apparently, Rust has gotten a lot of tooling nowadays, so I need to configure properly my dev env.

jbesraa · 2024-07-09T12:28:31Z

cool! this looks much better. Ill create a followup pr to cover another couple of things in mining-proxy (like remove the dead_code, adding tests..)
could you please tide up the commit message?

@jbesraa tide up? Is there a pattern that we must follow? Something like conventional commits? Asking because I didn't see any docs about this and this is my first PR here.

I still need to reformat and add the tests you mentioned. I was fighting tooling yesterday. Apparently, Rust has gotten a lot of tooling nowadays, so I need to configure properly my dev env.

Commit guidelines: https://cbea.ms/git-commit/

sounds good, feel free to bing me once its ready for review

jbesraa · 2024-07-09T13:24:41Z

@johnnyasantoss could you please cherry-pick this commit jbesraa@f91be81 here?

johnnyasantoss · 2024-07-09T17:08:31Z

@jbesraa the permission to write is enabled (you mentioned on the dev call)

See

jbesraa

I think we can merge this. Ill do a bit of testing with mining-proxy as part of #1066

roles/mining-proxy/src/lib/downstream_mining.rs

jbesraa · 2024-07-19T09:16:11Z

@johnnyasantoss This needs a rebase

jbesraa

@johnnyasantoss Can you please organize your commits so they are grouped correctly? for example the spawn thing shouldn't be in its own commit, same for the typos.
The first commits has refactor + small fixes + the actual solution, so I would organize that as well. In this PR you can end-up with the following commits:

1 - all the typos/format/ordering imports(you can also break this into multiple commits if one of the changes is too big)
2 - code refactor(same, you can decide to break this)
3 - actual code for graceful shutdown

Could you also remove 2e48ac1 as we currently don't commit lock files

Please refer to this https://cbea.ms/git-commit/ for a better understanding to what we are trying to achieve in the commit history.

Changes: - Add listener on exit signals - Add channel to unbind listener as well - Organize main fn to handle signals and ownership of sockets - Fix typos in docs - Format imports - Remove unused spawn - Simplifies the code a bit reducing nesting and matches with unused arms

(cherry picked from commit f91be81) Co-authored-by: jbesraa <[email protected]>

jbesraa reviewed Jul 1, 2024

View reviewed changes

johnnyasantoss force-pushed the johnny/issue/484 branch from 80f0106 to 329bfc3 Compare July 1, 2024 14:37

plebhash approved these changes Jul 1, 2024

View reviewed changes

GitGab19 linked an issue Jul 3, 2024 that may be closed by this pull request

mining-proxy needs graceful death instead of panic #484

Closed

johnnyasantoss requested a review from jbesraa July 3, 2024 13:40

jbesraa reviewed Jul 4, 2024

View reviewed changes

plebhash mentioned this pull request Jul 4, 2024

Update CONTRIBUTING.md #1030

Closed

johnnyasantoss force-pushed the johnny/issue/484 branch from 8aa6914 to a4b4640 Compare July 8, 2024 02:51

jbesraa reviewed Jul 9, 2024

View reviewed changes

pavlenex added this to the 1.0.2 milestone Jul 9, 2024

pavlenex assigned johnnyasantoss and jbesraa Jul 9, 2024

johnnyasantoss force-pushed the johnny/issue/484 branch from a717051 to 5816894 Compare July 9, 2024 17:47

jbesraa approved these changes Jul 19, 2024

View reviewed changes

roles/mining-proxy/src/lib/downstream_mining.rs Outdated Show resolved Hide resolved

johnnyasantoss force-pushed the johnny/issue/484 branch 2 times, most recently from 165d0dd to 690b109 Compare July 23, 2024 01:01

jbesraa reviewed Jul 23, 2024

View reviewed changes

johnnyasantoss and others added 2 commits July 24, 2024 11:13

Remove dead code from downstream_mining.rs

5b5d28e

(cherry picked from commit f91be81) Co-authored-by: jbesraa <[email protected]>

johnnyasantoss force-pushed the johnny/issue/484 branch from 690b109 to 5b5d28e Compare July 24, 2024 14:16

johnnyasantoss requested a review from jbesraa July 24, 2024 14:17

jbesraa approved these changes Jul 25, 2024

View reviewed changes

pavlenex merged commit f3c1e3d into stratum-mining:dev Jul 25, 2024
31 checks passed

johnnyasantoss deleted the johnny/issue/484 branch July 25, 2024 14:17

This was referenced Aug 18, 2024

Release 1.0.2 #1112

Merged

Framing refactor: framing.rs api #1033

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Graceful shutdown for mining-proxy #1021

Graceful shutdown for mining-proxy #1021

johnnyasantoss commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024 •

edited

Loading

jbesraa left a comment

plebhash commented Jul 1, 2024

johnnyasantoss commented Jul 1, 2024

johnnyasantoss commented Jul 3, 2024 •

edited

Loading

plebhash commented Jul 3, 2024

pavlenex commented Jul 3, 2024

johnnyasantoss commented Jul 3, 2024

jbesraa left a comment •

edited

Loading

plebhash commented Jul 4, 2024

jbesraa left a comment

johnnyasantoss commented Jul 9, 2024

jbesraa commented Jul 9, 2024

jbesraa commented Jul 9, 2024

johnnyasantoss commented Jul 9, 2024

jbesraa left a comment

jbesraa commented Jul 19, 2024

jbesraa left a comment •

edited

Loading

Graceful shutdown for mining-proxy #1021

Graceful shutdown for mining-proxy #1021

Conversation

johnnyasantoss commented Jun 30, 2024 • edited Loading

github-actions bot commented Jun 30, 2024 • edited Loading

Bencher

github-actions bot commented Jun 30, 2024 • edited Loading

Bencher

github-actions bot commented Jun 30, 2024 • edited Loading

Bencher

github-actions bot commented Jun 30, 2024 • edited Loading

Bencher

jbesraa left a comment

Choose a reason for hiding this comment

plebhash commented Jul 1, 2024

johnnyasantoss commented Jul 1, 2024

johnnyasantoss commented Jul 3, 2024 • edited Loading

plebhash commented Jul 3, 2024

pavlenex commented Jul 3, 2024

johnnyasantoss commented Jul 3, 2024

jbesraa left a comment • edited Loading

Choose a reason for hiding this comment

plebhash commented Jul 4, 2024

jbesraa left a comment

Choose a reason for hiding this comment

johnnyasantoss commented Jul 9, 2024

jbesraa commented Jul 9, 2024

jbesraa commented Jul 9, 2024

johnnyasantoss commented Jul 9, 2024

jbesraa left a comment

Choose a reason for hiding this comment

jbesraa commented Jul 19, 2024

jbesraa left a comment • edited Loading

Choose a reason for hiding this comment

johnnyasantoss commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024 •

edited

Loading

github-actions bot commented Jun 30, 2024 •

edited

Loading

johnnyasantoss commented Jul 3, 2024 •

edited

Loading

jbesraa left a comment •

edited

Loading

jbesraa left a comment •

edited

Loading