feat: bound custom dataset-loader resource usage (#226) - #368
Open
ny-shy wants to merge 7 commits into
Open
Conversation
Add timeout and record-cap protections for custom dataset loaders. Uses SIGALRM for timeout (Unix only) and resource.getrusage for memory diagnostics. Default values (0) preserve existing behavior. - New: areno/cli/dataset_loader_guard.py (run_loader_with_limits, diagnostics) - New: tests/test_dataset_loader_guard_cpu.py (31 CPU tests) - Modified: trainer_config.py (loader_timeout_s, max_loader_records fields) - Modified: train.py (--loader-timeout-s, --max-loader-records CLI flags) - Modified: docs/cli/observability.rst (user documentation)
- Add getattr fallbacks for args.loader_timeout_s/args.max_loader_records in _trainer_config_from_options to avoid breaking existing tests - Remove unused 'field' import from dataclasses - Add 4 integration tests for _load_dataset_for_training -> guard path (skipped when torch is unavailable) - Skip guard overhead when custom loader is used without limits: call loader_fn directly instead of run_loader_with_limits
P1 fixes: - Default loader path now always goes through guard for diagnostics - LoaderDiagnostics.to_dict() for structured output - resource import made optional (Windows compatibility) P2 fixes: - Use setitimer for sub-second timeout precision, save/restore old timer - Record cap handles generators/non-sliceable via itertools.islice fallback - Unify memory units: macOS bytes -> KB conversion Tests: - Add TestBoundaryCases: generator, empty data, max_records=1, sub-second timeout, diagnostics to_dict (7 new tests) - Total: 38 passed + 4 skipped (integration tests need torch) Docs: - Add deterministic local example (tiny JSONL + loader script) - Add invalid parameters section - Update platform limitations with setitimer and resource notes
…lusionAI#226) - refactor timeout into _timeout_context with main-thread detection - log diagnostics on timeout and user exceptions - persist loader diagnostics to metrics_log_dir as JSON - fix invalid test assertions and add cross-platform skipif guards - add HuggingFace Dataset integration test for record cap - document SIGALRM limitations, global handler side effects, and Windows compatibility
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
描述:
What this PR does
Custom dataset loaders (
--dataset-loader-fn) currently run with no resource bounds — a slow loader blocks training indefinitely, an oversized return exhausts memory, and there's no diagnostic info on how long loading took or how much memory it consumed.This PR wraps every loader call with a guard that provides three capabilities:
Timeout.
--loader-timeout-s Ninstalls aSIGALRMtimer before invoking the loader. If the budget is exceeded,DatasetLoaderTimeout(aTimeoutErrorsubclass) is raised. User exceptions from the loader itself are re-raised unchanged — the two failure modes are distinguishable.SIGALRMis Unix-only; on other platforms the timeout is skipped with a logged warning. The alarm is always cancelled in afinallyblock, even on error.Record cap.
--max-loader-records Ninspects the returned object after the loader finishes. If it contains more than N items, the result is sliced to the first N. The original count and truncation flag are logged.Diagnostics. Every loader call — regardless of whether limits are set — logs wall-clock duration (
time.perf_counter), peak memory delta (resource.getrusage), record count, and truncation status. This gives operators visibility into loader behavior without enabling any flags.Both flags default to 0 (disabled). With defaults, the only overhead is the timing/diagnostics logging — the loader call itself is unchanged. No new dependencies; everything uses
signal,resource,timefrom the standard library.The guard lives in
areno/cli/dataset_loader_guard.pyas a singlerun_loader_with_limits()function. It's called from_load_dataset_for_training()intrain.py, wrapping both custom and default loaders. Config fields are added toTrainerConfigwith negative-value validation in__post_init__.Files changed:
areno/cli/dataset_loader_guard.py—run_loader_with_limits(),LoaderDiagnostics,DatasetLoaderTimeouttests/test_dataset_loader_guard_cpu.py— 31 CPU teststrainer_config.py—loader_timeout_s/max_loader_recordsfields with validationtrain.py— CLI flags, 4 config construction sites updated, loader call wrappeddocs/cli/observability.rst— user documentationRelated issue
Fixes #226
Type of change
How was it tested?
CPU unit tests on macOS (no GPU):
bash
python3 -m pytest tests/test_dataset_loader_guard_cpu.py -v
31 passed
Tests cover: timeout trigger, record truncation, user exception preservation, timeout-vs-error distinction, alarm cancellation after both success and timeout, default/unlimited behavior, config validation, diagnostics fields, and combined limits.
End-to-end on Kaggle T4 x2 with Qwen3-0.6B + alpaca-cleaned (51760 records):
truncated=False. Backward-compatible.--max-loader-records 10) — 51760 records truncated to 10. Log:truncated=True. Memory delta dropped from 224MB to 103MB. Training completed.timeout_s=2raisedDatasetLoaderTimeout.SIGALRMcancelled cleanly, subsequent calls unaffected.--max-loader-records 50 --loader-timeout-s 30) — truncated to 50, within timeout budget, training completed normally.No local GPU available, so full training was only validated on Kaggle T4 x2.
Checklist
pytest tests/test_dataset_loader_guard_cpu.py -v)