Skip to content

Commit fbc5e1a

Browse files
facontidavideclaude
andcommitted
perf(builder): default --extract-workers 8→64; size boto3 pool to match
Catalog extraction is latency-bound: each file is 1-2 sequential S3 range GETs (~190ms each over WAN), so throughput = concurrency / per-file latency. At the old default of 8 workers a full cold scan runs ~20 files/s (measured on a ~24.5k-file partition: ~20 min). Raise the default to 64. The companion fix is essential: boto3's default connection pool is 10, so >10 workers would contend and retry (pool-full → slower). Build the S3 client with Config(max_pool_connections=max(workers,10)) so the pool tracks the worker count. botocore is imported defensively so the s3 path stays exercisable with a bare fake boto3 (no botocore) in tests. Docs + test updated: README flag table, --help text, and test_parser_defaults (asserts 64; fake boto3.client stub now accepts the config= kwarg). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ec4a7a8 commit fbc5e1a

3 files changed

Lines changed: 29 additions & 6 deletions

File tree

mcap_catalog/mcap_catalog_builder/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ python3 -m mcap_catalog_builder --source s3 --s3-bucket B --sqs-url U # S3
2424
| `--tag-socket` | off | path for the tag-edit IPC unix socket (daemon mode only; see [Tag-edit IPC](#tag-edit-ipc)) |
2525
| `--rescan-interval` | `300.0` | seconds between safety re-scans |
2626
| `--no-watch` | off | daemon mode: start **no** live event producer (no local watchdog/inotify observer, no S3 SQS long-poll thread) — discovery is then rescan-only, driven purely by `--rescan-interval`. With `--source s3`, also drops the `--sqs-url` requirement. No-op for `--source gcs` (already rescan-only) and for `--once` |
27-
| `--extract-workers` | `8` | threads that fetch MCAP summaries during a full reconcile (the network-bound, out-of-transaction read). DB writes stay single-threaded; `1` = sequential. Higher values hide per-file round-trip latency on a remote bucket |
27+
| `--extract-workers` | `64` | threads that fetch MCAP summaries during a full reconcile (the network-bound, out-of-transaction read). DB writes stay single-threaded; `1` = sequential. Extraction is latency-bound (each file is 1–2 sequential range GETs), so throughput scales ~linearly with this until S3's per-prefix limits — the boto3 client's HTTP connection pool is sized to match (`max_pool_connections = max(workers, 10)`). Lower it for a tiny or local bucket |
2828
| `--debounce` | `2.0` | [local] seconds to debounce file events |
2929
| `--stability-checks` | `3` | [local] size-stability polls before cataloging |
3030
| `--stability-interval` | `0.5` | [local] seconds between stability polls |

mcap_catalog/mcap_catalog_builder/__main__.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,15 @@ def build_parser() -> argparse.ArgumentParser:
7979
help="[local] size-stability poll count before cataloging (default: 3)")
8080
p.add_argument("--stability-interval", type=float, default=0.5,
8181
help="[local] seconds between size-stability polls (default: 0.5)")
82-
p.add_argument("--extract-workers", type=int, default=8,
82+
p.add_argument("--extract-workers", type=int, default=64,
8383
help="number of threads that fetch MCAP summaries during a full "
8484
"reconcile (the network-bound, out-of-transaction read). DB "
8585
"writes stay single-threaded; this only parallelizes reads. "
86-
"1 = sequential. Higher values hide per-file round-trip "
87-
"latency on a remote bucket (default: 8).")
86+
"1 = sequential. Extraction is latency-bound (each file is "
87+
"1-2 sequential range GETs), so throughput scales ~linearly "
88+
"with this until S3's per-prefix limits — the S3 client's HTTP "
89+
"connection pool is sized to match. Lower it for a tiny or "
90+
"local bucket (default: 64).")
8891
p.add_argument("--log-level", default="INFO",
8992
choices=["DEBUG", "INFO", "WARNING", "ERROR"])
9093
return p
@@ -165,7 +168,24 @@ def main(argv: list[str] | None = None) -> int:
165168
from .s3_storage import S3Source
166169
from .s3_producer import s3_event_producer
167170

168-
source = S3Source(boto3.client("s3"), args.s3_bucket, args.s3_prefix)
171+
# Size the HTTP connection pool to the extract concurrency: extraction fans
172+
# out --extract-workers threads of range GETs, and boto3's default pool of 10
173+
# would throttle anything above ~10 (pool-full warnings + retries → SLOWER,
174+
# not faster). Never drop below the default 10 for a low worker count.
175+
# botocore is imported defensively so the s3 path stays exercisable with a
176+
# bare fake boto3 (no botocore) in tests / no-AWS environments.
177+
client_kwargs: dict = {}
178+
try:
179+
from botocore.config import Config
180+
181+
client_kwargs["config"] = Config(
182+
max_pool_connections=max(args.extract_workers, 10)
183+
)
184+
except ImportError:
185+
pass # no botocore -> fall back to boto3's default connection pool
186+
source = S3Source(
187+
boto3.client("s3", **client_kwargs), args.s3_bucket, args.s3_prefix
188+
)
169189

170190
def start_producer() -> None:
171191
threading.Thread(

mcap_catalog/mcap_catalog_builder/tests/test_cli.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ def test_parser_defaults():
3434
assert args.stability_interval == 0.5
3535
assert args.log_level == "INFO"
3636
assert args.source == "local" # default backend
37+
assert args.extract_workers == 64 # wide default: extraction is latency-bound
3738

3839

3940
def test_parser_s3_options():
@@ -129,7 +130,9 @@ def test_main_s3_daemon_no_watch_skips_sqs_requirement(tmp_path, monkeypatch):
129130
import mcap_catalog_builder.s3_producer as s3_producer_mod
130131

131132
fake_boto3 = types.ModuleType("boto3")
132-
fake_boto3.client = lambda name: _EmptyS3Client() if name == "s3" else object()
133+
# Real boto3.client accepts a `config=` kwarg (we pass a botocore Config to size
134+
# the connection pool); the fake must tolerate it.
135+
fake_boto3.client = lambda name, **kw: _EmptyS3Client() if name == "s3" else object()
133136
monkeypatch.setitem(sys.modules, "boto3", fake_boto3)
134137

135138
producer_calls = []

0 commit comments

Comments
 (0)