Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Remove fragile assert_memory_usage_stable #1645

Merged
merged 1 commit into from
Dec 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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