diff --git a/Daily Notes/2024/10/2024-10-22 (Tuesday).md b/Daily Notes/2024/10/2024-10-22 (Tuesday).md index b4736d7..37ec308 100644 --- a/Daily Notes/2024/10/2024-10-22 (Tuesday).md +++ b/Daily Notes/2024/10/2024-10-22 (Tuesday).md @@ -7,13 +7,13 @@ A few learnings from launch morning: 2. Caching was not carefully implemented (ie, not implemented at all in obvious places) 3. We have a ridiculous web of interconnected microservices. Discounts, orders, products, subscriptions (and more)--none of them are stand-alone services. They all depend on each others, it seems. I believe there has not be careful planning around the boundaries of these APIs and, unless replatforming supplants them completely, we need some major overhaul or at least strongly-worded guided on incremental improvements as we maintain these services. See [this thread](https://faithlife.slack.com/archives/C03R24VCERG/p1729552403256199) as an example where a bug arose because Orders API was not calculating tax in preview routes the same way Subscriptions API was--absolutely inexcusable (to be really dramatic) that two services have to calculate tax _at all_ (instead of a single one doing so for use cases) or _at the very least_ don't use a shared codebase for doing so. 4. I see a couple of potential causes for this sloppiness: - 1. Some of our most experienced senior engineers (who also had the most senior insight into what Commerce and Payments was doing) leaving shortly before launch (Nate Merritt and Timothy Baumgartner). This left us not only short-handed, but also not knowing what we didn't know in some cases. I can't point to specific problems that I could draw a direct from to them leaving, but it feels like some problems may have simply been caught and never appeared as an issue long before launch. + 1. Some of our most experienced senior engineers (who also had the most senior insight into what Commerce and Payments was doing) leaving shortly before launch ([[Nate Merritt]] and Timothy Baumgartner). This left us not only short-handed, but also not knowing what we didn't know in some cases. I can't point to specific problems that I could draw a direct from to them leaving, but it feels like some problems may have simply been caught and never appeared as an issue long before launch. 2. Time: teams were working up to the last minute on things. We had little time to optimize, refactor and improve code. 3. Legacy design decisions: my impression (not quite a fully educated one) is that we regret a lot of design choices around Subscriptions API (and subscribable products specifically). This has a lot to do with the "web of interconnected services" I mentioned above. 5. Flaky builds were a major issue 6. Slow builds were a major issue 7. No pre-disaster drills was an issue--we couldn't figure out how to deploy from our local machines when we suddenly had to -8. Having Michael Sterling and Bradley jump in and help was vital. Bradley handled the haproxy problems and Michael did a ton of optimization research and implementation +8. Having [[Michael Sterling]] and Bradley jump in and help was vital. Bradley handled the haproxy problems and Michael did a ton of optimization research and implementation Feedback from [[Bradley Grainger]] ``` @@ -28,7 +28,7 @@ I haven't really dug into the commerce code much, other than a few places I jump [10:49](https://faithlife.slack.com/archives/C07TCUA47K3/p1729612154792719) -Bradley Grainger +[[Bradley Grainger]] Overall, I think launch went well. We proactively scaled up all services we thought were problematic, and I think they were all provisioned at 2x what they needed to be. But better safe than sorry. However, we had a blind spot with HAProxy: no one noticed that frontend `maxconn` was only 20,000 which was nowhere near what we needed for the 30k+ people registered for the live event. We also (of course) had the stability issues with keepalived (which probably would have bitten us even if we had increased HAProxy capacity). So it was unfortunate that we _almost_ but not quite got everything scaled up to handle the load. I think we were ![:pinching_hand:](https://a.slack-edge.com/production-standard-emoji-assets/14.0/google-medium/1f90f.png) this close to delivering flawlessly on the day. @@ -75,7 +75,7 @@ This was the primary "failure" if I recall -- with a better "baseline" performan General feedback: - Wrangling the build was painful. - - Hugo had to disable the 30-minute flakey Cypress tests for Skeletor + - Hugo had to disable the 30-minute flakey Cypress tests for [[Skeletor]] - Suggested fix: [https://faithlife.slack.com/archives/C03R1MQSR51/p1727716783476109?thread_ts=1727711581.537559&cid=C03R1MQSR51](https://faithlife.slack.com/archives/C03R1MQSR51/p1727716783476109?thread_ts=1727711581.537559&cid=C03R1MQSR51) - Suggested target: no more than **5 minutes** on **all testing** after code is deployed to an environment - The CEM build spends 8 minutes building the code _for each group_, in order to determine which group has changes. @@ -97,7 +97,7 @@ General feedback: - But, there's a chance we've taken cache-avoidance too far and missed some easy wins. - There also seems to be a "just fetch the data where I need it" approach to a lot of this code, even to the point of not re-using data fetched in a calling method - We also weren't consistently using _the caching we already had_ -- so we ate the complexity for not the full benefit - - Case Study: We can't _possibly_ need to re-calculate tax as much as we do. But there's no caching whatsoever, not even at SalesTaxApi, and we re-call SalesTaxApi at every level of the Skeletor => OrdersApi => SubscriptionsApi chain. + - Case Study: We can't _possibly_ need to re-calculate tax as much as we do. But there's no caching whatsoever, not even at SalesTaxApi, and we re-call SalesTaxApi at every level of the [[Skeletor]] => OrdersApi => SubscriptionsApi chain. - "Crochet Code" API surfaces -- augmenting Bradley's point on microservices, because we've got one team owning both ends we've "stitched together" the API surface to a degree that makes reasoning about one end alone impossible - Example: the `freeTrialOverridePeriodUnitKind ` and `freeTrialOverridePeriodCount` parameters on SubscriptionsApi's `GetSubscribableProductAsync` - The caller passes up two parameters for the sole purpose of modifying SubscriptionsApi's response in a manner the _caller could have done itself_ even without those parameters @@ -114,12 +114,12 @@ General feedback: - General microservice hygiene: - Use `fields` more to prevent un-necessary data-fetching - Ensure data flows "one way" as much as possible - - "Flatten" the call tree (e.g. instead of Skeletor => OrdersApi => SubscriptionsApi => SalesTaxApi, have Skeletor call _each_ of its dependencies _itself_ for the info owned by that dependency then combine them afterwards) + - "Flatten" the call tree (e.g. instead of [[Skeletor]] => OrdersApi => SubscriptionsApi => SalesTaxApi, have [[Skeletor]] call _each_ of its dependencies _itself_ for the info owned by that dependency then combine them afterwards) - Reduces the need for "`fields`" as it means less potentially-superfluous data gets fetched from each level - - Eliminate duplicate data-fetching _considering the whole call tree_ starting at the Skeletor root request + - Eliminate duplicate data-fetching _considering the whole call tree_ starting at the [[Skeletor]] root request ``` -Suggestions from Michael Sterling for improving testing. +Suggestions from [[Michael Sterling]] for improving testing. ``` For generic flakiness, I'd suggest compiling a list of the flakey ones (by looking at spurious failures across successive runs). Each one has a _reason_ why it's flakey, which _can_ require some skill to divine; but usually it's solvable even with Cypress.For the general test length, I would aim for 5 minutes instead of 18 minutes.Looks like your "Run Cypress Tests" step doesn't put the result files into a GHA Artifact, from which you could download the results for more details on what precisely is slow: [https://git.faithlife.dev/Logos/Skeletor/actions/runs/1034750/job/2100271](https://git.faithlife.dev/Logos/Skeletor/actions/runs/1034750/job/2100271)From the [test results](https://git.faithlife.dev/Logos/Skeletor/runs/2100327#r2), seems like the culprits are: