Skip to content

Commit c7709cb

Browse files
author
MarcoFalke
committed
Merge #17469: test: Remove fragile assert_memory_usage_stable
fac942c test: Remove fragile assert_memory_usage_stable (MarcoFalke) Pull request description: This test fails on arm64 and a fuzz tests seems inappropriate for the functional test suite anyway, so remove it. Example failures: * https://travis-ci.org/bitcoin/bitcoin/jobs/611497963#L14517 * https://travis-ci.org/MarcoFalke/bitcoin-core/jobs/611029104#L3876 ACKs for top commit: jamesob: ACK bitcoin/bitcoin@fac942c Tree-SHA512: 3577e7ce5891d221cb798454589ba796ed0c06621a26351bb919c23bc6bb46aafcd0b11cb02bbfde64b74d67cb2950da44959a7ecdc436491a34e8b045c1ccf4
2 parents 21ee676 + fac942c commit c7709cb

File tree

2 files changed

+15
-68
lines changed

2 files changed

+15
-68
lines changed

test/functional/p2p_invalid_messages.py

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Test node responses to invalid network messages."""
66
import asyncio
7-
import os
87
import struct
98
import sys
109

@@ -66,27 +65,21 @@ def run_test(self):
6665
msg_at_size = msg_unrecognized(str_data="b" * valid_data_limit)
6766
assert len(msg_at_size.serialize()) == msg_limit
6867

69-
increase_allowed = 0.5
70-
if [s for s in os.environ.get("BITCOIN_CONFIG", "").split(" ") if "--with-sanitizers" in s and "address" in s]:
71-
increase_allowed = 3.5
72-
with node.assert_memory_usage_stable(increase_allowed=increase_allowed):
73-
self.log.info(
74-
"Sending a bunch of large, junk messages to test "
75-
"memory exhaustion. May take a bit...")
76-
77-
# Run a bunch of times to test for memory exhaustion.
78-
for _ in range(80):
79-
node.p2p.send_message(msg_at_size)
80-
81-
# Check that, even though the node is being hammered by nonsense from one
82-
# connection, it can still service other peers in a timely way.
83-
for _ in range(20):
84-
conn2.sync_with_ping(timeout=2)
85-
86-
# Peer 1, despite serving up a bunch of nonsense, should still be connected.
87-
self.log.info("Waiting for node to drop junk messages.")
88-
node.p2p.sync_with_ping(timeout=320)
89-
assert node.p2p.is_connected
68+
self.log.info("Sending a bunch of large, junk messages to test memory exhaustion. May take a bit...")
69+
70+
# Run a bunch of times to test for memory exhaustion.
71+
for _ in range(80):
72+
node.p2p.send_message(msg_at_size)
73+
74+
# Check that, even though the node is being hammered by nonsense from one
75+
# connection, it can still service other peers in a timely way.
76+
for _ in range(20):
77+
conn2.sync_with_ping(timeout=2)
78+
79+
# Peer 1, despite serving up a bunch of nonsense, should still be connected.
80+
self.log.info("Waiting for node to drop junk messages.")
81+
node.p2p.sync_with_ping(timeout=320)
82+
assert node.p2p.is_connected
9083

9184
#
9285
# 1.

test/functional/test_framework/test_node.py

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -135,25 +135,6 @@ def get_deterministic_priv_key(self):
135135
assert len(self.PRIV_KEYS) == MAX_NODES
136136
return self.PRIV_KEYS[self.index]
137137

138-
def get_mem_rss_kilobytes(self):
139-
"""Get the memory usage (RSS) per `ps`.
140-
141-
Returns None if `ps` is unavailable.
142-
"""
143-
assert self.running
144-
145-
try:
146-
return int(subprocess.check_output(
147-
["ps", "h", "-o", "rss", "{}".format(self.process.pid)],
148-
stderr=subprocess.DEVNULL).split()[-1])
149-
150-
# Avoid failing on platforms where ps isn't installed.
151-
#
152-
# We could later use something like `psutils` to work across platforms.
153-
except (FileNotFoundError, subprocess.SubprocessError):
154-
self.log.exception("Unable to get memory usage")
155-
return None
156-
157138
def _node_msg(self, msg: str) -> str:
158139
"""Return a modified msg that identifies this node by its index as a debugging aid."""
159140
return "[node %d] %s" % (self.index, msg)
@@ -332,33 +313,6 @@ def assert_debug_log(self, expected_msgs, timeout=2):
332313
time.sleep(0.05)
333314
self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log))
334315

335-
@contextlib.contextmanager
336-
def assert_memory_usage_stable(self, *, increase_allowed=0.03):
337-
"""Context manager that allows the user to assert that a node's memory usage (RSS)
338-
hasn't increased beyond some threshold percentage.
339-
340-
Args:
341-
increase_allowed (float): the fractional increase in memory allowed until failure;
342-
e.g. `0.12` for up to 12% increase allowed.
343-
"""
344-
before_memory_usage = self.get_mem_rss_kilobytes()
345-
346-
yield
347-
348-
after_memory_usage = self.get_mem_rss_kilobytes()
349-
350-
if not (before_memory_usage and after_memory_usage):
351-
self.log.warning("Unable to detect memory usage (RSS) - skipping memory check.")
352-
return
353-
354-
perc_increase_memory_usage = (after_memory_usage / before_memory_usage) - 1
355-
356-
if perc_increase_memory_usage > increase_allowed:
357-
self._raise_assertion_error(
358-
"Memory usage increased over threshold of {:.3f}% from {} to {} ({:.3f}%)".format(
359-
increase_allowed * 100, before_memory_usage, after_memory_usage,
360-
perc_increase_memory_usage * 100))
361-
362316
@contextlib.contextmanager
363317
def profile_with_perf(self, profile_name):
364318
"""

0 commit comments

Comments
 (0)