From bba6818c3852e1b7d279df56c84b95a3ebcd8fad Mon Sep 17 00:00:00 2001 From: Weny Xu Date: Fri, 6 Nov 2026 04:06:14 +0000 Subject: [PATCH] fix(ci): honor configured region in metric snapshot benchmark (#10157) * fix(ci): honor configured region in metric snapshot benchmark Signed-off-by: WenyXu * docs(ci): clarify AWS runner region configuration Include the documentation changes from #10158 alongside the region fix. Signed-off-by: Weny Xu --------- Signed-off-by: WenyXu Signed-off-by: Weny Xu --- .github/scripts/ci-runners.md | 8 +++- .github/scripts/metric-snapshot-s3-bench.py | 7 ++-- .github/scripts/metric-snapshot-s3-ops.py | 37 +++++++++++------ .../tests/test_metric_snapshot_s3_bench.py | 19 +++++++++ .../tests/test_metric_snapshot_s3_ops.py | 40 +++++++++++++++++++ .../workflows/metric-snapshot-s3-bench.yml | 6 +++ docs/how-to/metric-snapshot-s3-bench.md | 17 +++++--- 7 files changed, 112 insertions(+), 22 deletions(-) diff --git a/.github/scripts/ci-runners.md b/.github/scripts/ci-runners.md index 657d76866dd..f21bdbdb0af 100644 --- a/.github/scripts/ci-runners.md +++ b/.github/scripts/ci-runners.md @@ -13,8 +13,12 @@ AWS uses `aws-ec2-runner-provision.py` and `aws-ec2-runner-teardown.py` directly Repository variables supply `EC2_RUNNER_REGION`, `EC2_RUNNER_SUBNET_ID`, and `EC2_RUNNER_SECURITY_GROUP_ID`; credentials use `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and `GH_PERSONAL_ACCESS_TOKEN` secrets. -The current region is Singapore (`ap-southeast-1`). By default the script selects -the newest available Canonical Ubuntu 24.04 amd64 server AMI in that region +The runner region comes from `EC2_RUNNER_REGION` (currently `us-east-1`). +The subnet and security group must belong to the selected AWS account and region. +For benchmarks using real S3, `AWS_CI_TEST_BUCKET_REGION` must match the runner +region; `AWS_CI_TEST_BUCKET` must be accessible with the configured credentials. +By default the script selects the newest available Canonical Ubuntu 24.04 amd64 +server AMI in that region (owner `099720109477`) and logs its ID. Set `BENCHMARK_EC2_IMAGE_ID` to pin a prepared Ubuntu AMI. Existing build CI's `EC2_RUNNER_LINUX_AMD64_IMAGE_ID` is unaffected. Both workflows default `system_disk_gib` to `auto` (also accepting diff --git a/.github/scripts/metric-snapshot-s3-bench.py b/.github/scripts/metric-snapshot-s3-bench.py index 7bb01a410b1..6542a18ba04 100644 --- a/.github/scripts/metric-snapshot-s3-bench.py +++ b/.github/scripts/metric-snapshot-s3-bench.py @@ -105,8 +105,9 @@ class Benchmark: self.endpoint = os.environ.get("S3_ENDPOINT", "") if self.endpoint and not local_smoke: raise ValueError("Custom S3 endpoints are restricted to local smoke") - if not local_smoke and os.environ["S3_REGION"] != "ap-southeast-1": - raise ValueError("Cloud benchmark requires ap-southeast-1") + if not local_smoke and (not os.environ["S3_REGION"].strip() + or os.environ["S3_REGION"] != os.environ["RUNNER_REGION"]): + raise ValueError("Cloud benchmark requires the same nonempty bucket and runner region") self.containers = set() self.network_created = False self.phases = [] @@ -324,7 +325,7 @@ level = "info" lines = ["# Metric packed snapshot benchmark", "", f"Image: `{self.image}`", f"Source commit: `{self.commit}`", "", "Local smoke only; timings are not AWS results." if self.local_smoke else - "AWS S3 in ap-southeast-1; source and target use local File storage.", "", + f"S3 in {os.environ['S3_REGION']}; source and target use local File storage.", "", "| Round | Phase | Seconds | Result |", "| --- | --- | ---: | --- |"] for phase in self.phases: lines.append(f"| {phase['round']} | {phase['phase']} | {phase['wall_seconds']:.3f} | {phase['outcome']} |") diff --git a/.github/scripts/metric-snapshot-s3-ops.py b/.github/scripts/metric-snapshot-s3-ops.py index f1e34defb89..f639ec90979 100644 --- a/.github/scripts/metric-snapshot-s3-ops.py +++ b/.github/scripts/metric-snapshot-s3-ops.py @@ -21,7 +21,7 @@ """Budget preflight and strictly scoped S3 operations for the metric snapshot benchmark.""" import argparse -from decimal import Decimal +from decimal import Decimal, InvalidOperation import json import os from pathlib import Path @@ -29,8 +29,8 @@ import re import time import urllib.request -REGION = "ap-southeast-1" -HOURLY_USD = Decimal("0.9408") +# Historical budget estimate; dispatch can supply a rate for the configured region. +DEFAULT_HOURLY_USD = "0.9408" OTHER_RESERVE_USD = Decimal("1.50") @@ -40,6 +40,16 @@ def owned_prefix(prefix): return prefix +def hourly_estimate(env): + try: + estimate = Decimal(env.get("EC2_HOURLY_USD", DEFAULT_HOURLY_USD)) + except InvalidOperation as error: + raise ValueError("EC2_HOURLY_USD must be a positive finite estimate") from error + if not estimate.is_finite() or estimate <= 0: + raise ValueError("EC2_HOURLY_USD must be a positive finite estimate") + return estimate + + def plan(env): image = env["GREPTIMEDB_IMAGE"] if not re.fullmatch(r"greptime/greptimedb(?:-nightly)?@sha256:[0-9a-f]{64}", image): @@ -49,21 +59,22 @@ def plan(env): tables, minutes = int(env["LOGICAL_TABLES_PER_GROUP"]), int(env["BENCHMARK_TIMEOUT_MINUTES"]) if not 50 <= tables <= 10_000 or not 30 <= minutes <= 240: raise ValueError("logical tables per group must be 50..10000; timeout must be 30..240 minutes") - if env["S3_REGION"] != REGION or env["RUNNER_REGION"] != REGION: - raise ValueError("bucket and runner must both use ap-southeast-1") + if not env["S3_REGION"].strip() or env["S3_REGION"] != env["RUNNER_REGION"]: + raise ValueError("bucket and runner must use the same nonempty region") if not env["S3_BUCKET"]: raise ValueError("S3 bucket is required") prior = Decimal(env["PRIOR_SPEND_USD"]) # Reserve 45m provision, 10m queue allowance and 20m cleanup beyond the job timeout. - reservation = HOURLY_USD * Decimal(minutes + 75) / 60 + OTHER_RESERVE_USD + hourly = hourly_estimate(env) + reservation = hourly * Decimal(minutes + 75) / 60 + OTHER_RESERVE_USD if not prior.is_finite() or prior < 0 or prior >= 15 or prior + reservation > 20: raise ValueError("campaign budget exhausted: prior spend must be below $15 and leave the full attempt reserve within $20") return {"image": image, "expected_commit": env["EXPECTED_COMMIT"], "logical_tables_per_group": tables, "smoke_logical_tables": 500, "timeout_minutes": minutes, "prefix": owned_prefix(env["RUN_PREFIX"]), - "instance_type": "c6id.4xlarge", "region": REGION, + "instance_type": "c6id.4xlarge", "region": env["S3_REGION"], "prior_estimated_usd": str(prior), "attempt_reserve_usd": str(reservation), - "ec2_hourly_usd": str(HOURLY_USD), "other_reserve_usd": str(OTHER_RESERVE_USD), + "ec2_hourly_usd": str(hourly), "other_reserve_usd": str(OTHER_RESERVE_USD), "cost_note": "Estimate, not an AWS billing total or a hard spending cap."} @@ -71,8 +82,8 @@ def client(region): import boto3 from botocore.config import Config - if region != REGION: - raise ValueError("S3 region must be ap-southeast-1") + if not region.strip(): + raise ValueError("S3 region is required") return boto3.client("s3", region_name=region, config=Config( connect_timeout=10, read_timeout=30, retries={"mode": "standard", "max_attempts": 3})) @@ -193,8 +204,10 @@ def main(): if args.started_at is None: parser.error("cost requires --started-at") seconds = max(0, int(time.time()) - args.started_at) - estimate = HOURLY_USD * Decimal(seconds) / 3600 - result = {"allocation_window_seconds": seconds, "ec2_estimate_usd": str(round(estimate, 4)), + hourly = hourly_estimate(os.environ) + estimate = hourly * Decimal(seconds) / 3600 + result = {"allocation_window_seconds": seconds, "ec2_hourly_usd": str(hourly), + "ec2_estimate_usd": str(round(estimate, 4)), "campaign_estimate_with_other_reserve_usd": str(round( Decimal(os.environ["PRIOR_SPEND_USD"]) + estimate + OTHER_RESERVE_USD, 4)), "note": "Allocation wall-time estimate plus $1.50 reserve for S3, disk and IPv4; not an AWS bill."} diff --git a/.github/scripts/tests/test_metric_snapshot_s3_bench.py b/.github/scripts/tests/test_metric_snapshot_s3_bench.py index c662ecbff76..092771e1154 100644 --- a/.github/scripts/tests/test_metric_snapshot_s3_bench.py +++ b/.github/scripts/tests/test_metric_snapshot_s3_bench.py @@ -28,6 +28,25 @@ SPEC.loader.exec_module(bench) class PhaseTests(unittest.TestCase): + def test_cloud_region_configuration(self): + for bucket, runner, accepted in (("us-east-1", "us-east-1", True), + ("ap-southeast-1", "ap-southeast-1", True), + ("us-east-1", "ap-southeast-1", False), + ("", "", False)): + with self.subTest(bucket=bucket, runner=runner), tempfile.TemporaryDirectory() as directory: + env = dict(GREPTIMEDB_IMAGE="greptime/greptimedb@sha256:" + "a" * 64, + EXPECTED_COMMIT="b" * 40, RUN_PREFIX="metric-snapshot-bench/123-1/", + RESULTS=str(Path(directory) / "results"), DATA_ROOT=str(Path(directory) / "data"), + S3_REGION=bucket, RUNNER_REGION=runner, S3_ENDPOINT="") + with patch.dict(bench.os.environ, env): + if accepted: + benchmark = bench.Benchmark(local_smoke=False) + benchmark.summary() + self.assertIn(f"S3 in {bucket};", (benchmark.results / "summary.md").read_text()) + else: + with self.assertRaisesRegex(ValueError, "same nonempty"): + bench.Benchmark(local_smoke=False) + def test_docker_nanosecond_timestamps(self): start = bench.docker_timestamp("2026-10-14T06:06:44.72000818Z") end = bench.docker_timestamp("2026-10-14T06:06:46.123456789Z") diff --git a/.github/scripts/tests/test_metric_snapshot_s3_ops.py b/.github/scripts/tests/test_metric_snapshot_s3_ops.py index d22c09044b2..5c1a265af0f 100644 --- a/.github/scripts/tests/test_metric_snapshot_s3_ops.py +++ b/.github/scripts/tests/test_metric_snapshot_s3_ops.py @@ -20,6 +20,7 @@ import io import json from pathlib import Path import tempfile +import types import unittest from unittest import mock @@ -82,6 +83,45 @@ class OperationsTest(unittest.TestCase): with self.subTest(changes=changes), self.assertRaises(ValueError): ops.plan(self.env(**changes)) + def test_regions_follow_configuration_and_reject_mismatch_or_empty(self): + for region in ("us-east-1", "ap-southeast-1"): + with self.subTest(region=region): + self.assertEqual(ops.plan(self.env(S3_REGION=region, RUNNER_REGION=region))["region"], region) + for bucket, runner in (("us-east-1", "ap-southeast-1"), ("", ""), (" ", " ")): + with self.subTest(bucket=bucket, runner=runner), self.assertRaises(ValueError): + ops.plan(self.env(S3_REGION=bucket, RUNNER_REGION=runner)) + + def test_client_uses_configured_region(self): + boto = types.ModuleType("boto3") + boto.client = mock.Mock() + config = types.ModuleType("botocore.config") + config.Config = mock.Mock() + with mock.patch.dict("sys.modules", {"boto3": boto, "botocore.config": config}): + for region in ("us-east-1", "ap-southeast-1"): + ops.client(region) + self.assertEqual(boto.client.call_args.kwargs["region_name"], region) + with self.assertRaises(ValueError): + ops.client("") + + def test_hourly_estimate_controls_reservation_and_cost(self): + env = self.env(EC2_HOURLY_USD="2", BENCHMARK_TIMEOUT_MINUTES="30") + plan = ops.plan(env) + self.assertEqual(plan["ec2_hourly_usd"], "2") + self.assertEqual(plan["attempt_reserve_usd"], "5.00") + with mock.patch.dict(ops.os.environ, env), mock.patch.object(ops.time, "time", return_value=4600), \ + mock.patch("sys.argv", ["ops", "cost", "--started-at", "1000"]), \ + mock.patch("sys.stdout", new_callable=io.StringIO) as output: + ops.main() + cost = json.loads(output.getvalue()) + self.assertEqual(cost["ec2_hourly_usd"], "2") + self.assertEqual(cost["ec2_estimate_usd"], "2.0000") + self.assertEqual(cost["campaign_estimate_with_other_reserve_usd"], "3.5000") + for rate in ("", "invalid", "0", "-1", "NaN", "Infinity", "-Infinity"): + with self.subTest(rate=rate), self.assertRaises(ValueError): + ops.plan(self.env(EC2_HOURLY_USD=rate)) + with self.assertRaises(ValueError): + ops.plan(self.env(EC2_HOURLY_USD="4")) + def watcher_env(self): return self.env(GITHUB_REPOSITORY="GreptimeTeam/greptimedb", GITHUB_RUN_ID="123", GITHUB_RUN_ATTEMPT="2", GH_TOKEN="test-token", BENCHMARK_TIMEOUT_MINUTES="30") diff --git a/.github/workflows/metric-snapshot-s3-bench.yml b/.github/workflows/metric-snapshot-s3-bench.yml index 32dd9242fdb..8b961b1d4fa 100644 --- a/.github/workflows/metric-snapshot-s3-bench.yml +++ b/.github/workflows/metric-snapshot-s3-bench.yml @@ -18,6 +18,11 @@ on: description: 'Setup, 500-table smoke and full run timeout, 30..240 minutes' type: number default: 240 + ec2_hourly_usd: + description: 'Positive EC2 hourly USD budget estimate; historical default, not live regional pricing' + required: true + type: string + default: '0.9408' prior_spend_usd: description: 'Cumulative estimated cloud spend for all earlier attempts in this $20 campaign' required: true @@ -34,6 +39,7 @@ env: LOGICAL_TABLES_PER_GROUP: ${{ inputs.logical_tables_per_group }} BENCHMARK_TIMEOUT_MINUTES: ${{ inputs.benchmark_timeout_minutes }} PRIOR_SPEND_USD: ${{ inputs.prior_spend_usd }} + EC2_HOURLY_USD: ${{ inputs.ec2_hourly_usd }} RUN_PREFIX: metric-snapshot-bench/${{ github.run_id }}-${{ github.run_attempt }}/ S3_BUCKET: ${{ vars.AWS_CI_TEST_BUCKET }} S3_REGION: ${{ vars.AWS_CI_TEST_BUCKET_REGION }} diff --git a/docs/how-to/metric-snapshot-s3-bench.md b/docs/how-to/metric-snapshot-s3-bench.md index 86468bf3ffd..7bd5fb2efed 100644 --- a/docs/how-to/metric-snapshot-s3-bench.md +++ b/docs/how-to/metric-snapshot-s3-bench.md @@ -1,14 +1,17 @@ # Metric snapshot benchmark on AWS S3 The manual `Metric Snapshot S3 Benchmark` workflow tests packed Metric export and -restore against real S3 in `ap-southeast-1`. It must first exist on the repository's -default branch for GitHub to register `workflow_dispatch`. It was validated on AWS +restore against real S3 in the configured CI bucket region. It must first exist on +the repository's default branch for GitHub to register `workflow_dispatch`. It was validated on AWS by run [38544464731](https://github.com/GreptimeTeam/greptimedb/actions/runs/38544464731); [#9573](https://github.com/GreptimeTeam/greptimedb/pull/9573) records its conditions and results. ## Run Use the repository's existing EC2 runner configuration and CI S3 credentials. +`AWS_CI_TEST_BUCKET_REGION` and `EC2_RUNNER_REGION` must be nonempty and equal; +for example, both can be `us-east-1`. The configured subnet, security group and +AMI (if explicitly supplied) must also belong to the runner region. The workflow passes credentials only through CI; no local AWS credentials are needed. Source, target and CLI use the same immutable image. This image's source SHA and required CLI capabilities have been checked: @@ -20,6 +23,7 @@ gh workflow run metric-snapshot-s3-bench.yml \ -f expected_commit=f690bbb2040865c0d22b0b57d51612b60ae27cd0 \ -f logical_tables_per_group=10000 \ -f benchmark_timeout_minutes=240 \ + -f ec2_hourly_usd=0.9408 \ -f prior_spend_usd=0 ``` @@ -28,9 +32,12 @@ earlier cloud attempts in the campaign, including failed setup and cleanup. Do not reset the estimate to zero when retrying. The workflow accepts 50–10,000 logical tables per group and a 30–240 minute benchmark job timeout. -The runner is fixed at `c6id.4xlarge` (16 vCPU, 32 GiB RAM). At the recorded -Singapore Linux on-demand estimate of $0.9408/hour, the default attempt reserves -$6.4392: four benchmark hours, 45 minutes of provisioning, 10 minutes for queueing, +The runner is fixed at `c6id.4xlarge` (16 vCPU, 32 GiB RAM). Set +`ec2_hourly_usd` to a positive, finite hourly budget estimate appropriate for the +selected region and instance. Its default $0.9408/hour preserves the historical +Singapore estimate; it is not a live regional price or a guaranteed upper bound. +The same estimate is used for preflight reservation and the final cost summary. +At that default rate, the default attempt reserves $6.4392: four benchmark hours, 45 minutes of provisioning, 10 minutes for queueing, 20 minutes of cleanup, and $1.50 for S3, disk and IPv4 costs. Preflight rejects prior spend at or above $15, or an attempt whose reserve would exceed the cumulative $20 budget. These estimates and checks are not an AWS hard spending cap or an AWS bill.