Skip to content

Add warning on ParallelRunner about serialization/pickling errors - #5734

Open
lrcouto wants to merge 11 commits into
mainfrom
cloud-storage-warning-parallel-runner
Open

Add warning on ParallelRunner about serialization/pickling errors#5734
lrcouto wants to merge 11 commits into
mainfrom
cloud-storage-warning-parallel-runner

Conversation

@lrcouto

@lrcouto lrcouto commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

#4690

Add warning on ParallelRunner about serialization/pickling errors when using cloud-based datasets

What it does

Datasets backed by remote/cloud storage using fsspec frequently hold non-picklable internal states, like a cached filesystem client for example, and these do not play nice with ParallelRunner. It multiprocesses by running separate OS processes that don't share memory. Because these processes do not share memories, a pipe is needed to to communicate and share bytes between those processes. For this to happen, objects need to be serialized/pickled (multiprocessing.reduction.ForkingPickler), so no pickling, no multiprocessing.

This PR adds a warning to ParallelRunner in case one of these cloud protocols (s3, gcs, hdfs, etc) is being used, suggesting that they choose ThreadRunner instead. It uses threading instead of forking processes, so it doesn't have this same limitation.

Why not change the Datasets themselves?

It would be possible to change the self._fs = fsspec.filesystem(...) line on the "offending" datasets with a different pattern that avoids this issue. The problem is that this change would have to be done on around 40 different datasets, which would have to be either one massive PR that alters a lot of files, or many separate PRs that alter few files each, and would widen the scope of the original issue. Something that we might want to address in the future.

Checklist

  • Read the contributing guidelines
  • Linked to a relevant GitHub issue
  • Signed off each commit with a DCO
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

…n using cloud-based datasets

Signed-off-by: Laura Couto <laurarccouto@gmail.com>
@lrcouto lrcouto changed the title Add warning on ParallelRunner about serialization/pickling errors whe… Add warning on ParallelRunner about serialization/pickling errors Aug 18, 2026
Signed-off-by: Laura Couto <laurarccouto@gmail.com>
Signed-off-by: Laura Couto <laurarccouto@gmail.com>
Signed-off-by: Laura Couto <laurarccouto@gmail.com>
@lrcouto
lrcouto marked this pull request as ready for review August 18, 2026 03:45
@lrcouto
lrcouto requested review from SajidAlamQB and jitu5 August 18, 2026 03:45

@merelcht merelcht left a comment

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.

Thanks @lrcouto, this looks good to me 👍

One thing I was wondering is if we could also suggest using SequentialRunner? Or is the assumption the user wants some sort of optimisation and that's why they picked ParallelRunner in the first place?

@lrcouto

lrcouto commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @lrcouto, this looks good to me 👍

One thing I was wondering is if we could also suggest using SequentialRunner? Or is the assumption the user wants some sort of optimisation and that's why they picked ParallelRunner in the first place?

I was working under the assumption that if user picks ParallelRunner it's because they want some sort of optimization, since SequentialRunner is the default option. But I don't see an issue in adding it to the suggestions.

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