Skip to content

Only consider it a block waiver if all future stops are SKIPPED#390

Merged
lemald merged 8 commits into
masterfrom
lem-only-cancel-when-all-stus-skipped
Oct 21, 2025
Merged

Only consider it a block waiver if all future stops are SKIPPED#390
lemald merged 8 commits into
masterfrom
lem-only-cancel-when-all-stus-skipped

Conversation

@lemald

@lemald lemald commented Oct 17, 2025

Copy link
Copy Markdown
Member

Summary of changes

Asana Ticket: 🧠 Trips are only cancelled when all of their StopTimes are cancelled

This is presently running on dev-blue.

On prod, we see the following when trying to plan a trip for later in the day on the 71:
Screenshot 2025-10-17 at 11 24 45

However, on dev-blue, we get the correct trip plan:
Screenshot 2025-10-17 at 11 25 40

Meanwhile, genuine cases of block waivers continue to receive cancellations:
Screenshot 2025-10-17 at 11 22 09

@lemald
lemald requested review from a team and rudiejd and removed request for a team October 17, 2025 15:31

@paulswartz paulswartz 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.

Comment thread lib/concentrate/group_filter/cancelled_trip.ex Outdated
@lemald

lemald commented Oct 17, 2025

Copy link
Copy Markdown
Member Author

I didn't dig into it too much, but it does look like this is adding about 500ms processing latency: https://mbta.splunkcloud.com/en-US/app/search/search?earliest=%40d&latest=now&q=search%20index%3Dconcentrate-dev*%20update_latency_ms%7C%20eval%20update_latency%20%3D%20(update_latency_ms%20%2F%201000)%20%7C%20timechart%20median(update_latency)%20by%20index&display.page.search.mode=fast&dispatch.sample_ratio=1&display.general.type=visualizations&workload_pool=&display.page.search.tab=visualizations&sid=1760723853.21367

Interesting, thanks for pointing that out. I tried to make it so that this code path runs only if absolutely necessary to keep processing time down but it seems like that isn't enough. I'll deploy your MapSet suggestion and see if that helps at all.

@lemald

lemald commented Oct 17, 2025

Copy link
Copy Markdown
Member Author

Okay, the MapSet thing didn't have any noticeable improvement to performance so I've tried some fiddling around with the ETS tables so that we can get all of the information we need with one query rather than querying the table in a loop. We'll see if that's any better.

@lemald

lemald commented Oct 17, 2025

Copy link
Copy Markdown
Member Author

Update: processing time is down, but confusingly it's down across al dev environments and starting before I deployed the change. I'll let it run longer so that we can see if any trends emerge.

@lemald
lemald force-pushed the lem-only-cancel-when-all-stus-skipped branch from 88c4a32 to caf6bf2 Compare October 20, 2025 19:41
@lemald
lemald merged commit a196354 into master Oct 21, 2025
15 checks passed
@lemald
lemald deleted the lem-only-cancel-when-all-stus-skipped branch October 21, 2025 13:14
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.

3 participants