Bitcoin Core Github
44 subscribers
122K links
Download Telegram
🤔 tdb3 reviewed a pull request: "rpc: increase the defaults for -rpcthreads and -rpcworkqueue"
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2425588523)
Approach ACK

Performed a very simplistic sanity check comparing 6463117a29294f6ddc9fafecfd1e9023956cc41b (commit under head) to head (e56fc7ce6a92eae7e80657d9f57a148cc002358d).

Ran `bitcoin-cli gettxoutsetinfo` 8x simultaneously on a system with 4 cores and timed the result (not claiming this to be the best test, just a quick check). The idea was to see if a core-constrained system would see a performance degradation from the PR change.

Without the increase of rpcthreads and rpcworkqueu
...
💬 maciejsszmigiero commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2466468043)
@l0rinc
> are you still working on this or should we take over?

I can obviously change the default in this PR to 16 MiB but I think having #30059 is important too: as you measured here on Oct 2 the best performing size on HDD storage actually seems to be 32 MiB.
💬 fjahr commented on pull request "scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}":
(https://github.com/bitcoin/bitcoin/pull/31163#issuecomment-2466520047)
Code review ACK 4120c7543ee32efed7396d7153411ecbbd588ad3

Did some light checks to see if anything was missed and reviewed changes.
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835675175)
Wouldn't I need to make `AddFlags` static as well in that case?
Which would trigger `this` removal and rewrite of the whole `AddFlags` method which is done in the next commit. Or you mean squash the first two commits?
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681246)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681261)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681273)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681298)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681312)
Done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1835681334)
> check that pointers are non-null if flags are set, or go further and check prev->next == self, and next->prev == self

the latter seems redundant, but the first is simple to check - done
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2466714671)
Thanks @ryanofsky & @andrewtoth, applied most nits, please re-review: `git range-diff a6921049..aa2f3139 a6921049..9edbe5a4` or https://github.com/bitcoin/bitcoin/compare/3b5b2457f713014fd5073da34b76a5a8cd796e4a..9edbe5a48fe9a2c0d364882fcb7d44ae6aac6d24
💬 BebeSparkelSparkel commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#issuecomment-2466758707)
@willcl-ark Thanks!
👍 willcl-ark approved a pull request: "doc: Fixup bitcoin-wallet manpage chain selection args"
(https://github.com/bitcoin/bitcoin/pull/31264#pullrequestreview-2425778116)
utACK fa729ab4a276c3462e390bf2fab6cad93d3a590d

Thanks Marco, this makes sense to me.
💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#discussion_r1835720824)
Fixed
💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#discussion_r1835720992)
I updated the commit message to include a reference to a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed where this check was added.

(will push shortly)
Sjors closed a pull request: "GitHub skip branch"
(https://github.com/bitcoin/bitcoin/pull/31265)
👍 tdb3 approved a pull request: "doc: Fixup bitcoin-wallet manpage chain selection args"
(https://github.com/bitcoin/bitcoin/pull/31264#pullrequestreview-2425820823)
Code Review ACK fa729ab4a276c3462e390bf2fab6cad93d3a590d

Would also support adding an example:
```diff
- "To change the target wallet, use the -datadir, -wallet and (test)chain selection arguments.\n"
+ "To change the target wallet, use the -datadir, -wallet and (test)chain selection arguments (e.g. -signet).\n"
```
💬 Sjors commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2466833320)
Github documentation is pretty terrible. The change here should work, but the jobs are not skipped in https://github.com/Sjors/bitcoin/pull/70
👍 andrewtoth approved a pull request: "rpc: increase the defaults for -rpcthreads and -rpcworkqueue"
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2425823091)
ACK e56fc7ce6a92eae7e80657d9f57a148cc002358d

Bumping the default threads and work queue enables a lot of workflows by default, like using it with a local address indexer. Right now these users have to manually configure the higher limits.
The higher limits won't increase memory unless they are being used. And these defaults can be set lower on memory constrained systems to disable using them.