Skip to content

fix(utils): append_to_jsonl serialisation across threads - #511

Open
praann07 wants to merge 1 commit into
SylphAI-Inc:mainfrom
praann07:fix/jsonl-append-thread-safe
Open

praann07 wants to merge 1 commit into
SylphAI-Inc:mainfrom
praann07:fix/jsonl-append-thread-safe

Conversation

@praann07

Copy link
Copy Markdown

objective of this PR?

append_to_jsonl just calls jsonlines.open(f, mode="a") and writes one record per call. If multiple threads are writing to the same file (like trainer.diagnose run with multiple workers), their writes can interleave and a line gets chopped up by the next record. That is the corrupt line reported in #232.

This puts a module-level lock around the append so that threads in one process write one record at a time. The record is serialised (to_dict) outside the lock so the critical section is just the file write. Separate processes writing to the same file are still not coordinated; I noted that in the docstring.

Closes #232

How did I verify it?

Reproduction: 400 records with 20k char payload each, 8 threads appending.

Before the change, 253 of 400 lines were not valid JSON. After it, all 400 are.

new test adalflow/tests/test_file_io.py::test_append_to_jsonl_is_thread_safe (200 records, 8 threads). Without the change it fails (JSONDecodeError: Extra data) and with it it passes.

tests/test_file_io.py and tests/test_tracing.py: 34 passed

black --line-length=88 and ruff (versions pinned in .pre-commit-config.yaml) succeed on the changed files.

Things to know

I couldn't tell if this was the exact cause of the initial report; the thread above cites a bad file from a previous run as another possibility. This fixes the thread race I was able to reproduce.

Before Submitting

  • Have you discussed this issue with others? Yes, diagnose produces corrupted jsonl #232
  • I have read the Contributor Guide.
  • This PR does one thing.
  • I added a test for the change.
  • Existing tests I ran still pass

append_to_jsonl opened the file in append mode and wrote each record without any locking. This means that threads logging to the same jsonl file (e.g. trainer.diagnose with several workers) could interleave and leave a truncated line. Add a regression test that appends from 8 threads and checks every line parses. Protect the write with a module-level lock.

This branch has not been deployed

No deployments
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.

diagnose produces corrupted jsonl

1 participant