Skip to content

refactor(DetailedStopGroup): replace async_stream#3310

Open
thecristen wants to merge 1 commit into
mainfrom
cbj/stop-controller-timeout
Open

refactor(DetailedStopGroup): replace async_stream#3310
thecristen wants to merge 1 commit into
mainfrom
cbj/stop-controller-timeout

Conversation

@thecristen

Copy link
Copy Markdown
Collaborator

Scope

MBTA-DOTCOM-JV

In the continued vein of, we unearth old but recurring Sentry issues from the archive...

Uncaught exit - {:timeout, {Task.Supervised, :stream, [5000]}}
    lib/task/supervised.ex:349: Elixir.Task.Supervised.Task.Supervised.stream_reduce/7
    lib/enum.ex:4570: Elixir.Enum.Enum.flat_map/2
    lib/dotcom_web/controllers/stop_controller.ex:269: Elixir.DotcomWeb.StopController.DotcomWeb.StopController.get_stop_info/0
    lib/dotcom_web/controllers/stop_controller.ex:44: Elixir.DotcomWeb.StopController.DotcomWeb.StopController.show/2
    lib/dotcom_web/controllers/stop_controller.ex:1: Elixir.DotcomWeb.StopController.DotcomWeb.StopController.action/2
    lib/dotcom_web/controllers/stop_controller.ex:1: Elixir.DotcomWeb.StopController.DotcomWeb.StopController.phoenix_controller_pipeline/2
    lib/phoenix/router.ex:416: Elixir.Phoenix.Router.Phoenix.Router.__call__/5
    deps/plug/lib/plug/error_handler.ex:80: Elixir.DotcomWeb.Router.DotcomWeb.Router.call/2

Implementation

This message appears when Task.async_stream doesn't specify an on_timeout action - the default behavior is the task exits!

In this case I think it's better if we don't create possible timeouts: just get everything anyway without async calls, since we'll always want this page (/stops) to list all the stops.

I think the code had likely been structured this way because fetching and processing all the stops can be kinda slow. So this updated code leverages Stream over Enum in a few spots for some hopeful performance improvements. The flame charts are still massive here, but I did see this page go from producing an SVG of 5.2MB to one that's 3.4MB...

Before After
flame_on (detailedstop before) flame_on (detailedstop after)

Don't create possible timeouts, just get everything anyway
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.

1 participant