Audit uses of forkIO #3768
Assignees
Comments
27 tasks
|
Possible relation to #3388 |
jberryman
added a commit
to jberryman/graphql-engine
that referenced
this issue
Feb 13, 2020
This is the result of a general audit of how we fork threads, with a detour into how we're using mutable state especially in websocket codepaths, making more robust to async exceptions and exceptions resulting from bugs. Some highlights: - use a wrapper around 'immortal' so threads that die due to bugs are restarted, and log the error - use 'withAsync' some places - use bracket a few places where we might break invariants - log some codepaths that represent bugs - export UnstructuredLog for ad hoc logging (the alternative is we continue not logging useful stuff) I had to timebox this. There are a few TODOs I didn't want to address. And we'll wait until this is merged to attempt hasura#3705 for Control.Concurrent.Extended
Merged
10 tasks
jberryman
added a commit
to jberryman/graphql-engine
that referenced
this issue
Feb 25, 2020
This is the result of a general audit of how we fork threads, with a detour into how we're using mutable state especially in websocket codepaths, making more robust to async exceptions and exceptions resulting from bugs. Some highlights: - use a wrapper around 'immortal' so threads that die due to bugs are restarted, and log the error - use 'withAsync' some places - use bracket a few places where we might break invariants - log some codepaths that represent bugs - export UnstructuredLog for ad hoc logging (the alternative is we continue not logging useful stuff) I had to timebox this. There are a few TODOs I didn't want to address. And we'll wait until this is merged to attempt hasura#3705 for Control.Concurrent.Extended
jberryman
added a commit
to jberryman/graphql-engine
that referenced
this issue
Feb 25, 2020
This is the result of a general audit of how we fork threads, with a detour into how we're using mutable state especially in websocket codepaths, making more robust to async exceptions and exceptions resulting from bugs. Some highlights: - use a wrapper around 'immortal' so threads that die due to bugs are restarted, and log the error - use 'withAsync' some places - use bracket a few places where we might break invariants - log some codepaths that represent bugs - export UnstructuredLog for ad hoc logging (the alternative is we continue not logging useful stuff) I had to timebox this. There are a few TODOs I didn't want to address. And we'll wait until this is merged to attempt hasura#3705 for Control.Concurrent.Extended
jberryman
added a commit
to jberryman/graphql-engine
that referenced
this issue
Mar 2, 2020
This is the result of a general audit of how we fork threads, with a detour into how we're using mutable state especially in websocket codepaths, making more robust to async exceptions and exceptions resulting from bugs. Some highlights: - use a wrapper around 'immortal' so threads that die due to bugs are restarted, and log the error - use 'withAsync' some places - use bracket a few places where we might break invariants - log some codepaths that represent bugs - export UnstructuredLog for ad hoc logging (the alternative is we continue not logging useful stuff) I had to timebox this. There are a few TODOs I didn't want to address. And we'll wait until this is merged to attempt hasura#3705 for Control.Concurrent.Extended
0x777
added a commit
that referenced
this issue
Mar 5, 2020
This is the result of a general audit of how we fork threads, with a detour into how we're using mutable state especially in websocket codepaths, making more robust to async exceptions and exceptions resulting from bugs. Some highlights: - use a wrapper around 'immortal' so threads that die due to bugs are restarted, and log the error - use 'withAsync' some places - use bracket a few places where we might break invariants - log some codepaths that represent bugs - export UnstructuredLog for ad hoc logging (the alternative is we continue not logging useful stuff) I had to timebox this. There are a few TODOs I didn't want to address. And we'll wait until this is merged to attempt #3705 for Control.Concurrent.Extended
tirumaraiselvan
pushed a commit
to tirumaraiselvan/graphql-engine
that referenced
this issue
Mar 16, 2020
* docs: fix broken link (hasura#4005) * cli, server: use prerelease tag as channel for console assets cdn (hasura#3975) Co-authored-by: Shahidh K Muhammed <[email protected]> * Don't update catalog version if using --dryRun (hasura#3970) * cli(migrations-img): add env to skip update prompts (fix hasura#3964) (hasura#3968) * cli: add version flag in update-cli command (hasura#3996) Co-authored-by: Shahidh K Muhammed <[email protected]> * console: add TypeScript setup (hasura#3902) * docs: correct version info for config v2 section (close hasura#4019) (hasura#4026) * tag release v1.2.0-beta.2 (hasura#4028) * More robust forking, exception safety. Closes hasura#3768 (hasura#3860) This is the result of a general audit of how we fork threads, with a detour into how we're using mutable state especially in websocket codepaths, making more robust to async exceptions and exceptions resulting from bugs. Some highlights: - use a wrapper around 'immortal' so threads that die due to bugs are restarted, and log the error - use 'withAsync' some places - use bracket a few places where we might break invariants - log some codepaths that represent bugs - export UnstructuredLog for ad hoc logging (the alternative is we continue not logging useful stuff) I had to timebox this. There are a few TODOs I didn't want to address. And we'll wait until this is merged to attempt hasura#3705 for Control.Concurrent.Extended * docs: avoid redirect, update title tag suffix (hasura#4030) * console: hide starter kit button if a framework has no starter kit (hasura#4023) * console: update actions intro image (hasura#4042) * docs: add note on pg versions for actions (hasura#4034) * console: fix run_sql migration modal messaging (close hasura#4020) (hasura#4060) * cli: fix typo in cli example for squash (fix hasura#4047) (hasura#4049) * update changelog to include all commits for 1.2 (hasura#4066) * docs: add latest prerelease build info (close hasura#4041) (hasura#4048) * Server upgrade tests: Do not fail it no tests were collected (hasura#4071) Co-authored-by: Nizar Malangadan <[email protected]> Co-authored-by: Vamshi Surabhi <[email protected]> * server(events): utilize proper backpressure scheme (close hasura#3839) (hasura#4013) * Test working through a backlog of change events * Use a slightly more performant threaded http server in eventing pytests This helped locally but not on CI it seems... * Rework event processing for backpressure. Closes hasura#3839 With loo low `HASURA_GRAPHQL_EVENTS_FETCH_INTERVAL` and/or slow webhooks and/or too small `HASURA_GRAPHQL_EVENTS_HTTP_POOL_SIZE` we might previously check out events from the DB faster than we can service them, leading to space leaks, weirdness, etc. Other changes: - avoid fetch interval sleep latency when we previously did a non-empty fetch - prefetch event batch while http pool is working - warn when it appears we can't keep up with events being generated - make some effort to process events in creation order so we don't starve older ones. ALSO NOTE: HASURA_GRAPHQL_EVENTS_FETCH_INTERVAL changes semantics slightly, since it only comes into play after an empty fetch. The old semantics weren't documented in detail, so I think this is fine. * modified the logic of consuming scheduled events - made it similar to the logic being used in the event-triggers * fix the warnings in the EventTrigger and ScheduledTrigger files * use "scheduled events" in the logs instead of "events" * add documentation for the processScheduledQueue function * log the HTTP error in processEventQueue * revert the Scheduled Triggers logic to as it was earlier * fork new threads using forkImmortal instead of forkIO for ST threads * remove the commented functions from Eventing/HTTP.hs * refactor ExtraContext in Eventing back to ExtraLogContext * refactor the Eventing/HTTP file - move event trigger specific things to EventTrigger.hs - refactor mkInvo to mkInvocation * use bracket_ to do the async stuff in event triggers * refactor the processEventQueue function Co-authored-by: Praveen Durairaju <[email protected]> Co-authored-by: Aravind Shankar <[email protected]> Co-authored-by: Shahidh K Muhammed <[email protected]> Co-authored-by: Vamshi Surabhi <[email protected]> Co-authored-by: Shahidh K Muhammed <[email protected]> Co-authored-by: Rikin Kachhia <[email protected]> Co-authored-by: Rishichandra Wawhal <[email protected]> Co-authored-by: Marion Schleifer <[email protected]> Co-authored-by: Meera Sundar <[email protected]> Co-authored-by: Dmitry Minkovsky <[email protected]> Co-authored-by: nizar-m <[email protected]> Co-authored-by: Nizar Malangadan <[email protected]> Co-authored-by: Brandon Simmons <[email protected]>
tirumaraiselvan
added a commit
to tirumaraiselvan/graphql-engine
that referenced
this issue
Mar 16, 2020
* docs: fix broken link (hasura#4005) * cli, server: use prerelease tag as channel for console assets cdn (hasura#3975) Co-authored-by: Shahidh K Muhammed <[email protected]> * Don't update catalog version if using --dryRun (hasura#3970) * cli(migrations-img): add env to skip update prompts (fix hasura#3964) (hasura#3968) * cli: add version flag in update-cli command (hasura#3996) Co-authored-by: Shahidh K Muhammed <[email protected]> * console: add TypeScript setup (hasura#3902) * docs: correct version info for config v2 section (close hasura#4019) (hasura#4026) * tag release v1.2.0-beta.2 (hasura#4028) * More robust forking, exception safety. Closes hasura#3768 (hasura#3860) This is the result of a general audit of how we fork threads, with a detour into how we're using mutable state especially in websocket codepaths, making more robust to async exceptions and exceptions resulting from bugs. Some highlights: - use a wrapper around 'immortal' so threads that die due to bugs are restarted, and log the error - use 'withAsync' some places - use bracket a few places where we might break invariants - log some codepaths that represent bugs - export UnstructuredLog for ad hoc logging (the alternative is we continue not logging useful stuff) I had to timebox this. There are a few TODOs I didn't want to address. And we'll wait until this is merged to attempt hasura#3705 for Control.Concurrent.Extended * docs: avoid redirect, update title tag suffix (hasura#4030) * console: hide starter kit button if a framework has no starter kit (hasura#4023) * console: update actions intro image (hasura#4042) * docs: add note on pg versions for actions (hasura#4034) * console: fix run_sql migration modal messaging (close hasura#4020) (hasura#4060) * cli: fix typo in cli example for squash (fix hasura#4047) (hasura#4049) * update changelog to include all commits for 1.2 (hasura#4066) * docs: add latest prerelease build info (close hasura#4041) (hasura#4048) * Server upgrade tests: Do not fail it no tests were collected (hasura#4071) Co-authored-by: Nizar Malangadan <[email protected]> Co-authored-by: Vamshi Surabhi <[email protected]> * server(events): utilize proper backpressure scheme (close hasura#3839) (hasura#4013) * Test working through a backlog of change events * Use a slightly more performant threaded http server in eventing pytests This helped locally but not on CI it seems... * Rework event processing for backpressure. Closes hasura#3839 With loo low `HASURA_GRAPHQL_EVENTS_FETCH_INTERVAL` and/or slow webhooks and/or too small `HASURA_GRAPHQL_EVENTS_HTTP_POOL_SIZE` we might previously check out events from the DB faster than we can service them, leading to space leaks, weirdness, etc. Other changes: - avoid fetch interval sleep latency when we previously did a non-empty fetch - prefetch event batch while http pool is working - warn when it appears we can't keep up with events being generated - make some effort to process events in creation order so we don't starve older ones. ALSO NOTE: HASURA_GRAPHQL_EVENTS_FETCH_INTERVAL changes semantics slightly, since it only comes into play after an empty fetch. The old semantics weren't documented in detail, so I think this is fine. * docs: fix typo in action example (hasura#4064) Co-authored-by: Rikin Kachhia <[email protected]> * docs: remove spaces from action handler templated url (hasura#4081) * docs: add metadata descriptions to actions docs (hasura#4082) Co-authored-by: Rikin Kachhia <[email protected]> * auto-include `__typename` field in custom types' objects (fix hasura#4063) (hasura#4074) * include `__typename` field in custom types' objects, fix hasura#4063 Co-authored-by: Vamshi Surabhi <[email protected]> * console: manage postgres check constraints (hasura#3881) * Add check constraints to create table view * Add input field for check in new column row * Minor changes * Remove check input field * Add tooltip * Move tooltips to Common/ * Refactor tooltips * Move expandedContent to separate component * Add quotation marks for constraint name * update changelog * Revert "update changelog" This reverts commit 6e6e483. * update changelog * Update CHANGELOG.md Co-authored-by: Rikin Kachhia <[email protected]> Co-authored-by: rikinsk <[email protected]> * console: add multi select in browse rows to allow bulk delete (close hasura#1739) (hasura#3735) * console: disable renaming action relationships (hasura#4072) * console: add dropdown for enum fields in insert/edit row pages (close hasura#3748) (hasura#3810) * docs: replace doc with ref (close hasura#4054) (hasura#4068) * modified the logic of consuming scheduled events - made it similar to the logic being used in the event-triggers * fix the warnings in the EventTrigger and ScheduledTrigger files * use "scheduled events" in the logs instead of "events" * add documentation for the processScheduledQueue function * log the HTTP error in processEventQueue * revert the Scheduled Triggers logic to as it was earlier * console: track runtime errors (hasura#4083) * fork new threads using forkImmortal instead of forkIO for ST threads * remove the commented functions from Eventing/HTTP.hs * ci: fix ciignore script to ignore certain directories (hasura#4086) * docs: bump MarkupSafe version (hasura#4102) * refactor ExtraContext in Eventing back to ExtraLogContext * refactor the Eventing/HTTP file - move event trigger specific things to EventTrigger.hs - refactor mkInvo to mkInvocation * use bracket_ to do the async stuff in event triggers * server: Fix buggy rewrite rule for Rule We’re lucky that this never bit us. For the most part, these rules aren’t actually used; most code programs against ArrowCache and doesn’t get specialized enough for these rules to fire. Even if we did have code that could trigger this rule, the situations where it would actually fire are slim. In order for the rule to typecheck at all, both sides of the pair being passed through the arrow must have exactly the same type. Of course, that would just make this even more hellish to debug. Rewrite rules are dangerous. * refactor the processEventQueue function * undo all the unrelated js file changes * remove the unused import in Eventing/HTTP.hs Co-authored-by: Praveen Durairaju <[email protected]> Co-authored-by: Aravind Shankar <[email protected]> Co-authored-by: Shahidh K Muhammed <[email protected]> Co-authored-by: Vamshi Surabhi <[email protected]> Co-authored-by: Shahidh K Muhammed <[email protected]> Co-authored-by: Rikin Kachhia <[email protected]> Co-authored-by: Rishichandra Wawhal <[email protected]> Co-authored-by: Marion Schleifer <[email protected]> Co-authored-by: Meera Sundar <[email protected]> Co-authored-by: Dmitry Minkovsky <[email protected]> Co-authored-by: nizar-m <[email protected]> Co-authored-by: Nizar Malangadan <[email protected]> Co-authored-by: Brandon Simmons <[email protected]> Co-authored-by: Tim Whitbeck <[email protected]> Co-authored-by: Tirumarai Selvan <[email protected]> Co-authored-by: Rakesh Emmadi <[email protected]> Co-authored-by: Aleksandra Sikora <[email protected]> Co-authored-by: rikinsk <[email protected]> Co-authored-by: Alexis King <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Usually
asyncwithlinkorwithAsyncis the right choice (since exceptions get raised in calling thread, for instance).forkIO . forevershould usually useimmortalor our own version of that (what if the thread just dies?)The text was updated successfully, but these errors were encountered: