Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Displays with a large number of plots take a long time to load due to annotation requests #6708

Closed
2 of 7 tasks
akhenry opened this issue Jun 2, 2023 · 14 comments · Fixed by #6719
Closed
2 of 7 tasks
Assignees
Labels
bug:regression It used to work. Now it doesn't :( performance impacts or improves performance severity:critical type:bug
Milestone

Comments

@akhenry
Copy link
Contributor

akhenry commented Jun 2, 2023

Summary

A display with a large number of plotted telemetry points can result in a large number of requests being issued for annotations. This can cause delays in loading those displays when there is request contention in the browser or server. I think that the main problem is the number of requests, so if they could be debounced and batched I think that might be a reasonable solution.

Expected vs Current Behavior

Displays with a large number of plots should load quickly, especially when there are no annotations to retrieve.

Steps to Reproduce

  1. Configure Open MCT to run with Couch DB as the persistence store
  2. Create a display layout with 10 plots of 30 telemetry points each. I suspect the total number of telemetry points is more important than the way they are distributed between the plots
  3. Open the network tab
  4. Navigate away from, and back to the display
  5. Note the large number of requests to the _find endpoint searching for annotations (one request per telemetry point).

Note that in the production environment many of these requests are also taking a long time to return, probably because of the large number of documents in the production database.

The response for the long pending requests includes the following message:
No matching index found, create an index to optimize query time.\nThe number of documents examined is high in proportion to the number of results returned. Consider adding a more specific index to improve this

This implies that there are two ways of approaching this issue, and we could potentially do both:

  1. Debounce and batch the requests, building an "$or" request for all of the telemetry points. Debouncing would also have the effect of allowing the display to load before any annotation requests are issued.
  2. Create an index to improve the performance of the query itself.

Note additionally that pending requests for annotations are not cancelled when navigating away. This can completely block navigation for a long time.

Environment

  • Open MCT Version: 2.2.0 (but almost certainly present in newer versions as well)
  • Deployment Type: MMOC Production

Impact Check List

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?
  • Does this block the execution of e2e tests?
  • Does this have an impact on Performance?

Additional Information

Video available upon request...

@scottbell
Copy link
Contributor

@akhenry Fiddling around with CouchDB indices/views to improve performance, and also looking at batching requests and have a few questions:

  1. It looks like slow annotation lookup aren't necessarily the issue (from what I can tell). I introduced a 5s delay into the code for loadAnnotations() in MctPlot:
async loadAnnotations() {
      await new Promise((resolve) => setTimeout(resolve, 5000));
      if (!this.openmct.annotation.getAvailableTags().length) {
        // don't bother loading annotations if there are no tags
        return;
      }

      const rawAnnotationsForPlot = [];
      await Promise.all(
        this.seriesModels.map(async (seriesModel) => {
          const seriesAnnotations = await this.openmct.annotation.getAnnotations(
            seriesModel.model.identifier
          );
          rawAnnotationsForPlot.push(...seriesAnnotations);
        })
      );
      if (rawAnnotationsForPlot) {
        this.annotatedPoints = this.findAnnotationPoints(rawAnnotationsForPlot);
      }
    },

and though the annotations take a long time to load, it didn't appear to slow the plot load:

Screen.Recording.2023-06-06.at.3.25.43.PM.mov

We're not awaiting the annotation load, though maybe the blocking networking calls are prohibiting other parts of OpenMCT from functioning properly?

  1. If we do want to batch annotation requests up, the abort signal gets a little tricky. Perhaps the last abort signal sent wins?

@scottbell
Copy link
Contributor

scottbell commented Jun 8, 2023

I've got an initial implementation, note the single _find for 10 separate plots:

Screen.Recording.2023-06-08.at.4.58.15.PM.mov

@akhenry is something we'll want to test on an integration server before a PR?

@scottbell scottbell linked a pull request Jun 12, 2023 that will close this issue
15 tasks
@scottbell
Copy link
Contributor

To test:

  1. Configure OpenMCT to use CouchDB
  2. Create a display layout
  3. Create a bunch of sine wave generators (5)
  4. Drop them all on the display layout
  5. Ensure they're all in plot mode
  6. Bring up your network tab, and reload the page.
  7. Ensure only one _find requests goes out to CouchDB (instead of 5)

@unlikelyzero unlikelyzero added performance impacts or improves performance unverified labels Jun 13, 2023
@unlikelyzero unlikelyzero added this to the Target:2.2.5 milestone Jun 15, 2023
@rukmini-bose
Copy link
Contributor

Verified Testathon 6/22/23. I only see one _find request in the Networks tab.

@khalidadil
Copy link
Contributor

Verified Fixed in Testathon on 06/22/23. I'm seeing a single _find request for annotations.

@michaelrogers
Copy link
Contributor

Fix verified during testathon 06/22/2023.

@shefalijoshi
Copy link
Contributor

Reopening this. I see this again in 3.0.0 on banner/testathon.

@scottbell
Copy link
Contributor

I'm not able to replicate this locally:

Screen.Recording.2023-07-26.at.10.07.55.AM.mov

I suspect the issue is the poor performance in general of plots with annotations captured in this ticket. If the plot is slow to load, either through network or CPU load, the annotation batches are 100ms apiece and will fire multiple times, which is expected behavior (at least for this fix). @shefalijoshi is this what's happening? If so, we can either increase the batch time (which I'm not sure we want), or fix the other performance issue and keep this issue closed.

@unlikelyzero unlikelyzero added the bug:regression It used to work. Now it doesn't :( label Aug 4, 2023
@unlikelyzero unlikelyzero removed this from the Target:3.0.0 milestone Aug 4, 2023
@akhenry akhenry added this to the Target:3.0.1 milestone Aug 7, 2023
@akhenry akhenry modified the milestones: Target:3.0.1, Target:3.1.0 Aug 24, 2023
@akhenry
Copy link
Contributor Author

akhenry commented Aug 24, 2023

Moving to 3.1.0. The immediate issue here is obviated by #6866.

@akhenry
Copy link
Contributor Author

akhenry commented Oct 2, 2023

Closing to re-test with Yamcs performance fix in place.

@jvigliotta
Copy link
Contributor

Not Fixed: Testathon 10/3
I'm still seeing multiple _find requests for annotations.

@ozyx ozyx reopened this Oct 3, 2023
@scottbell
Copy link
Contributor

Not Fixed: Testathon 10/3 I'm still seeing multiple _find requests for annotations.

@jvigliotta I'll take a look, but please note you'll see multiple _find requests if it takes more than 100ms to load the page (see here).

@scottbell
Copy link
Contributor

scottbell commented Oct 4, 2023

@jvigliotta @ozyx @akhenry Yeah, I think we're in the same position as have been in:

Screen.Recording.2023-10-04.at.4.54.38.PM.mov

Namely, you'll see more than one _find request if the page load takes more than 100ms. Note there are also CouchDB indexes and design documents in Open MCT now that we can use to further increase performance if it's the annotations that are taking a long time to load.

Do we want to keep this issue open?

@scottbell
Copy link
Contributor

Closing per @akhenry - fixed per original ticket.

@ozyx ozyx removed the unverified label Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:regression It used to work. Now it doesn't :( performance impacts or improves performance severity:critical type:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants