Skip to content

HIGH: [Reliability] Stop doing a full report load + Auth call for any thread reports returned by GetChats #39225

Closed

Description

@quinthar

Problem:

My app times out attempting ReconnectApp after loading 6000+ reports in 3 min, as discussed here.

One observation is that that we are returning many "thread reports" and then calling Auth once per report to load it so we can build the report action object.

Solution:

  • Investigate what these reports are and why they are getting returned.
  • Move the logic that is loading the reports into Auth as explained in this comment so that we aren't calling Auth thousands of times in a row.
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d88324b92cfff3d0
  • Upwork Job ID: 1775305647007121408
  • Last Price Increase: 2024-04-02

Activity

  1. converted this from a draft issue on Mar 29, 2024
  2. melvin-bot commented on Mar 29, 2024

    @melvin-bot

    Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.

  3. marcaaron commented on Mar 29, 2024

    @marcaaron
    Contributor

    Ok I see this get called once: https://github.com/Expensify/Web-Expensify/blob/fae68e50dd8e05bd948d194a7bf27b2da781eab2/lib/ReportAPI.php#L6762

    And then a lot of report actions getting instantiated which trigger a full load of the report...

    https://github.com/Expensify/Web-Expensify/blob/1e7d277cc54f26e554a7e3ea57bfc7f224a7b0d6/lib/ReportAPI.php#L6775

    Explanation of that code:

    If one of the reports returned has a parentReportAction (a.k.a. it is a thread) then we call:

    ReportUtils::buildParentActionOnyxData()

    Which does a full report load right here: https://github.com/Expensify/Web-Expensify/blob/1e7d277cc54f26e554a7e3ea57bfc7f224a7b0d6/lib/ReportUtils.php#L3489

    Gonna add some logs because there are not many (and maybe I missed what the problem really is - but seems right)...

    And also think of some way to stop this....

  4. added
    InternalRequires API changes or must be handled by Expensify staff
    on Apr 2, 2024
  5. melvin-bot commented on Apr 2, 2024

    @melvin-bot
  6. melvin-bot commented on Apr 2, 2024

    @melvin-bot

    Triggered auto assignment to Contributor Plus for review of internal employee PR - @DylanDylann (Internal)

  7. changed the title [-]HIGH: [Reliability] Fix dbarrett's 3-min `ReconnectApp` timeout[/-] [+]HIGH: [Reliability] Stop doing a full report load + Auth call for any thread reports returned by `GetChats`[/+] on Apr 2, 2024
  8. marcaaron commented on Apr 3, 2024

    @marcaaron
    Contributor

    Left a P/S here on how to get started on this.

    And also thinking that even with those changes (which would improve the performance) - we probably still need pagination since way too many reports are getting returned.

  9. marcaaron commented on Apr 18, 2024

    @marcaaron
    Contributor

    Focusing on AddComment 1:1:1 changes this week. Will give this a look next week.

  10. marcaaron commented on Apr 25, 2024

    @marcaaron
    Contributor

    Chatted with @quinthar and we're gonna close this one for now. Probably this improvement can be rolled into whatever issue we create for making OpenApp 1:1:1 (or maybe we can roll all of those improvements into pagination).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

EngineeringInternalRequires API changes or must be handled by Expensify staffTaskWeeklyKSv2

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions