Bitcoin Core Github
43 subscribers
122K links
Download Telegram
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/27776)
Im not saying my idea is the solution. I wrote it so that the internal team can have a real honest conversation about the ethic and integrity of the project.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch a
...
💬 brunoerg commented on pull request "p2p, rpc, test: `getpeerinfo` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#issuecomment-1567622316)
CI failure seems unrelated to these changes.
💬 brunoerg commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1209611483)
In `377e81e2e409470d00e69247d0d9e980773ab73e`, I suppose this verification could be avoided. In `Select`,
`new_only` is false by default and `preferred_net` is optional and it will only have a value when `MaybePickPreferredNetwork`returns true. So, if `preferred_net` doesn't have any value, I suppose that `addrman.Select(false, preferred_net)` and `addrman.Select()` will have the same effect.

```diff
@@ -1901,15 +1915,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string>
...
📝 MarcoFalke opened a pull request: "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN"
(https://github.com/bitcoin/bitcoin/pull/27777)
This should prevent the persistent workers from running out of disk space. Containers are already removed, but not images. This is required since CI images are built and cached.
💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1567867741)
This should fix the out-of-space error, see https://cirrus-ci.com/task/6118746146734080?logs=ci#L4903
💬 MarcoFalke commented on pull request "p2p, rpc, test: `getpeerinfo` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#issuecomment-1567868668)
> CI failure seems unrelated to these changes.

Thanks, fixed in https://github.com/bitcoin/bitcoin/pull/27777
💬 MarcoFalke commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567870784)
What about a one-line fix to clear the test issue and then a follow-up with the RPC, which may take longer to review than a trivial one-line patch?
💬 MarcoFalke commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1567890421)
A quick hack in the meantime could be:

```
sed -i 's/constexpr auto timeout{10s};/constexpr auto timeout{300s};/g' ./src/test/blockfilter_index_tests.cpp ./src/test/txindex_tests.cpp
```
💬 martinus commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#discussion_r1209810958)
About that function, I do not think that it is accurate any more. I wrote a different version of that estimation that makes use of `malloc_usable_size`, and that way I get different values, see here: https://godbolt.org/z/3a6E57dPx

It might be worth having a look if using that estimation is more accurate
💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1567905527)
Looks like it is working, but not claiming a lot of space:


https://cirrus-ci.com/task/6656469207089152?logs=ci#L408

```
Total reclaimed space: 949.4MB
💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1567995893)
Not sure what is going on. Seems there are unreachable zombie containers in the overlay2 directory. Let's try `podman` instead.
💬 hebasto commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568005106)
> Looks like it is working, but not claiming a lot of space:
>
> [cirrus-ci.com/task/6656469207089152?logs=ci#L408](https://cirrus-ci.com/task/6656469207089152?logs=ci#L408)
>
> ```
> Total reclaimed space: 949.4MB
> ```

What amount do you expect?
💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568008377)
I already wiped all servers, but I was expecting that about 50GB of zombie stuff is cleared:


```
# du -sh /var/lib/docker/overlay2/
53G /var/lib/docker/overlay2/
💬 hebasto commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568021295)
> Not sure what is going on. Seems there are unreachable zombie containers in the overlay2 directory. Let's try `podman` instead.

Maybe add:
```
docker volume prune --force
```
?
💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568026526)
I listed all imaged, volumes, containers, and networks and the list was empty, apart from 3 small image layers.
💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568100802)
(CI is green, but it will probably take 6 months to find out if podman is running out of storage space as well)
💬 willcl-ark commented on pull request "log: don't log total disk read time in ConnectTip bench":
(https://github.com/bitcoin/bitcoin/pull/27673#issuecomment-1568107647)
tACK bc862fad29

I was able to reproduce dropping the commit from this pull (and on master):

```log
2023-05-30T09:27:25Z loadblk thread start
validation.cpp:2839:5: runtime error: division by zero
2023-05-30T09:27:25Z [bench] - Load block from disk: 0.06ms [0.00s (infms/blk)]
2023-05-30T09:27:25Z [bench] - Connect total: 0.36ms [0.00s (0.36ms/blk)]
2023-05-30T09:27:25Z [bench] - Flush: 0.03ms [0.00s (0.03ms/blk)]
2023-05-30T09:27:25Z [bench] - Writing chainstate: 0.02ms [0.00s
...
fanquake closed an issue: "--with-sanitizers=float-divide-by-zero crash with -debug=bench in Chainstate::ConnectTip"
(https://github.com/bitcoin/bitcoin/issues/27635)
🚀 fanquake merged a pull request: "log: don't log total disk read time in ConnectTip bench"
(https://github.com/bitcoin/bitcoin/pull/27673)
💬 hebasto commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1568147747)
> (CI is green, but it will probably take 6 months to find out if podman is running out of storage space as well)

Could explicit logging of disk space usage help in the meanwhile?