Skip to content

feat: Optimize the clone process to use threadpool - #54

Open
FallenDeity wants to merge 5 commits into
PokeAPI:masterfrom
FallenDeity:threadpool-and-order
Open

feat: Optimize the clone process to use threadpool#54
FallenDeity wants to merge 5 commits into
PokeAPI:masterfrom
FallenDeity:threadpool-and-order

Conversation

@FallenDeity

@FallenDeity FallenDeity commented Aug 20, 2026

Copy link
Copy Markdown

few updates to the ditto package

  • clone now uses a threadpool instead of a process pool (workload is io bound) some robustness added for session mgmt and retries
  • @from_path(api_path) has been refactored to use pathlib relative path over global state mutation by using os to change the dir
  • odictliteral remove this since python dicts are now ordered by default
  • orjson for fast json serialization and deserialization (to note now it outputs 2 space json)

and tada! (12 threads)

image
clone: 100%|███████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 50/50 [01:13<00:00,  1.46s/it]

1min 13sec

and dumps in same order as the api response, i think previously the order got messed up due to multiple sort_keys=True in a bunch of places, diff viewing should be clear now

$ python scripts/diff_data.py D:\projects\pokeapi-api-data\data D:\projects\pokeapi-ditto\data
[*] Scanning files in:
    Old: D:\projects\pokeapi-api-data\data
    New: D:\projects\pokeapi-ditto\data

============================================================
Summary: 15633 compared | 0 changed (0 total diffs) | 0 added | 0 removed
============================================================

ran an absolute json diff against api data, 0 diffs

@FallenDeity
FallenDeity marked this pull request as ready for review August 20, 2026 23:04
def _build_session() -> requests.Session:
session = requests.Session()
retry = Retry(
total=3,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, since we cannot affort to skip a resource, can we have a mechanism that fails the entire build if an endpoint cannot be scraped? Or is this the current behaviour? On my system and in the CICD I sometimes get these errors:

Request failure: http://localhost:8000/api/v2/location-area/943/ (HTTPConnectionPool(host='localhost', port=8000): Max retries exceeded with url: /api/v2/location-area/943/ (Caused by ReadTimeoutError("HTTPConnectionPool(host='localhost', port=8000): Read timed out. (read timeout=30)")))

But the file is present in the data directory, so I think a retry saved the scraping. I'd just like some thing that fails the entire process if an endpoint cannot be scraped.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the later transform step I got also:

FileNotFoundError: [Errno 2] No such file or directory: 'data/api/v2/pokemon/442/index.json'

So we must make sure to scrape everything. Maybe turning down the speed? Those errors might be because the server is stressed too much?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noted that the endpoints http://localhost:8000/api/v2/location-area/863/ on my PC are taking ~40s to respond (while ditto is running). Even by setting the CPU to only 4 (when I have 16) still dosen't help much

@FallenDeity FallenDeity Aug 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it looks like its timing out 12 threads might be overwhelming the webserver running which is a bit weird since thats not a lot of we are running it via gunicorn which spins up 8 workers and 8 threads per worker based on gunicorn.conf.py in pokeapi I'll give this a look after work today

8 since it uses 2 * cpu count (which is 4) I'll checkout the docker behaviour once

@Naramsim

Copy link
Copy Markdown
Member

I made some changes to the code to make it run and left a commend @FallenDeity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants