Skip to content

Commit

Permalink
Fix flaky p2p-fullblocktest (#2605)
Browse files Browse the repository at this point in the history
* Announce blocks as HEADERS instead of INV in comptool

Announcing as INV results in one GETHEADERS message per INV item with
every GETHEADERS containing the same block locator info. This in turn
results in many (1088 in p2p-fullblocktests) replies with each having i+1
headers. As every message is sent in one go, the message processing queue
gets overloaded, leading to GETDATA timeouts for blocks and thus failing
the tests.

Announcing the blocks directly with HEADERS avoids all this and results in
one GETDATA per announced header.

This will later conflict with backported improvements from Bitcoin. When we
get to this point, we can simply ignore changes on Dash's side and take
the improvements from Bitcoin (which also switch to header announcement,
but after some refactorings)

* Give wait_for_pings more time

Large reorgs as in the p2p-fullblocktest can cause very long long delays
for the ping that we're waiting for. This was not a problem before #2593,
as before this wait_until was waiting forever.
  • Loading branch information
codablock authored and UdjinM6 committed Jan 3, 2019
1 parent 96d4f74 commit 0648496
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions qa/rpc-tests/test_framework/comptool.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,10 @@ def veracked():
return all(node.verack_received for node in self.test_nodes)
return wait_until(veracked, timeout=10)

def wait_for_pings(self, counter):
def wait_for_pings(self, counter, timeout=float('inf')):
def received_pongs():
return all(node.received_ping_response(counter) for node in self.test_nodes)
return wait_until(received_pongs)
return wait_until(received_pongs, timeout=timeout)

# sync_blocks: Wait for all connections to request the blockhash given
# then send get_headers to find out the tip of each node, and synchronize
Expand All @@ -222,7 +222,7 @@ def blocks_requested():

# Send ping and wait for response -- synchronization hack
[ c.cb.send_ping(self.ping_counter) for c in self.connections ]
self.wait_for_pings(self.ping_counter)
self.wait_for_pings(self.ping_counter, timeout=300)
self.ping_counter += 1

# Analogous to sync_block (see above)
Expand Down Expand Up @@ -358,12 +358,13 @@ def run(self):
else:
[ c.send_message(msg_block(block)) for c in self.connections ]
[ c.cb.send_ping(self.ping_counter) for c in self.connections ]
self.wait_for_pings(self.ping_counter)
self.wait_for_pings(self.ping_counter, timeout=300)
self.ping_counter += 1
if (not self.check_results(tip, outcome)):
raise AssertionError("Test failed at test %d" % test_number)
else:
invqueue.append(CInv(2, block.sha256))
block_header = CBlockHeader(block)
[ c.cb.send_header(block_header) for c in self.connections ]
elif isinstance(b_or_t, CBlockHeader):
block_header = b_or_t
self.block_store.add_header(block_header)
Expand Down

0 comments on commit 0648496

Please sign in to comment.