Fix DNS seed dump flushing
ClosedPublic
Actions

Authored by thonkle on Mar 8 2024, 23:49.

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rABCdbb0719a8033: Fix DNS seed dump flushing

Summary

D10666 introduced a regression where the UninterruptibleSleep() call at the end of the loop may stall flushing of the DNS dump files until the sleep has completed. This results in long periods where some contents have been written to the files but not all. Example last line of the dump file snapshotted on some days:

==> /var/ecash-seeders/mainnet/dnsseed-2024-03-05.dump <==
158.69.24.50:8336                                   0   1590017628
==> /var/ecash-seeders/mainnet/dnsseed-2024-03-06.dump <==
203.134.96.84:8333                                  0   1578195195    6.44%   2.92%   1.11%   0.17%   0.04%  616413  00000424  70015 "/Bitcoin ABC:0.20.6(EB32.0)/
==> /var/ecash-seeders/mainnet/dnsseed-2024-03-07.dump <==
203.134.100.63:8333                                 0   1575572289    6.52%   2.99%   1.14%   0.17%   0.04
==> /var/ecash-seeders/mainnet/dnsseed.dump <==
188.234.101.27:8333                                 0

Test Plan

ninja bitcoin-seeder check-seeder
./src/seeder/bitcoin-seeder -host=seeder.status.cash -ns=localhost -mbox=thonkle@protonmail.com -port=5555 -dumpinterval=1

Over a few minutes, observe the end of the dump file:

while true ; do clear ; tail src/seeder/dnsseed.dump ; sleep 1 ; done

There should be no truncation on the last line with patch applied.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

seed-dump-flush

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27771
Build 55102: Build Diff	build-clang · build-debug · build-diff · build-without-wallet · lint-circular-dependencies · build-clang-tidy
Build 55101: arc lint + arc unit

Event Timeline

thonkle created this revision.Mar 8 2024, 23:49

Owners added a reviewer: Restricted Owners Package.Mar 8 2024, 23:49

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 8 2024, 23:49

thonkle requested review of this revision.Mar 8 2024, 23:49

Harbormaster completed remote builds in B27771: Diff 46067.Mar 9 2024, 00:07

Fabien requested changes to this revision.Mar 9 2024, 08:12

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/seeder/main.cpp
287	Why not just call ff.flush() ?

This revision now requires changes to proceed.Mar 9 2024, 08:12

thonkle added inline comments.Mar 12 2024, 17:10

src/seeder/main.cpp
287	Because there are more than one stream so the code must be maintained to flush all streams in this code block. `d` was actually the stream I cared about when doing initial investigation since that handles dnsseed.dump. But I do suspect the same issue exists for `ff`. The code would look like this if flushing each stream: d.flush(); f.flush(); ff.flush(); An explicit scope seems more comprehensive and robust to future changes.