Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eth,core: add api debug_getTrieFlushInterval #27303

Merged
merged 3 commits into from
Jun 6, 2023

Conversation

jsvisa
Copy link
Contributor

@jsvisa jsvisa commented May 19, 2023

No description provided.

eth/api.go Outdated Show resolved Hide resolved
@jsvisa jsvisa changed the title eth: check interval in debug_setTrieFlushInterval api eth,core: add api debug_getTrieFlushInterval May 20, 2023
Copy link
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

Sounds good to me!

Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

Looks okish to me. The feature itself is tiny enough.

But honestly I am not a big fan of it. I am not sure when the reference based
in-memory garbage collection will be deprecated. It's supposed to be
replaced by path-based anyway. Feel like we will remove this feature a few
months later.

eth/api.go Outdated Show resolved Hide resolved
Signed-off-by: jsvisa <delweng@gmail.com>
Signed-off-by: jsvisa <delweng@gmail.com>
@jsvisa
Copy link
Contributor Author

jsvisa commented May 30, 2023

@rjl493456442 Thank you for your honest suggestion and careful consideration. We now adjust the gc commit rate on our node based on the current CPU and memory usage, as well as the growth ratio of the disk. We hope to know the current commit rate before making any adjustments. And we can drop the two apis together after merging pbss.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

Two nitpicks, but generally LGTM.
As rjl points out, this will be moot after PBSS though 🤷

eth/api_debug.go Outdated Show resolved Hide resolved
eth/api_debug.go Outdated Show resolved Hide resolved
@holiman holiman added this to the 1.12.1 milestone Jun 6, 2023
@holiman holiman merged commit 0783cb7 into ethereum:master Jun 6, 2023
@jsvisa jsvisa deleted the eth-check-trie-interval branch June 7, 2023 07:16
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
* core,eth: add api debug_getTrieFlushInterval

Signed-off-by: jsvisa <delweng@gmail.com>

* eth/api_debug: comment of SetTrieFlushInterval

Signed-off-by: jsvisa <delweng@gmail.com>

* Apply suggestions from code review

---------

Signed-off-by: jsvisa <delweng@gmail.com>
Co-authored-by: Martin Holst Swende <martin@swende.se>
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants