Skip to content

Commit

Permalink
test: Remove fragile assert_memory_usage_stable (#1645)
Browse files Browse the repository at this point in the history
  • Loading branch information
Bushstar authored Dec 9, 2022
1 parent 4605cf4 commit 456eaac
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 70 deletions.
38 changes: 15 additions & 23 deletions test/functional/p2p_invalid_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
# file LICENSE or http://www.opensource.org/licenses/mit-license.php.
"""Test node responses to invalid network messages."""
import asyncio
import os
import struct
import sys

Expand Down Expand Up @@ -66,28 +65,21 @@ def run_test(self):
msg_at_size = msg_unrecognized(str_data="b" * valid_data_limit)
assert len(msg_at_size.serialize()) == msg_limit

increase_allowed = 0.7 # was 0.5
if [s for s in os.environ.get("DEFI_CONFIG", "").split(" ") if "--with-sanitizers" in s and "address" in s]:
# how much we should increase for that "sanitizers"?
increase_allowed = 3.5
with node.assert_memory_usage_stable(increase_allowed=increase_allowed):
self.log.info(
"Sending a bunch of large, junk messages to test "
"memory exhaustion. May take a bit...")

# Run a bunch of times to test for memory exhaustion.
for _ in range(40): # decreased (old value 80) due to VERY slow
node.p2p.send_message(msg_at_size)

# Check that, even though the node is being hammered by nonsense from one
# connection, it can still service other peers in a timely way.
for _ in range(20):
conn2.sync_with_ping(timeout=10) # increased (old value 2)

# Peer 1, despite serving up a bunch of nonsense, should still be connected.
self.log.info("Waiting for node to drop junk messages. Slow")
node.p2p.sync_with_ping(timeout=600) # increased (old value 120) due to VERY slow
assert node.p2p.is_connected
self.log.info("Sending a bunch of large, junk messages to test memory exhaustion. May take a bit...")

# Run a bunch of times to test for memory exhaustion. DeFi reduced to 40, was 80 before.
for _ in range(40):
node.p2p.send_message(msg_at_size)

# Check that, even though the node is being hammered by nonsense from one
# connection, it can still service other peers in a timely way.
for _ in range(20):
conn2.sync_with_ping(timeout=2)

# Peer 1, despite serving up a bunch of nonsense, should still be connected.
self.log.info("Waiting for node to drop junk messages.")
node.p2p.sync_with_ping(timeout=320)
assert node.p2p.is_connected

#
# 1.
Expand Down
47 changes: 0 additions & 47 deletions test/functional/test_framework/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,26 +174,6 @@ def generate(self, nblocks, maxtries=1000000, address=None):
mintedHashes.append(self.getblockhash(self.getblockcount())) # always "tip" due to chain switching (possibly wrong)
return mintedHashes


def get_mem_rss_kilobytes(self):
"""Get the memory usage (RSS) per `ps`.
Returns None if `ps` is unavailable.
"""
assert self.running

try:
return int(subprocess.check_output(
["ps", "h", "-o", "rss", "{}".format(self.process.pid)],
stderr=subprocess.DEVNULL).split()[-1])

# Avoid failing on platforms where ps isn't installed.
#
# We could later use something like `psutils` to work across platforms.
except (FileNotFoundError, subprocess.SubprocessError):
self.log.exception("Unable to get memory usage")
return None

def _node_msg(self, msg: str) -> str:
"""Return a modified msg that identifies this node by its index as a debugging aid."""
return "[node %d] %s" % (self.index, msg)
Expand Down Expand Up @@ -369,33 +349,6 @@ def assert_debug_log(self, expected_msgs, timeout=2):
time.sleep(0.05)
self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log))

@contextlib.contextmanager
def assert_memory_usage_stable(self, *, increase_allowed=0.03):
"""Context manager that allows the user to assert that a node's memory usage (RSS)
hasn't increased beyond some threshold percentage.
Args:
increase_allowed (float): the fractional increase in memory allowed until failure;
e.g. `0.12` for up to 12% increase allowed.
"""
before_memory_usage = self.get_mem_rss_kilobytes()

yield

after_memory_usage = self.get_mem_rss_kilobytes()

if not (before_memory_usage and after_memory_usage):
self.log.warning("Unable to detect memory usage (RSS) - skipping memory check.")
return

perc_increase_memory_usage = (after_memory_usage / before_memory_usage) - 1

if perc_increase_memory_usage > increase_allowed:
self._raise_assertion_error(
"Memory usage increased over threshold of {:.3f}% from {} to {} ({:.3f}%)".format(
increase_allowed * 100, before_memory_usage, after_memory_usage,
perc_increase_memory_usage * 100))

@contextlib.contextmanager
def profile_with_perf(self, profile_name):
"""
Expand Down

0 comments on commit 456eaac

Please sign in to comment.