fix: deduplicate shared downloads across test suites - #390
Conversation
999bd70 to
ba391b4
Compare
73eecd5 to
10cffd4
Compare
|
@rsanchez87 all comments addressed. |
10cffd4 to
64faa42
Compare
64faa42 to
95eb4f3
Compare
|
I see a first issue is that the download extraction is very slow in case of AV1, it seems to be due to the read of the archive which is repeated for each test vector when it could be extracted once. So I would extract everything at once to a cache and move the test vector to a secure place |
dabrain34
left a comment
dabrain34
left a comment
There was a problem hiding this comment.
The main one is the large archive extraction
| @@ -0,0 +1,446 @@ | |||
| # Fluster - testing framework for decoders conformance | |||
There was a problem hiding this comment.
the download manager should also provide the download patch for a given test vector which can then be used in gen_mpeg4_video.py+282
|
|
||
| print("All downloads finished") | ||
|
|
||
| def download_test_suite(self, test_suite: Any, jobs: int) -> None: |
| @@ -0,0 +1,446 @@ | |||
| # Fluster - testing framework for decoders conformance | |||
| # Copyright (C) 2020, Fluendo, S.A. | |||
| # Author: Pablo Marcos Oltra <pmarcos@fluendo.com>, Fluendo, S.A. | |||
There was a problem hiding this comment.
Are you Pablo is the author ? Do you think I could be added to the credits as well ?
| @@ -0,0 +1,446 @@ | |||
| # Fluster - testing framework for decoders conformance | |||
| # Copyright (C) 2020, Fluendo, S.A. | |||
There was a problem hiding this comment.
I assume its not 2020 but 2026
| """Path to the shared download cache directory.""" | ||
| return os.path.join(self.out_dir, self.CACHE_DIR) | ||
|
|
||
| def download(self, test_suites: List[Any], jobs: int) -> None: |
There was a problem hiding this comment.
do you want any here or TestSuite ?
|
|
||
| for result in results: | ||
| if not result.successful(): | ||
| sys.exit("Some download failed") |
There was a problem hiding this comment.
would great to know which download failed as it can happen any time
| suite_name: str | ||
| test_vector_name: str | ||
| input_file: str | ||
| suite_root: bool = False |
There was a problem hiding this comment.
we could keep the output path here
| if not self.keep_file and os.path.exists(cache_path): | ||
| os.remove(cache_path) | ||
|
|
||
| def _extract_to_suite_root(self, destination: _Destination, cache_path: str, source_filename: str) -> None: |
There was a problem hiding this comment.
This method can be very slow in case of large archive such as the AV1 test vectors one. It can last more than 40 minutes to extract all the test vectors from the archive because it extracts (1.5s) X 3500 test vectors
@dabrain34 agreed and good catch, each Suggestion I tested: extract all zip members in a single pass (one
The download-to-cache / distribute structure stays unchanged, only the extraction loop is batched see #395 |
Download each unique source URL once for the whole selection of test suites and distribute it to every suite that needs it, instead of fetching shared archives once per suite. Sources are downloaded in parallel into resources/.cache/<md5(url)>/ and the cache is removed at the end of the run unless --keep is used. Download orchestration moves from TestSuite into a new fluster/download_manager.py module. TestSuite.download() remains as a thin wrapper for the generator scripts, and --mirror support is preserved.
Add filename_from_url() so query strings and fragments are stripped when naming files, and use it for the download destination and the shared download cache. Signed URLs like GCS/S3 links end with ?X-Amz-Signature=..., which broke extension checks and left odd filenames on disk. Ported from fluendo#338.
DownloadManager used retries**retries, so -r 5 meant 3125 download attempts per URL before failing. download() already retries internally with backoff; pass the user-provided count instead. Ported from fluendo#338.
When two selected test suites share a source URL but declare different checksums, abort instead of silently trusting whichever checksum was seen first. Also prefer a real checksum over __skip__ when both appear for the same URL. Ported from fluendo#338.
If a cached archive cannot be extracted (corrupt zip/tar or gunzip failure), delete it from the cache and report a clear error, so the next run re-downloads it instead of failing forever. A missing archive member is not treated as corruption and keeps the cache. Ported from fluendo#338.
Explain that sources shared by test suites are fetched once into resources/.cache/, that the cache is removed at the end of the run unless --keep is used, and that concurrent download runs against the same resources directory are not supported. Ported from fluendo#338.
95eb4f3 to
73204c5
Compare
3 participants
Test suites that share a source URL (such as the multi-GB AV1-ARGON archive used by a dozen suites) used to download it once per suite. This introduces a single
DownloadManager(fluster/download_manager.py) that fetches each unique source exactly once for the entire selection, then distributes and extracts it wherever needed.Based on the review of #338.
cc @dabrain34