Skip to content

EmptyMempool-error better management and bug fix (#744) #747

Merged
GitGab19 merged 5 commits intostratum-mining:mainfrom
GitGab19:fix-empty-mempool
Feb 13, 2024
Merged

EmptyMempool-error better management and bug fix (#744) #747
GitGab19 merged 5 commits intostratum-mining:mainfrom
GitGab19:fix-empty-mempool

Conversation

@GitGab19
Copy link
Member

Fix #744

@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2024

🐰Bencher

ReportMon, February 12, 2024 at 18:31:25 UTC
ProjectStratum v2 (SRI)
Branchfix-empty-mempool
Testbedsv1
BenchmarkEstimated CyclesEstimated Cycles Results
estimated cycles | (Δ%)
InstructionsInstructions Results
instructions | (Δ%)
L1 AccessesL1 Accesses Results
accesses | (Δ%)
L2 AccessesL2 Accesses Results
accesses | (Δ%)
RAM AccessesRAM Accesses Results
accesses | (Δ%)
get_authorize✅ (view plot)8557.000✅ (view plot)3772.000✅ (view plot)5292.000✅ (view plot)9.000✅ (view plot)92.000
get_submit✅ (view plot)95700.000✅ (view plot)59522.000✅ (view plot)85495.000✅ (view plot)53.000✅ (view plot)284.000
get_subscribe✅ (view plot)8126.000✅ (view plot)2848.000✅ (view plot)3976.000✅ (view plot)18.000✅ (view plot)116.000
serialize_authorize✅ (view plot)12386.000✅ (view plot)5343.000✅ (view plot)7451.000✅ (view plot)14.000✅ (view plot)139.000
serialize_deserialize_authorize✅ (view plot)24608.000✅ (view plot)9950.000✅ (view plot)14043.000✅ (view plot)41.000✅ (view plot)296.000
serialize_deserialize_handle_authorize✅ (view plot)30218.000✅ (view plot)12127.000✅ (view plot)17163.000✅ (view plot)63.000✅ (view plot)364.000
serialize_deserialize_handle_submit✅ (view plot)126565.000✅ (view plot)73307.000✅ (view plot)105090.000✅ (view plot)116.000✅ (view plot)597.000
serialize_deserialize_handle_subscribe✅ (view plot)27536.000✅ (view plot)9650.000✅ (view plot)13646.000✅ (view plot)69.000✅ (view plot)387.000
serialize_deserialize_submit✅ (view plot)115327.000✅ (view plot)68167.000✅ (view plot)97837.000✅ (view plot)68.000✅ (view plot)490.000
serialize_deserialize_subscribe✅ (view plot)23010.000✅ (view plot)8209.000✅ (view plot)11560.000✅ (view plot)43.000✅ (view plot)321.000
serialize_submit✅ (view plot)100081.000✅ (view plot)61566.000✅ (view plot)88341.000✅ (view plot)52.000✅ (view plot)328.000
serialize_subscribe✅ (view plot)11519.000✅ (view plot)4195.000✅ (view plot)5834.000✅ (view plot)17.000✅ (view plot)160.000

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2024

🐰Bencher

ReportMon, February 12, 2024 at 18:31:22 UTC
ProjectStratum v2 (SRI)
Branchfix-empty-mempool
Testbedsv1
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)
client-submit-serialize✅ (view plot)6867.300
client-submit-serialize-deserialize✅ (view plot)7830.000
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle✅ (view plot)8349.700
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle✅ (view plot)934.590
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize✅ (view plot)720.180
client-sv1-authorize-serialize/client-sv1-authorize-serialize✅ (view plot)246.970
client-sv1-get-authorize/client-sv1-get-authorize✅ (view plot)158.140
client-sv1-get-submit✅ (view plot)6629.700
client-sv1-get-subscribe/client-sv1-get-subscribe✅ (view plot)283.370
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle✅ (view plot)765.260
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize✅ (view plot)775.970
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize✅ (view plot)204.410

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2024

🐰Bencher

ReportMon, February 12, 2024 at 18:31:23 UTC
ProjectStratum v2 (SRI)
Branchfix-empty-mempool
Testbedsv2
BenchmarkEstimated CyclesEstimated Cycles Results
estimated cycles | (Δ%)
InstructionsInstructions Results
instructions | (Δ%)
L1 AccessesL1 Accesses Results
accesses | (Δ%)
L2 AccessesL2 Accesses Results
accesses | (Δ%)
RAM AccessesRAM Accesses Results
accesses | (Δ%)
client_sv2_handle_message_common✅ (view plot)2067.000✅ (view plot)469.000✅ (view plot)727.000✅ (view plot)9.000✅ (view plot)37.000
client_sv2_handle_message_mining✅ (view plot)8107.000✅ (view plot)2081.000✅ (view plot)3067.000✅ (view plot)42.000✅ (view plot)138.000
client_sv2_mining_message_submit_standard✅ (view plot)6378.000✅ (view plot)1756.000✅ (view plot)2553.000✅ (view plot)23.000✅ (view plot)106.000
client_sv2_mining_message_submit_standard_serialize✅ (view plot)14733.000✅ (view plot)4700.000✅ (view plot)6763.000✅ (view plot)47.000✅ (view plot)221.000
client_sv2_mining_message_submit_standard_serialize_deserialize✅ (view plot)27416.000✅ (view plot)10543.000✅ (view plot)15341.000✅ (view plot)84.000✅ (view plot)333.000
client_sv2_open_channel✅ (view plot)4443.000✅ (view plot)1461.000✅ (view plot)2158.000✅ (view plot)9.000✅ (view plot)64.000
client_sv2_open_channel_serialize✅ (view plot)14164.000✅ (view plot)5064.000✅ (view plot)7319.000✅ (view plot)39.000✅ (view plot)190.000
client_sv2_open_channel_serialize_deserialize✅ (view plot)22566.000✅ (view plot)7979.000✅ (view plot)11606.000✅ (view plot)78.000✅ (view plot)302.000
client_sv2_setup_connection✅ (view plot)4791.000✅ (view plot)1502.000✅ (view plot)2271.000✅ (view plot)14.000✅ (view plot)70.000
client_sv2_setup_connection_serialize✅ (view plot)16220.000✅ (view plot)5963.000✅ (view plot)8660.000✅ (view plot)42.000✅ (view plot)210.000
client_sv2_setup_connection_serialize_deserialize✅ (view plot)35550.000✅ (view plot)14806.000✅ (view plot)21750.000✅ (view plot)93.000✅ (view plot)381.000

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2024

🐰Bencher

ReportMon, February 12, 2024 at 18:31:21 UTC
ProjectStratum v2 (SRI)
Branchfix-empty-mempool
Testbedsv2
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)
client_sv2_handle_message_common✅ (view plot)43.984
client_sv2_handle_message_mining✅ (view plot)67.449
client_sv2_mining_message_submit_standard✅ (view plot)14.641
client_sv2_mining_message_submit_standard_serialize✅ (view plot)265.120
client_sv2_mining_message_submit_standard_serialize_deserialize✅ (view plot)579.630
client_sv2_open_channel✅ (view plot)166.940
client_sv2_open_channel_serialize✅ (view plot)285.860
client_sv2_open_channel_serialize_deserialize✅ (view plot)370.290
client_sv2_setup_connection✅ (view plot)165.280
client_sv2_setup_connection_serialize✅ (view plot)469.840
client_sv2_setup_connection_serialize_deserialize✅ (view plot)1028.000

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

warn!("Template Provider is running, but its mempool is empty (possible reasons: you're testing in testnet, signet, or regtest)")
}
mempool::JdsMempoolError::NoClient => {
error!("Unable to connect to Template Provider (possible reasons: not fully synced, down)");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe clarify that this is about the RPC connection, not the sv2 connection.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe clarify that this is about the RPC connection, not the sv2 connection.

Do you have a suggestion for that?

warn!("{:?}", err);
match err {
mempool::JdsMempoolError::EmptyMempool => {
warn!("Template Provider is running, but its mempool is empty (possible reasons: you're testing in testnet, signet, or regtest)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but it's logged every second.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate is determined by the [mempool_update_timeout] parameter set in config

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, now I remember you already suggested a change in this parameter name!
Don't know where to find that suggestion btw :D

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Polling every second is fine, but the warning should just appear once, otherwise the log becomes too noisy.

Copy link
Member Author

@GitGab19 GitGab19 Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? In this way it could be harder to detect it, and I think that EmptyMempool is very important to detect in production

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could log it only once using something like let mut empty_mempool_logged = false

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you could make it more noisy on mainnet only.

@lorbax lorbax mentioned this pull request Feb 12, 2024
@GitGab19
Copy link
Member Author

GitGab19 commented Feb 12, 2024

@Sjors I applied your suggestions by changing the error log into "Unable to establish RPC connection with Template Provider (possible reasons: not fully synced, down)". I also added a way to log the EmptyMempool warning every 10 / mempool_update_interval.as_secs() cycles of the loop.
For example, with mempool_update_interval = 2 secs in config, the warning should be displayed every 5 cycles (or 10 seconds)

let mempool_cloned_ = mempool.clone();
let (status_tx, status_rx) = unbounded();
let sender = status::Sender::Downstream(status_tx.clone());
let empty_mempool_log_counter = 10 / mempool_update_interval.as_secs();
Copy link
Collaborator

@Sjors Sjors Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about (in pseudo-code):

let last_empty_mempool_warning = 1970-1-1;

and then below

if last_empty_mempool_warning < 1.minute.ago() {
  log...
  last_empty_mempool_warning = now()
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 log message per minute should be plenty. That's 10 times per average block interval.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to log the warning every minute, right?
If so, I can push this last changes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not every minute: no more than once a minute. If the interval is set to more than 1 minute it would log every interval.

let (status_tx, status_rx) = unbounded();
let sender = status::Sender::Downstream(status_tx.clone());
let empty_mempool_log_counter = 10 / mempool_update_interval.as_secs();
let mut last_empty_mempool_warning = std::time::Instant::now();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to set this in the past, otherwise it won't warn the first time.

@Sjors
Copy link
Collaborator

Sjors commented Feb 12, 2024

Looks good, but I didn't re-test.

I suggest squashing (most) commits.

(and renaming the PR to something more descriptive)

fmt

EmptyMempool warning logged every minute

last fixes

fmt fix
@GitGab19 GitGab19 changed the title Fix #744 EmptyMempool-error better management and bug fix (#744) Feb 12, 2024
@GitGab19
Copy link
Member Author

Looks good, but I didn't re-test.

I suggest squashing (most) commits.

(and renaming the PR to something more descriptive)

Done

@Sjors
Copy link
Collaborator

Sjors commented Feb 12, 2024

Lgtm. Will retest later.

Copy link
Collaborator

@Fi3 Fi3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@Sjors Sjors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ef5b0c3

Tested that the warning only appears once a minute. And that it doesn't appear when there's something in the mempool.

@GitGab19 GitGab19 merged commit 0b2d2fb into stratum-mining:main Feb 13, 2024
@GitGab19 GitGab19 deleted the fix-empty-mempool branch July 7, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Job Declarator server fails with EmptyMempool

3 participants

Comments