Skip to content

fix: fix the hasrate loss when running cm on 2.9.5 alpha#852

Merged
JamesPiechota merged 6 commits into
masterfrom
jp/fix-cm-hashrate-20250819
Aug 20, 2025
Merged

fix: fix the hasrate loss when running cm on 2.9.5 alpha#852
JamesPiechota merged 6 commits into
masterfrom
jp/fix-cm-hashrate-20250819

Conversation

@JamesPiechota

Copy link
Copy Markdown
Collaborator

Also fixes the cache_limit_exceeded error spam

ar_mining_io_tests,
ar_mining_worker_tests,
ar_poller_tests,
ar_reject_chunks_tests,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it normal to remove this test from the suite?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It wsa a duplicate registration. Also exists in the long running tests section

ar_test_node:post_chunk(main, ar_serialize:jsonify(FirstProof3))
),
ar_test_node:mine(peer1),
assert_wait_until_height(main, 1),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need to wait here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it turns out we ma not need it. I added it because the loop afterwards is checking the status of the tx on th main node but the block had been mined on the peer1 node so without the wait it's possible the block doesn't get gossipped to main before the time out occurs.

That said, adding the wait did not ddress the core issue. I can remove it if needed.

Comment thread apps/arweave/src/ar_mining_worker.erl Outdated
@@ -525,8 +525,7 @@ handle_task({compute_h2_for_peer, Candidate, _ExtraArgs}, State) ->
ar_mining_stats:h1_received_from_peer(Peer, length(H1List)),
%% Mark second recall range as missing, just like if we didn't have the recall range

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These comments are outdated, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated!

@JamesPiechota JamesPiechota force-pushed the jp/fix-cm-hashrate-20250819 branch from ae8a3e4 to fcae200 Compare August 20, 2025 11:23
Comment thread apps/arweave/src/ar_mining_worker.erl Outdated
%% After we marked the whole second recall range as missing, we can cache the H1 list.
%% During this process, we also reset the chunk2_missing flag to false for the entries
%% we have H1 for.
%% First we mark the whole second recall range as missing

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am confused, do we mark the second or the first recall range as missing?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sigh. first recall range. I'd done the comment update before having coffee and incorrectly said "second"

Ive updated it again!

@JamesPiechota JamesPiechota force-pushed the jp/fix-cm-hashrate-20250819 branch from fcae200 to 58d1fae Compare August 20, 2025 12:51
@JamesPiechota JamesPiechota merged commit 139db35 into master Aug 20, 2025
38 of 86 checks passed
@JamesPiechota JamesPiechota deleted the jp/fix-cm-hashrate-20250819 branch May 4, 2026 01:03
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.

3 participants