Page MenuHomePhabricator

Fix DNS seed dump flushing
ClosedPublic

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

Details

Reviewers
Fabien
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.

Event Timeline

Owners added a reviewer: Restricted Owners Package.Mar 8 2024, 23:49
thonkle requested review of this revision.Mar 8 2024, 23:49
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
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.

thonkle requested review of this revision.Mar 19 2024, 19:08
This revision is now accepted and ready to land.Mar 19 2024, 19:50
This revision was automatically updated to reflect the committed changes.