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

[Tech spec] How should we batch async calls from the frontend? #8934

Closed
joeyyang opened this issue Jan 24, 2019 · 13 comments
Closed

[Tech spec] How should we batch async calls from the frontend? #8934

joeyyang opened this issue Jan 24, 2019 · 13 comments
Assignees

Comments

@joeyyang
Copy link
Contributor

joeyyang commented Jan 24, 2019

related ticket #8904

Description

image

The issue

In most <TaskTable>s, we do a couple calls per Task asynchronously in order to quickly load the Tasks but lazy load information that would come from VACOLS or other expensive sources.

However, Chrome (for instance) can only make 6 requests to the same host simultaneously. If there are 30 tasks, and 3 async calls per task (for the hearing, number of documents, and new files indicator), this process can take a really long time.

@lowellrex @mdbenjam @amprokop @tomas-nava, what's your opinion on how we should handle this in Rails? @lowellrex proposed considering websockets, and I don't have much of a clue how to batch these requests in Rails.

@mdbenjam
Copy link
Contributor

Can we break out what specific API requests we're talking about? Is there a way to do it on initial page load? The only things I don't think we can do on initial load are BGS calls, but I think we should eliminate those from the Queue page anyway because we don't want someone with 1000 cases triggering 1000 BGS calls.

Also note that @aroltsch and @hschallhorn have been doing a great job solving this problem for the "new document" and "document count" features. So they might have input!

@amprokop
Copy link
Contributor

Agreed with @mdbenjam, mind adding some detail about what the requests are? Would love to start from there. Thanks!

@joeyyang
Copy link
Contributor Author

hey, these are the 3 we have at the moment:

3 async calls per task (for the hearing, number of documents, and new files indicator)

@mdbenjam
Copy link
Contributor

So I think we'll be getting the number of documents and new files indicator on page load from now on @aroltsch or @hschallhorn to correct me.

I'd love to see if we could include the hearing in page load as well.

@joeyyang
Copy link
Contributor Author

joeyyang commented Jan 25, 2019 via email

@anyakhvost
Copy link

@hschallhorn worked on improving on how fast we get results for the number of documents and new documents. Now instead of querying VBMS on each call, we query our documents table. Our nightly reader job pre-downloads all documents for users so we don't have to query VBMS on every call. I think we could still benefit from batch processing though.

@mdbenjam
Copy link
Contributor

We definitely shouldn't run a bunch of serial VACOLS requests, but is there a way to load all the data in one VACOLS call? That should be fast.

@hschallhorn
Copy link
Contributor

Number of documents and new documents could also be combined into one call. We almost always use the two together and they both use the same query to the Documents table. That knocks out some requests to the server and also the db.

@lowellrex
Copy link
Contributor

I like this idea! I think it could really speed up the responsiveness of the queue table views.

When we load the queue table view we could batch all of the requests into one (or three) calls to the backend. Something like GET /appeals-document-count/{appeal_id_1},{appeal_id_2},{appeal_id_3} (with similar calls for new documents and hearings). We could retain the existing behaviour of the individual document count requests in the AppealDocumentCount component so that we could load document counts for individual appeals when they pop onto the queue outside of the initial load (I don't think we currently expect it to happen, but it might in the future). And in order to prevent doing double work (batch load and individual appeal doc count load), we could check if a request for the batch load is in flight before making the individual appeal doc count request (alongside where we currently check to see if we already have the document count in the store).

@anyakhvost
Copy link

I like the idea of calling a document count endpoint for all appeals at once. Just like @lowellrex showed above. The same goes for new documents and hearings. Is this something @hschallhorn or @joeyyang would be interested to work on maybe next sprint? I've also noticed we make calls to VACOLS for legacy appeals for documents endpoints, we should get rid of it because these endpoints don't need any VACOLS data.

@youngfreezyVA
Copy link
Contributor

youngfreezyVA commented Apr 5, 2019

have we already explored implementing infinite scrolling/lazy loading for this table, which would require pagination on the backend? From what I understand this is specific to individual rows, and not the entire table

@lpciferri
Copy link

lpciferri commented Jul 12, 2019

@lowellrex - wondering if we can close this considering all the recent refactoring echo has done?

@lowellrex
Copy link
Contributor

I think this tech spec extends beyond the pagination work. This tech spec is about the asynchronous calls that the front-end makes after the queue table view has loaded: Hearing badges, new documents icons, and document counts.

I am happy to close this though since I don't think there is any active effort on this at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants