-
Notifications
You must be signed in to change notification settings - Fork 8k
[Data] Add autoscaler_scales_up_when_compute_bound release test #65772
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
ac24c52
1ff8a4a
d38fb8e
78896ae
24596c3
a68523d
61582d3
3c22277
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,115 @@ | ||||||
| import argparse | ||||||
| import math | ||||||
| import time | ||||||
| from typing import Any, Dict, Iterator | ||||||
|
|
||||||
| import numpy as np | ||||||
| import pyarrow as pa | ||||||
|
|
||||||
| import ray | ||||||
|
|
||||||
| from benchmark import Benchmark | ||||||
| from cluster_resource_monitor import ClusterResourceMonitor | ||||||
|
|
||||||
| # With 1000 inputs this takes ~30 minutes, long enough that node provisioning | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CMIIW I don’t understand the For example: def produce(_, *, blocks_per_input, sleep_s, block_shape):
for _ in range(blocks_per_input):
time.sleep(sleep_s)
yield {"data": np.zeros(block_shape, dtype=np.uint8)}
def consume(batch, *, sleep_s):
time.sleep(sleep_s)
return batch
def main(args):
num_inputs = 1000
blocks_per_input = 4
produce_sleep_s = 5
consume_sleep_s = 1
block_shape = (128, 1024, 1024)
rows_per_block = block_shape[0]
consume_batch_size = 2 * rows_per_block
expected_gpu_nodes = 10
cpus_per_node = 8
# Each consumer processes 2 blocks/s. Each producer emits 0.2 blocks/s,
# so one consumer needs 10 producers.
blocks_per_consumer_batch = consume_batch_size / rows_per_block
producers_per_consumer = (
blocks_per_consumer_batch * produce_sleep_s / consume_sleep_s
)
# Ten consumers need 100 producer CPUs. The GPU nodes provide 80,
# leaving a 20-CPU shortfall, or 3 CPU-only nodes.
producer_cpus_needed = expected_gpu_nodes * producers_per_consumer
producer_cpus_on_gpu_nodes = expected_gpu_nodes * cpus_per_node
cpu_shortfall = producer_cpus_needed - producer_cpus_on_gpu_nodes
min_cpu_nodes = math.ceil(cpu_shortfall / cpus_per_node)
producer = functools.partial(
produce,
blocks_per_input=blocks_per_input,
sleep_s=produce_sleep_s,
block_shape=block_shape,
)
consumer = functools.partial(consume, sleep_s=consume_sleep_s) |
||||||
| # speed doesn't make the test flaky. | ||||||
| NUM_INPUTS = 1000 | ||||||
| BLOCKS_PER_INPUT = 4 | ||||||
| PRODUCE_SLEEP_S = 5 | ||||||
| CONSUME_SLEEP_S = 1 | ||||||
| BLOCK_SHAPE = (128, 1024, 1024) | ||||||
| ROWS_PER_BLOCK = BLOCK_SHAPE[0] | ||||||
| CONSUME_BATCH_SIZE = 2 * ROWS_PER_BLOCK | ||||||
|
|
||||||
| # From the compute config. | ||||||
| MAX_GPU_NODES = 10 | ||||||
| CPUS_PER_NODE = 8 | ||||||
|
|
||||||
| EXPECTED_GPU_NODES = MAX_GPU_NODES | ||||||
|
|
||||||
| # Calculation for MIN_CPU_NODES. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we simplify this calculation? There are many intermediate constants, which makes it hard to follow. For example, a short comment could explain that 10 GPU workers require 100 producer CPUs, the GPU nodes provide 80 CPUs, and the remaining 20 CPUs require 3 CPU-only nodes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I'd like to keep the derivation. With hardcoded values, changing any single constant means editing several lines and redoing the arithmetic by hand. I've collapsed the intermediates so the whole calculation reads in one place. Any thoughts about this? |
||||||
| _PRODUCE_BLOCKS_PER_S = 1 / PRODUCE_SLEEP_S | ||||||
| _BLOCKS_PER_CONSUME_BATCH = CONSUME_BATCH_SIZE / ROWS_PER_BLOCK | ||||||
| _CONSUME_BLOCKS_PER_S = _BLOCKS_PER_CONSUME_BATCH / CONSUME_SLEEP_S | ||||||
| _PRODUCE_WORKERS_PER_CONSUME_WORKER = _CONSUME_BLOCKS_PER_S / _PRODUCE_BLOCKS_PER_S | ||||||
|
|
||||||
| _CPUS_NEEDED = EXPECTED_GPU_NODES * _PRODUCE_WORKERS_PER_CONSUME_WORKER | ||||||
| _CPUS_FROM_GPU_NODES = EXPECTED_GPU_NODES * CPUS_PER_NODE | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the CPU requirements are fully satisfied by the GPU nodes (i.e.,
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree. However, if that is the case, the test needs changes in sleeps or configs. Therefore, added assert to positive number and print(change numbers) when fails |
||||||
| MIN_CPU_NODES = math.ceil((_CPUS_NEEDED - _CPUS_FROM_GPU_NODES) / CPUS_PER_NODE) | ||||||
| assert MIN_CPU_NODES > 0, ( | ||||||
| "The GPU nodes' own CPUs already satisfy the pipeline, so this test no " | ||||||
| "longer exercises CPU scale-up. Adjust the sleeps or the compute config." | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def produce(_: Dict[str, np.ndarray]) -> Iterator[Dict[str, np.ndarray]]: | ||||||
| for _ in range(BLOCKS_PER_INPUT): | ||||||
| time.sleep(PRODUCE_SLEEP_S) | ||||||
| yield {"data": np.zeros(BLOCK_SHAPE, dtype=np.uint8)} | ||||||
|
|
||||||
|
|
||||||
| def consume(batch: Dict[str, np.ndarray]) -> Dict[str, np.ndarray]: | ||||||
| time.sleep(CONSUME_SLEEP_S) | ||||||
| return batch | ||||||
|
|
||||||
|
|
||||||
| def parse_args() -> argparse.Namespace: | ||||||
| parser = argparse.ArgumentParser() | ||||||
| parser.add_argument( | ||||||
| "--assert-max-cpu-nodes", | ||||||
| type=int, | ||||||
| default=None, | ||||||
| help="Assert that the autoscaler provisions at most this many CPU nodes.", | ||||||
| ) | ||||||
| return parser.parse_args() | ||||||
|
|
||||||
|
|
||||||
| def main(args: argparse.Namespace) -> Dict[str, Any]: | ||||||
| """This test checks if the cluster scales up enough to balance the pipeline.""" | ||||||
| if not ray.is_initialized(): | ||||||
| ray.init() | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this may be redundant. |
||||||
|
|
||||||
| input_blocks = [ | ||||||
| pa.Table.from_pydict({"input_id": [input_id]}) for input_id in range(NUM_INPUTS) | ||||||
| ] | ||||||
|
|
||||||
| with ClusterResourceMonitor() as monitor: | ||||||
| ds = ( | ||||||
| ray.data.from_blocks(input_blocks) | ||||||
| .map_batches(produce) | ||||||
| .map_batches(consume, num_gpus=1, batch_size=CONSUME_BATCH_SIZE) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we set |
||||||
| ) | ||||||
| # Don't materialize, so blocks are freed as they're consumed. | ||||||
| for _ in ds.iter_internal_ref_bundles(): | ||||||
| pass | ||||||
|
|
||||||
| peak_nodes = monitor.get_peak_node_counts() | ||||||
| print(f"Peak worker nodes: {peak_nodes.cpu} CPU, {peak_nodes.gpu} GPU") | ||||||
|
|
||||||
| assert peak_nodes.gpu == EXPECTED_GPU_NODES, ( | ||||||
| f"Expected the autoscaler to provision {EXPECTED_GPU_NODES} GPU nodes, " | ||||||
| f"but it provisioned {peak_nodes.gpu}" | ||||||
| ) | ||||||
| assert peak_nodes.cpu >= MIN_CPU_NODES, ( | ||||||
| f"Expected the autoscaler to provision at least {MIN_CPU_NODES} CPU nodes " | ||||||
| f"to balance the pipeline, but it provisioned {peak_nodes.cpu}" | ||||||
| ) | ||||||
| if args.assert_max_cpu_nodes is not None: | ||||||
| assert peak_nodes.cpu <= args.assert_max_cpu_nodes, ( | ||||||
| f"Expected the autoscaler to provision at most " | ||||||
| f"{args.assert_max_cpu_nodes} CPU nodes, but it provisioned " | ||||||
| f"{peak_nodes.cpu}" | ||||||
| ) | ||||||
|
|
||||||
| return { | ||||||
| "peak_cpu_nodes": peak_nodes.cpu, | ||||||
| "peak_gpu_nodes": peak_nodes.gpu, | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| if __name__ == "__main__": | ||||||
| args = parse_args() | ||||||
|
|
||||||
| benchmark = Benchmark() | ||||||
| benchmark.run_fn("main", main, args) | ||||||
| benchmark.write_result() | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Two autoscaling worker groups that both start empty. The GPU group uses CPU | ||
| # instances each with a single fake GPU, so this test can exercise heterogeneous | ||
| # autoscaling without using real GPUs. | ||
| cloud: {{env["ANYSCALE_CLOUD_NAME"]}} | ||
|
|
||
| head_node: | ||
| instance_type: m5.2xlarge | ||
|
|
||
| worker_nodes: | ||
| # Runs the produce tasks. | ||
| - name: cpu-node | ||
| instance_type: m5.2xlarge | ||
| min_nodes: 0 | ||
| max_nodes: 10 | ||
|
|
||
| # Runs the consume tasks. Only `GPU` is overridden, so these nodes still | ||
| # expose the 8 vCPUs of an m5.2xlarge and can run produce tasks too. The | ||
| # test's CPU-node arithmetic depends on that. | ||
| - name: gpu-node | ||
| instance_type: m5.2xlarge | ||
| min_nodes: 0 | ||
| max_nodes: 10 | ||
| resources: | ||
| GPU: 1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,45 @@ | ||
| import logging | ||
| import time | ||
| import threading | ||
| from typing import Tuple, Optional | ||
| from typing import NamedTuple, Tuple, Optional | ||
|
|
||
| import ray | ||
| from ray._common.constants import HEAD_NODE_RESOURCE_NAME | ||
| from ray.data._internal.execution.interfaces import ExecutionResources | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class NodeCounts(NamedTuple): | ||
| """The number of alive worker nodes, by whether they have a GPU.""" | ||
|
|
||
| cpu: int | ||
| gpu: int | ||
|
|
||
|
|
||
| def _count_worker_nodes() -> NodeCounts: | ||
| """Count the alive worker nodes, excluding the head node. | ||
|
|
||
| A node counts as a GPU node if it has any GPU resource. | ||
| """ | ||
| cpu_nodes = 0 | ||
| gpu_nodes = 0 | ||
| for node in ray.nodes(): | ||
| if not node.get("Alive", False): | ||
| continue | ||
| resources = node.get("Resources", {}) | ||
| if HEAD_NODE_RESOURCE_NAME in resources: | ||
| continue | ||
| if resources.get("GPU", 0) > 0: | ||
| gpu_nodes += 1 | ||
| else: | ||
| cpu_nodes += 1 | ||
| return NodeCounts(cpu=cpu_nodes, gpu=gpu_nodes) | ||
|
|
||
|
|
||
| class ClusterResourceMonitor: | ||
| """Monitor and validate cluster resources during benchmark execution. | ||
|
|
||
| This class tracks the peak number of cluster resources during execution. | ||
|
|
||
| This can be used to validate that the autoscaler behaves well. | ||
| """ | ||
|
|
||
|
|
@@ -23,6 +52,7 @@ def __init__(self): | |
|
|
||
| self._peak_cpu_count: float = 0 | ||
| self._peak_gpu_count: float = 0 | ||
| self._peak_node_counts = NodeCounts(cpu=0, gpu=0) | ||
|
|
||
| def __repr__(self): | ||
| return "ClusterResourceMonitor()" | ||
|
|
@@ -37,20 +67,36 @@ def __enter__(self): | |
| def get_peak_cluster_resources(self) -> ExecutionResources: | ||
| return ExecutionResources(cpu=self._peak_cpu_count, gpu=self._peak_gpu_count) | ||
|
|
||
| def get_peak_node_counts(self) -> NodeCounts: | ||
| """Get the peak number of alive worker nodes, excluding the head node.""" | ||
| return self._peak_node_counts | ||
|
|
||
| def _start_background_thread( | ||
| self, interval_s: float = 5.0 | ||
| ) -> Tuple[threading.Thread, threading.Event]: | ||
| stop_event = threading.Event() | ||
|
|
||
| def monitor_cluster_resources(): | ||
| while not stop_event.is_set(): | ||
| resources = ray.cluster_resources() | ||
| self._peak_cpu_count = max( | ||
| self._peak_cpu_count, resources.get("CPU", 0) | ||
| ) | ||
| self._peak_gpu_count = max( | ||
| self._peak_gpu_count, resources.get("GPU", 0) | ||
| ) | ||
| # These query the GCS, so a transient failure shouldn't kill the | ||
| # thread and leave the peaks frozen for the rest of the run. | ||
| try: | ||
| resources = ray.cluster_resources() | ||
| self._peak_cpu_count = max( | ||
| self._peak_cpu_count, resources.get("CPU", 0) | ||
| ) | ||
| self._peak_gpu_count = max( | ||
| self._peak_gpu_count, resources.get("GPU", 0) | ||
| ) | ||
|
|
||
| node_counts = _count_worker_nodes() | ||
| self._peak_node_counts = NodeCounts( | ||
| cpu=max(self._peak_node_counts.cpu, node_counts.cpu), | ||
| gpu=max(self._peak_node_counts.gpu, node_counts.gpu), | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to keep the CPU and GPU counts from the same sample? Since their peaks are tracked separately, we could end up with a combination that never existed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we track the peak CPU and GPU counts from the same sample, we won't catch cases where CPU or GPU is at its peak but the composite is not. I still agree that I was not clear to mention that CPU and GPU are tracked separately. I will track them in separate variables. |
||
| except Exception: | ||
| logger.warning("Failed to sample cluster state.", exc_info=True) | ||
|
|
||
| time.sleep(interval_s) | ||
|
|
||
| thread = threading.Thread(target=monitor_cluster_resources, daemon=True) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we could move these test-specific values into the function and add a docstring explaining the setup and expected scaling behavior? That would make the test easier to follow without jumping between so many module-level constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the values are repetitively referenced in
produceandconsumeand used in derivation, moving them to main() will raise the NameError. Therefore, I would rather keep them where they are and add the docstring at the top of the file.