Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

並行実行可能なスレッド数を制限 & 外部プロセス由来のエラー時にリトライ #6

Merged
merged 7 commits into from
Aug 9, 2023

Conversation

White-Green
Copy link
Collaborator

@White-Green White-Green commented Aug 3, 2023

並列実行可能なスレッド数を制限

10,000スレッドぐらいから同時にcontext.evalを呼ぶとErrno::ECONNREFUSEDのエラーが出る
実用上10,000並列で呼び出したいことはほぼ無さそうなので、並列実行可能数に制限をかけることで回避したい

TODO:

  • Semaphoreの実装をどうするか
    • 現実装はThread::Queueだが、これは目的外使用という感じなのでできれば避けたい
    • concurrent-rubyにSemaphoreがあるが、これだけのために依存を増やすべきかどうか(どうせSprocketsなどで使っているので良いというのはある)
    • ConditionVariableで自前実装は可能だが、それはそれでリスクがある
    • concurrent-rubyのSemaphore、ConditionVariableでの自前実装を試したが、Thread::Queueを使ったものが一番速かったのでこれにする
  • 同時実行可能数の制限をどうするか
    • 現実装では適当に100で固定
    • せめて何らかの根拠に基づいた値にしたい
    • macOSでは単一プロセスが開けるファイルディスクリプタの数が最大256らしいので、その半分に制限するようにした

外部プロセス由来のエラー時にリトライ

現状、実行中にNode.jsのプロセスが何らかの理由で落ちたりSocketが閉じたりするとそのままRuby側も落ちる実装になっている
execjsデフォルトのNode.jsランタイムなどではexec毎にプロセスを起動しているのでこのような予期しない問題はほぼ起きないが、pcruntimeのように単一のプロセスを長時間使い回す実装ではこのような問題が起こる確率が比較的高いため、もしも起こった場合のリトライ処理を実装した

@White-Green White-Green marked this pull request as ready for review August 4, 2023 10:43
@White-Green White-Green requested a review from a team August 4, 2023 10:44
@White-Green
Copy link
Collaborator Author

Node.jsのような外部プロセス由来のエラーが出た際にプロセスを再起動する実装をしました
別PRに分けるべきだと思いますが、コンフリクトする気しかしないのでこのPRでまとめてやろうと思います

@White-Green White-Green changed the title 並行実行可能なスレッド数を制限 並行実行可能なスレッド数を制限 & 外部プロセス由来のエラー時にリトライ Aug 9, 2023
@White-Green White-Green merged commit e23cbf1 into main Aug 9, 2023
3 checks passed
@White-Green White-Green deleted the limit_concurrency branch August 9, 2023 03:40
@White-Green White-Green mentioned this pull request Aug 9, 2023
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