8000 fees: prevent redundant estimates flushes by ismaelsadeeq · Pull Request #32748 · bitcoin/bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fees: prevent redundant estimates flushes #32748

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fees: prevent redundant estimates flushes #32748

wants to merge 2 commits into from

Conversation

ismaelsadeeq
Copy link
Member
@ismaelsadeeq ismaelsadeeq commented Jun 14, 2025 •
8000
edited
Loading

This PR does two things:

  1. It prevents redundant writes to the fee_estimates.dat file when there is no usable data available. For example, during IBD, blocks are still being connected, but unnecessary flushes still occur every hour.

  2. It updates the flushing log to use debug-level logging under the estimatefee category. It also ensures the log consistently includes only the full file path.

@DrahtBot
Copy link
Contributor
DrahtBot commented Jun 14, 2025 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32748.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK Eunovo

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)
  • #29307 (util: explicitly close all AutoFiles that have been written by vasild)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@ismaelsadeeq ismaelsadeeq force-pushed the 06-2025-prevent-redundant-estimate-flushes branch from 21746c6 to 48a3e8f Compare June 14, 2025 06:05
@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed.
Task fuzzer,address,undefined,integer, no depends: https://github.com/bitcoin/bitcoin/runs/44091090061
LLM reason (✨ experimental): The CI failure is caused by a thread-safety analysis error in policy_estimator.cpp due to unsafe mutex usage during a function call.

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

This was referenced Jun 15, 2025
Draft
Open
Eunovo
Eunovo reviewed Jun 19, 2025
View reviewed changes
Copy link
Contributor
@Eunovo Eunovo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Concept ACK 48a3e8f

Left some comments about logging.

@ismaelsadeeq ismaelsadeeq force-pushed the 06-2025-prevent-redundant-estimate-flushes branch from 48a3e8f to 458fdac Compare June 19, 2025 13:34
ismaelsadeeq
ismaelsadeeq commented Jun 19, 2025
View reviewed changes
Copy link
Member Author
@ismaelsadeeq ismaelsadeeq left a comment

Choose a reason for hiding this comment

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

Addressed comments by @Eunovo thanks for review 21f0deaf1..458fdac

@ismaelsadeeq
fees: make flushes log debug only
4a0672f 
- Also log the full file path of fee_estimates.dat consistently
  not just the filename.
@ismaelsadeeq ismaelsadeeq force-pushed the 06-2025-prevent-redundant-estimate-flushes branch from 458fdac to 4a0672f Compare June 19, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eunovo Eunovo Eunovo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ismaelsadeeq @DrahtBot @Eunovo
0