Move `JDServer` lib code out of `main.rs` #1095

jbesraa · 2024-08-13T14:15:32Z

part of #1093

github-actions · 2024-08-13T14:26:50Z

Bencher

Report	Fri, August 30, 2024 at 18:07:38 UTC
Project	Stratum v2 (SRI)
Branch	2024-08-13-refactor-jdserver
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.51 (-0.26%)	45.56 (97.69%)
client_sv2_handle_message_mining	✅ (view plot)	74.93 (+1.35%)	83.01 (90.27%)
client_sv2_mining_message_submit_standard	✅ (view plot)	14.71 (+0.33%)	14.71 (99.97%)
client_sv2_mining_message_submit_standard_serialize	✅ (view plot)	270.14 (+1.35%)	284.66 (94.90%)
client_sv2_mining_message_submit_standard_serialize_deserialize	✅ (view plot)	590.94 (-0.46%)	627.42 (94.19%)
client_sv2_open_channel	✅ (view plot)	166.21 (+0.28%)	171.66 (96.83%)
client_sv2_open_channel_serialize	✅ (view plot)	276.89 (-1.64%)	294.20 (94.12%)
client_sv2_open_channel_serialize_deserialize	✅ (view plot)	372.30 (-1.49%)	427.38 (87.11%)
client_sv2_setup_connection	✅ (view plot)	161.40 (-1.28%)	174.98 (92.24%)
client_sv2_setup_connection_serialize	✅ (view plot)	491.51 (+4.30%)	511.35 (96.12%)
client_sv2_setup_connection_serialize_deserialize	✅ (view plot)	982.00 (+0.03%)	1,071.75 (91.63%)

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

github-actions · 2024-08-13T14:27:03Z

Bencher

Report	Fri, August 30, 2024 at 18:07:39 UTC
Project	Stratum v2 (SRI)
Branch	1095/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,815.80 (-0.29%)	7,293.09 (93.46%)
client-submit-serialize-deserialize	✅ (view plot)	7,783.70 (+0.07%)	8,273.31 (94.08%)
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle	✅ (view plot)	8,433.90 (+0.90%)	8,780.68 (96.05%)
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle	✅ (view plot)	901.81 (-0.32%)	941.53 (95.78%)
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize	✅ (view plot)	715.30 (+1.62%)	733.09 (97.57%)
client-sv1-authorize-serialize/client-sv1-authorize-serialize	✅ (view plot)	245.33 (-1.55%)	256.08 (95.80%)
client-sv1-get-authorize/client-sv1-get-authorize	✅ (view plot)	156.87 (-0.24%)	162.33 (96.64%)
client-sv1-get-submit	✅ (view plot)	6,605.80 (-0.11%)	7,053.60 (93.65%)
client-sv1-get-subscribe/client-sv1-get-subscribe	✅ (view plot)	274.36 (-0.98%)	292.41 (93.83%)
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle	✅ (view plot)	775.50 (+2.74%)	790.90 (98.05%)
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize	✅ (view plot)	627.32 (+1.66%)	642.86 (97.58%)
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize	✅ (view plot)	209.17 (+0.85%)	218.48 (95.74%)

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

github-actions · 2024-08-13T14:27:05Z

Bencher

Report	Fri, August 30, 2024 at 18:07:38 UTC
Project	Stratum v2 (SRI)
Branch	2024-08-13-refactor-jdserver
Testbed	sv1

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,402.00 (-0.39%)	8,690.75 (96.68%)	✅ (view plot)	3,746.00 (+0.02%)	3,846.70 (97.38%)	✅ (view plot)	5,252.00 (-0.02%)	5,391.38 (97.41%)	✅ (view plot)	7.00 (-5.92%)	10.22 (68.53%)	✅ (view plot)	89.00 (-0.96%)	93.47 (95.22%)
get_submit	✅ (view plot)	95,345.00 (-0.18%)	96,049.05 (99.27%)	✅ (view plot)	59,439.00 (-0.05%)	59,717.15 (99.53%)	✅ (view plot)	85,370.00 (-0.05%)	85,757.44 (99.55%)	✅ (view plot)	42.00 (-16.70%)	64.83 (64.79%)	✅ (view plot)	279.00 (-0.89%)	287.74 (96.96%)
get_subscribe	✅ (view plot)	8,025.00 (+0.35%)	8,262.15 (97.13%)	✅ (view plot)	2,841.00 (+0.25%)	2,931.38 (96.92%)	✅ (view plot)	3,970.00 (+0.23%)	4,090.87 (97.05%)	✅ (view plot)	13.00 (-7.57%)	20.46 (63.53%)	✅ (view plot)	114.00 (+0.61%)	117.56 (96.98%)
serialize_authorize	✅ (view plot)	12,249.00 (+0.10%)	12,519.03 (97.84%)	✅ (view plot)	5,317.00 (+0.01%)	5,417.70 (98.14%)	✅ (view plot)	7,414.00 (-0.01%)	7,553.68 (98.15%)	✅ (view plot)	8.00 (-17.88%)	13.36 (59.90%)	✅ (view plot)	137.00 (+0.45%)	141.20 (97.03%)
serialize_deserialize_authorize	✅ (view plot)	24,687.00 (+0.58%)	24,836.37 (99.40%)	✅ (view plot)	9,868.00 (-0.31%)	10,016.08 (98.52%)	✅ (view plot)	13,927.00 (-0.30%)	14,139.88 (98.49%)	✅ (view plot)	38.00 (+5.78%)	41.79 (90.94%)	✅ (view plot)	302.00 (+1.67%)	304.02 (99.33%)
serialize_deserialize_handle_authorize	✅ (view plot)	30,354.00 (+0.48%)	30,485.18 (99.57%)	✅ (view plot)	12,071.00 (-0.18%)	12,197.25 (98.96%)	✅ (view plot)	17,089.00 (-0.17%)	17,263.31 (98.99%)	✅ (view plot)	63.00 (+8.92%)	64.04 (98.37%)	✅ (view plot)	370.00 (+1.17%)	372.82 (99.24%)
serialize_deserialize_handle_submit	✅ (view plot)	126,518.00 (+0.06%)	126,994.05 (99.63%)	✅ (view plot)	73,280.00 (+0.02%)	73,561.84 (99.62%)	✅ (view plot)	105,053.00 (+0.03%)	105,444.54 (99.63%)	✅ (view plot)	107.00 (-5.78%)	133.66 (80.05%)	✅ (view plot)	598.00 (+0.34%)	601.88 (99.36%)
serialize_deserialize_handle_subscribe	✅ (view plot)	28,002.00 (+1.46%)	28,098.27 (99.66%)	✅ (view plot)	9,659.00 (+0.20%)	9,739.84 (99.17%)	✅ (view plot)	13,657.00 (+0.16%)	13,772.51 (99.16%)	✅ (view plot)	69.00 (+6.15%)	72.42 (95.28%)	✅ (view plot)	400.00 (+2.65%)	402.56 (99.36%)
serialize_deserialize_submit	✅ (view plot)	115,181.00 (+0.03%)	115,648.81 (99.60%)	✅ (view plot)	68,057.00 (-0.02%)	68,353.84 (99.57%)	✅ (view plot)	97,656.00 (-0.03%)	98,093.71 (99.55%)	✅ (view plot)	61.00 (-7.00%)	76.99 (79.23%)	✅ (view plot)	492.00 (+0.51%)	495.43 (99.31%)
serialize_deserialize_subscribe	✅ (view plot)	23,379.00 (+1.55%)	23,513.90 (99.43%)	✅ (view plot)	8,211.00 (+0.21%)	8,294.94 (98.99%)	✅ (view plot)	11,564.00 (+0.19%)	11,680.02 (99.01%)	✅ (view plot)	39.00 (+1.60%)	43.30 (90.06%)	✅ (view plot)	332.00 (+2.94%)	334.80 (99.16%)
serialize_submit	✅ (view plot)	99,814.00 (-0.08%)	100,403.55 (99.41%)	✅ (view plot)	61,483.00 (-0.05%)	61,765.00 (99.54%)	✅ (view plot)	88,209.00 (-0.05%)	88,608.39 (99.55%)	✅ (view plot)	46.00 (-10.50%)	64.13 (71.73%)	✅ (view plot)	325.00 (-0.10%)	330.99 (98.19%)
serialize_subscribe	✅ (view plot)	11,426.00 (+0.46%)	11,656.15 (98.03%)	✅ (view plot)	4,188.00 (+0.17%)	4,278.38 (97.89%)	✅ (view plot)	5,826.00 (+0.14%)	5,948.02 (97.95%)	✅ (view plot)	14.00 (-4.75%)	19.68 (71.14%)	✅ (view plot)	158.00 (+0.87%)	162.11 (97.47%)

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

github-actions · 2024-08-13T14:27:09Z

Bencher

Report	Fri, August 30, 2024 at 18:07:39 UTC
Project	Stratum v2 (SRI)
Branch	2024-08-13-refactor-jdserver
Testbed	sv2

🚨 3 ALERTS: Threshold Boundary Limits exceeded!

Benchmark	Measure (units)	View	Value	Upper Boundary
client_sv2_handle_message_common	Estimated Cycles (estimated cycles)	🚨 (view plot \| view alert)	2,167.00 (+4.88%)	2,151.10 (100.74%)
client_sv2_handle_message_common	RAM Accesses (accesses)	🚨 (view plot \| view alert)	40.00 (+7.59%)	39.53 (101.19%)
client_sv2_mining_message_submit_standard	L2 Accesses (accesses)	🚨 (view plot \| view alert)	23.00 (+32.66%)	22.83 (100.75%)

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 \| view alert)	2,167.00 (+4.88%)	2,151.10 (100.74%)	✅ (view plot)	473.00 (+0.38%)	484.12 (97.70%)	✅ (view plot)	732.00 (-0.11%)	752.22 (97.31%)	✅ (view plot)	7.00 (+9.49%)	12.26 (57.09%)	🚨 (view plot \| view alert)	40.00 (+7.59%)	39.53 (101.19%)
client_sv2_handle_message_mining	✅ (view plot)	8,200.00 (-0.05%)	8,317.53 (98.59%)	✅ (view plot)	2,137.00 (+0.24%)	2,168.17 (98.56%)	✅ (view plot)	3,160.00 (+0.26%)	3,210.33 (98.43%)	✅ (view plot)	35.00 (-5.55%)	43.05 (81.29%)	✅ (view plot)	139.00 (-0.04%)	141.60 (98.16%)
client_sv2_mining_message_submit_standard	✅ (view plot)	6,369.00 (+1.40%)	6,382.92 (99.78%)	✅ (view plot)	1,750.00 (-0.01%)	1,762.45 (99.29%)	✅ (view plot)	2,544.00 (-0.37%)	2,573.00 (98.87%)	🚨 (view plot \| view alert)	23.00 (+32.66%)	22.83 (100.75%)	✅ (view plot)	106.00 (+1.89%)	106.80 (99.25%)
client_sv2_mining_message_submit_standard_serialize	✅ (view plot)	14,822.00 (+0.34%)	14,986.58 (98.90%)	✅ (view plot)	4,694.00 (-0.00%)	4,706.45 (99.74%)	✅ (view plot)	6,752.00 (-0.06%)	6,774.17 (99.67%)	✅ (view plot)	46.00 (+1.21%)	52.43 (87.74%)	✅ (view plot)	224.00 (+0.67%)	228.63 (97.97%)
client_sv2_mining_message_submit_standard_serialize_deserialize	✅ (view plot)	27,608.00 (+0.37%)	27,804.39 (99.29%)	✅ (view plot)	10,585.00 (+0.25%)	10,606.23 (99.80%)	✅ (view plot)	15,398.00 (+0.22%)	15,432.65 (99.78%)	✅ (view plot)	83.00 (+0.92%)	88.38 (93.91%)	✅ (view plot)	337.00 (+0.55%)	343.51 (98.11%)
client_sv2_open_channel	✅ (view plot)	4,447.00 (+0.02%)	4,612.16 (96.42%)	✅ (view plot)	1,461.00 (+0.06%)	1,471.93 (99.26%)	✅ (view plot)	2,157.00 (+0.10%)	2,172.83 (99.27%)	✅ (view plot)	10.00 (-8.53%)	15.90 (62.89%)	✅ (view plot)	64.00 (+0.16%)	68.38 (93.59%)
client_sv2_open_channel_serialize	✅ (view plot)	14,114.00 (-0.31%)	14,419.38 (97.88%)	✅ (view plot)	5,064.00 (+0.02%)	5,074.93 (99.78%)	✅ (view plot)	7,324.00 (+0.05%)	7,340.37 (99.78%)	✅ (view plot)	35.00 (-1.86%)	41.29 (84.77%)	✅ (view plot)	189.00 (-0.66%)	197.57 (95.66%)
client_sv2_open_channel_serialize_deserialize	✅ (view plot)	22,741.00 (+0.42%)	22,949.32 (99.09%)	✅ (view plot)	8,027.00 (+0.34%)	8,045.50 (99.77%)	✅ (view plot)	11,671.00 (+0.29%)	11,704.32 (99.72%)	✅ (view plot)	79.00 (+7.21%)	82.11 (96.21%)	✅ (view plot)	305.00 (+0.33%)	312.49 (97.60%)
client_sv2_setup_connection	✅ (view plot)	4,749.00 (+1.25%)	4,757.66 (99.82%)	✅ (view plot)	1,502.00 (+0.06%)	1,512.93 (99.28%)	✅ (view plot)	2,274.00 (-0.11%)	2,295.39 (99.07%)	✅ (view plot)	12.00 (+27.14%)	14.32 (83.80%)	✅ (view plot)	69.00 (+2.04%)	69.53 (99.24%)
client_sv2_setup_connection_serialize	✅ (view plot)	16,212.00 (-0.08%)	16,444.40 (98.59%)	✅ (view plot)	5,963.00 (+0.02%)	5,973.93 (99.82%)	✅ (view plot)	8,662.00 (+0.05%)	8,678.59 (99.81%)	✅ (view plot)	40.00 (-6.54%)	51.56 (77.58%)	✅ (view plot)	210.00 (-0.05%)	216.02 (97.21%)
client_sv2_setup_connection_serialize_deserialize	✅ (view plot)	35,654.00 (+0.31%)	35,716.76 (99.82%)	✅ (view plot)	14,855.00 (+0.19%)	14,873.84 (99.87%)	✅ (view plot)	21,819.00 (+0.19%)	21,855.51 (99.83%)	✅ (view plot)	93.00 (-2.86%)	111.35 (83.52%)	✅ (view plot)	382.00 (+0.63%)	383.66 (99.57%)

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

average-gary

Did you intend to omit the Cargo.toml change to specify a [[bin]] value?

average-gary

LGTM.

Given the interaction with bitcoin core, would it be helpful to use this crate: https://github.com/rust-bitcoin/rust-bitcoincore-rpc
Or perhaps this would need to be updated for compatibility with @Sjors branch for the TP changes?

average-gary · 2024-08-14T14:06:21Z

roles/jd-server/src/lib/core.rs

+        let mut last_empty_mempool_warning =
+            std::time::Instant::now().sub(std::time::Duration::from_secs(60));
+
+        // TODO if the jd-server is launched with core_rpc_url empty, the following flow is never


We should migrate this to a GH issue.

average-gary · 2024-08-14T14:10:17Z

roles/jd-server/src/lib/core.rs

+                            Ok(_) => (),
+                            Err(err) => {
+                                // TODO
+                                // here there should be a better error management


Maybe not it's own issue but perhaps an overarching issue for tracking better error handling across the code base? Seems like this could be part of the refactoring documentation/story.
cc: @plebhash @pavlenex @rrybarczyk

This will need to happen as part of a different PR really. Its good that we have more visibility now about the technical debt in the code though

average-gary · 2024-08-14T14:12:52Z

roles/jd-server/src/lib/core.rs

+                                }
+                            }
+                            _ => {
+                                // TODO here there should be a better error managmenet


Strong indications that there is a need for a JDS specific error handling review. 😅

average-gary · 2024-08-14T14:16:12Z

roles/jd-server/src/main.rs

@@ -108,163 +103,5 @@ async fn main() {
        }
    };



I think it would be helpful to keep this sort of daemon awareness logging.

Suggested change

info!("Jds INITIALIZING with config: {:?}", &args.config_path);

average-gary · 2024-08-14T14:21:08Z

roles/jd-server/src/main.rs

-            }
-        }
-    }
+    lib::core::JDServer::new(config).start().await;


Need associated import.

Suggested change

lib::core::JDServer::new(config).start().await;

JDServer::new(config).start().await;

Sjors · 2024-08-15T07:24:44Z

@marathon-gary wrote:

Give the interaction with bitcoin core, would it be helpful to use this crate: https://github.com/rust-bitcoin/rust-bitcoincore-rpc
Or perhaps this would need to be updated for compatibility with @Sjors branch for the TP changes?

My branch does not change RPC methods, so that crate should not be impacted.

Shourya742

ACK. I believe some of the error cases are no longer being triggered. If possible, could you remove those and resolve the other warnings as well.

Warning logs JDS


warning: field `tx` is never read
  --> jd-server/src/lib/mempool/mod.rs:15:9
   |
13 | pub struct TransactionWithHash {
   |            ------------------- field in this struct
14 |     pub id: Txid,
15 |     pub tx: Option,
   |         ^^
   |
   = note: `TransactionWithHash` has derived impls for the traits `Debug` and `Clone`, but these are intentionally ignored during dead code analysis
   = note: `#[warn(dead_code)]` on by default
warning: field 0 is never read

--> jd-server/src/lib/mempool/error.rs:9:9

|

9 |     Rpc(RpcError),

|     --- ^^^^^^^^

|     |

|     field in this variant

|

help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field

|

9 |     Rpc(()),

|         ~~
warning: field 0 is never read

--> jd-server/src/lib/mempool/error.rs:10:16

|

10 |     PoisonLock(String),

|     ---------- ^^^^^^

|     |

|     field in this variant

|

help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field

|

10 |     PoisonLock(()),

|                ~~

Shourya742 · 2024-08-18T12:36:54Z

When shutting down the entire system for this branch, I followed this order: Pool, JDS, JDC, and Translator. However, the shutdown signaling isn't working for JDC and Translator. I'm not sure if this is the right place to mention it, but could someone check?

average-gary · 2024-08-19T12:26:37Z

When shutting down the entire system for this branch, I followed this order: Pool, JDS, JDC, and Translator. However, the shutdown signaling isn't working for JDC and Translator. I'm not sure if this is the right place to mention it, but could someone check?

Log screenshot

@Shourya742
Your screenshot is not showing for me?

Shourya742 · 2024-08-19T12:45:01Z

@Shourya742
Your screenshot is not showing for me?

I hope its visible now. Thanks for pointing it out.

jbesraa · 2024-08-19T16:59:18Z

When shutting down the entire system for this branch, I followed this order: Pool, JDS, JDC, and Translator. However, the shutdown signaling isn't working for JDC and Translator. I'm not sure if this is the right place to mention it, but could someone check?

Could you please explain how did you conduct the tests so I can try to replicate it?

jbesraa · 2024-08-19T17:00:55Z

ACK. I believe some of the error cases are no longer being triggered. If possible, could you remove those and resolve the other warnings as well.
Warning logs JDS
warning: field `tx` is never read --> jd-server/src/lib/mempool/mod.rs:15:9 | 13 | pub struct TransactionWithHash { | ------------------- field in this struct 14 | pub id: Txid, 15 | pub tx: Option, | ^^ | = note: `TransactionWithHash` has derived impls for the traits `Debug` and `Clone`, but these are intentionally ignored during dead code analysis = note: `#[warn(dead_code)]` on by default warning: field `0` is never read --> jd-server/src/lib/mempool/error.rs:9:9 | 9 | Rpc(RpcError), | --- ^^^^^^^^ | | | field in this variant | help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field | 9 | Rpc(()), | ~~ warning: field `0` is never read --> jd-server/src/lib/mempool/error.rs:10:16 | 10 | PoisonLock(String), | ---------- ^^^^^^ | | | field in this variant | help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field | 10 | PoisonLock(()), | ~~

Not sure I wanna do this as part of this PR tbh

Shourya742 · 2024-08-19T17:06:56Z

When shutting down the entire system for this branch, I followed this order: Pool, JDS, JDC, and Translator. However, the shutdown signaling isn't working for JDC and Translator. I'm not sure if this is the right place to mention it, but could someone check?

Could you please explain how did you conduct the tests so I can try to replicate it?

@jbesraa Run all the roles in the following order: Pool, JDS, JDC, and Translator. Once they are in sync, terminate them using Ctrl+C. You will notice that the JDC and Translator do not respond to the shutdown signal.

rrybarczyk · 2024-08-19T17:36:41Z

@jbesraa this is very much needed, but I think we should wait on this kind of roles refactoring until we have a more concrete plan and complete #845. Otherwise we will have a disorganized approach to the refactoring effort, which is causing confusion.

@plebhash thoughts?

plebhash · 2024-08-19T20:12:57Z

@jbesraa this is very much needed, but I think we should wait on this kind of roles refactoring until we have a more concrete plan and complete #845. Otherwise we will have a disorganized approach to the refactoring effort, which is causing confusion.

@plebhash thoughts?

I think the changes here are reasonable as a requirement for the new Integration Tests framework. The same applies to all PRs tracked under #1093

If you closely look at the diff, you'll see it's mostly shuffling things around without deep architectural changes. It simply declares a new struct JDServer and moves already existing code under some methods inside it.

but I do understand where @rrybarczyk is coming from, and overall I agree we should be very conservative and limit the scope of changes to the roles to only what is a strict requirement to the Integration Tests framework

I wrote a comment on a similar spirit on another, where (different to here) I believe we are getting a bit out of scope: #1115 (comment)

plebhash · 2024-08-19T21:40:00Z

When shutting down the entire system for this branch, I followed this order: Pool, JDS, JDC, and Translator. However, the shutdown signaling isn't working for JDC and Translator. I'm not sure if this is the right place to mention it, but could someone check?

Could you please explain how did you conduct the tests so I can try to replicate it?

@jbesraa Run all the roles in the following order: Pool, JDS, JDC, and Translator. Once they are in sync, terminate them using Ctrl+C. You will notice that the JDC and Translator do not respond to the shutdown signal.

I've seen this behavior before... I usually work around it by doing a kill -9 <pid>

indeed it's quite annoying, but this is just technical debt that unfortunately fall out of scope for this PR

plebhash · 2024-08-19T22:03:01Z

Given the interaction with bitcoin core, would it be helpful to use this crate: https://github.com/rust-bitcoin/rust-bitcoincore-rpc Or perhaps this would need to be updated for compatibility with @Sjors branch for the TP changes?

@marathon-gary I agree this is a desirable change, but we should keep in mind that this PR is simply introducing some necessary restructuring as a prerequisite for #1120

overall, there is technical debt all over the place (e.g.: todos, suboptimal error handling, suboptimal architecture), but we should resist the urge of trying to fix all of them now just because reviewing this PR made us aware of them

roles/jd-server/src/lib/core.rs

roles/jd-server/src/lib/mod.rs

Move code outside of the `main.rs` to `lib/mod.rs` file to make `JDServer` accessible through other enviornements.

jbesraa force-pushed the 2024-08-13-refactor-jdserver branch from 9de3000 to 5f5c319 Compare August 13, 2024 14:15

jbesraa mentioned this pull request Aug 13, 2024

Restructure Roles as a prepration for integration tests #1093

Open

6 tasks

jbesraa marked this pull request as ready for review August 13, 2024 14:24

jbesraa changed the title ~~Refactor JDServer~~ Move JDServer lib code out of main.rs Aug 13, 2024

average-gary reviewed Aug 14, 2024

View reviewed changes

average-gary mentioned this pull request Aug 14, 2024

MovePoolSv2 lib code out of main.rs #1097

Merged

average-gary reviewed Aug 14, 2024

View reviewed changes

Shourya742 suggested changes Aug 18, 2024

View reviewed changes

jbesraa mentioned this pull request Aug 19, 2024

Add stratum-cli project #1115

Closed

jbesraa force-pushed the 2024-08-13-refactor-jdserver branch from 5f5c319 to 5b0615a Compare August 19, 2024 16:54

average-gary mentioned this pull request Aug 20, 2024

Migrate Sv2 Roles to use rust-bitcoincore-rpc #1123

Closed

plebhash linked an issue Aug 20, 2024 that may be closed by this pull request

Restructure Roles as a prepration for integration tests #1093

Open

6 tasks

plebhash reviewed Aug 20, 2024

View reviewed changes

roles/jd-server/src/lib/core.rs Outdated Show resolved Hide resolved

jbesraa force-pushed the 2024-08-13-refactor-jdserver branch from 5b0615a to 5c5b0e1 Compare August 21, 2024 10:34

jbesraa force-pushed the 2024-08-13-refactor-jdserver branch 3 times, most recently from bb58ced to 9199cd5 Compare August 21, 2024 12:23

plebhash reviewed Aug 21, 2024

View reviewed changes

roles/jd-server/src/lib/mod.rs Outdated Show resolved Hide resolved

jbesraa force-pushed the 2024-08-13-refactor-jdserver branch from 9199cd5 to cb979df Compare August 23, 2024 10:19

plebhash mentioned this pull request Aug 23, 2024

Move TranslatorSv2 lib code out of main.rs #1092

Merged

Separate binary and library code.

006b838

Move code outside of the `main.rs` to `lib/mod.rs` file to make `JDServer` accessible through other enviornements.

jbesraa force-pushed the 2024-08-13-refactor-jdserver branch from cb979df to 006b838 Compare August 26, 2024 08:10

plebhash approved these changes Aug 26, 2024

View reviewed changes

jbesraa mentioned this pull request Aug 27, 2024

Add stop function for roles #1148

Closed

plebhash changed the base branch from dev to main August 28, 2024 19:19

Merge branch 'main' into 2024-08-13-refactor-jdserver

ef254e1

plebhash merged commit 5347351 into stratum-mining:main Aug 30, 2024
34 checks passed

jbesraa mentioned this pull request Oct 15, 2024

CI migration: interop-jdc-change-upstream MG to Integration Test #1207

Open

jbesraa deleted the 2024-08-13-refactor-jdserver branch February 12, 2025 08:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move `JDServer` lib code out of `main.rs` #1095

Move `JDServer` lib code out of `main.rs` #1095

jbesraa commented Aug 13, 2024

github-actions bot commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

average-gary left a comment

average-gary left a comment •

edited

Loading

average-gary Aug 14, 2024

average-gary Aug 14, 2024

jbesraa Aug 19, 2024

average-gary Aug 14, 2024

average-gary Aug 14, 2024

average-gary Aug 14, 2024

Sjors commented Aug 15, 2024

Shourya742 left a comment

Shourya742 commented Aug 18, 2024 •

edited

Loading

average-gary commented Aug 19, 2024

Shourya742 commented Aug 19, 2024

jbesraa commented Aug 19, 2024

jbesraa commented Aug 19, 2024

Shourya742 commented Aug 19, 2024

rrybarczyk commented Aug 19, 2024

plebhash commented Aug 19, 2024 •

edited

Loading

plebhash commented Aug 19, 2024 •

edited

Loading

plebhash commented Aug 19, 2024


	info!("Jds INITIALIZING with config: {:?}", &args.config_path);

	lib::core::JDServer::new(config).start().await;
	JDServer::new(config).start().await;

Move JDServer lib code out of main.rs #1095

Move JDServer lib code out of main.rs #1095

Conversation

jbesraa commented Aug 13, 2024

github-actions bot commented Aug 13, 2024 • edited Loading

Bencher

github-actions bot commented Aug 13, 2024 • edited Loading

Bencher

github-actions bot commented Aug 13, 2024 • edited Loading

Bencher

github-actions bot commented Aug 13, 2024 • edited Loading

Bencher

average-gary left a comment

Choose a reason for hiding this comment

average-gary left a comment • edited Loading

Choose a reason for hiding this comment

average-gary Aug 14, 2024

Choose a reason for hiding this comment

average-gary Aug 14, 2024

Choose a reason for hiding this comment

jbesraa Aug 19, 2024

Choose a reason for hiding this comment

average-gary Aug 14, 2024

Choose a reason for hiding this comment

average-gary Aug 14, 2024

Choose a reason for hiding this comment

average-gary Aug 14, 2024

Choose a reason for hiding this comment

Sjors commented Aug 15, 2024

Shourya742 left a comment

Choose a reason for hiding this comment

Shourya742 commented Aug 18, 2024 • edited Loading

average-gary commented Aug 19, 2024

Shourya742 commented Aug 19, 2024

jbesraa commented Aug 19, 2024

jbesraa commented Aug 19, 2024

Shourya742 commented Aug 19, 2024

rrybarczyk commented Aug 19, 2024

plebhash commented Aug 19, 2024 • edited Loading

plebhash commented Aug 19, 2024 • edited Loading

plebhash commented Aug 19, 2024

Move `JDServer` lib code out of `main.rs` #1095

Move `JDServer` lib code out of `main.rs` #1095

github-actions bot commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

average-gary left a comment •

edited

Loading

Shourya742 commented Aug 18, 2024 •

edited

Loading

plebhash commented Aug 19, 2024 •

edited

Loading

plebhash commented Aug 19, 2024 •

edited

Loading