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

Visiting local homepage too soon prevents classic books carousel from rendering #5473

Open
jimchamp opened this issue Jul 29, 2021 · 6 comments
Labels
Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] Module: Docker Issues related to the configuration or use of Docker. [managed] Priority: 3 Issues that we can consider at our leisure. [managed] Theme: Development Issues related to the developer experience and the dev environment. [managed] Type: Bug Something isn't working. [managed]

Comments

@jimchamp
Copy link
Collaborator

Our database population scripts may not be running properly after Docker images are built. Books are not properly loaded and accessible on local Open Library instances until Docker is restarted at least once (and sometimes more than once).

Evidence / Screenshot (if possible)

Relevant url?

Steps to Reproduce

  1. Build the Docker images.
  2. Run docker-compose up
  3. Watch for any errors in the db_1 logs
  4. Navigate to localhost:8080. The classic books carousel should be missing if the data wasn't properly loaded.
  • Actual: The local instance contains no books
  • Expected: The local instance is populated with a small number of books

Details

  • Logged in (Y/N)? N/A
  • Browser type/version? N/A
  • Operating system? N/A
  • Environment (prod/dev/local)? Local

Proposal & Constraints

Unfortunately, I currently don't have much information about this. The Docker build process is a little time consuming, so this will take awhile to troubleshoot. Here are some things that would be good to check while troubleshooting this:

  1. Capture any error messages emitted by all of the containers, particularly db_1
  2. After initially starting the app, open a shell for db_1 and see if the tables exist and are populated. If they are, the db population scripts may be running too late in the start up sequence.
  3. This seems like a relatively new issue, so checking recent changes to Docker files may be informative.

Related files

Stakeholders

@RayBB
@cdrini

@jimchamp jimchamp added Type: Bug Something isn't working. [managed] Module: Docker Issues related to the configuration or use of Docker. [managed] Priority: 2 Important, as time permits. [managed] Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] labels Jul 29, 2021
@mekarpeles mekarpeles added Priority: 1 Do this week, receiving emails, time sensitive, . [managed] and removed Priority: 2 Important, as time permits. [managed] labels Aug 2, 2021
@mekarpeles
Copy link
Member

@jimman2003 have you noticed this and do we possibly know of a good solution? Would love your feedback

@jimman2003
Copy link
Contributor

No...unfortunatly not

@jimman2003
Copy link
Contributor

jimman2003 commented Aug 2, 2021

In the past, it helped me to remove the solr volumes and rebuild the images... maybe try this?

@jimchamp
Copy link
Collaborator Author

jimchamp commented Aug 5, 2021

It seems that the database is being properly populated when a fresh Docker build is first started. The classic books carousel will not appear on the homepage in development environments if that page is opened too early after starting Docker. Since this page is cached, the carousel will still not appear after the page is refreshed.

I've found that the carousel will be present if I wait until home_1 has exited successfully (this occurs when openlibrary_home_1 exited with code 0 is displayed in the logs).

@RayBB suggested creating a dependency in the Docker startup that waits for home_1 to exit before saying that localhost is ready.

Renaming this issue to better reflect what is actually happening, and lowering the priority as this is no longer blocking.

@jimchamp jimchamp added Priority: 2 Important, as time permits. [managed] and removed Priority: 1 Do this week, receiving emails, time sensitive, . [managed] labels Aug 5, 2021
@jimchamp jimchamp changed the title Database not populating when initially starting the application after a Docker build Visiting local homepage too soon prevents classic books carousel from rendering Aug 5, 2021
@RayBB
Copy link
Collaborator

RayBB commented Sep 18, 2021

Since this came up again I did some research.
This page has info related to what we want to do: https://docs.docker.com/compose/startup-order/

In short, we'll probably need to have a bash scrip that waits until home_1 exits and then starts the service on port 8080.

Seems doable. I may look into it more tomorrow

@jimchamp jimchamp added the Theme: Development Issues related to the developer experience and the dev environment. [managed] label Sep 18, 2021
@RayBB
Copy link
Collaborator

RayBB commented Sep 18, 2021

Upon closer examination the issue is a bit more complicated than expected.
The ideal situation would be that web isn't available to the user until home exits.
However, it looks like home depends on web to be up since it calls it several times.

On my computer it takes about 30 seconds for update_work.py to run for works (which is hat home runs).
Within update_work.py update_keys takes almost the whole time. And within that these are the lines that take the most time:

for k in wkeys:
            w = data_provider.get_document(wkey) # takes ~200ms
            requests += update_work(w) # takes ~500ms

^^ the above lines run like a hundred times or so.

It would seem one way to help with this issue is just speed up this reindexing.

For me right now it takes 54 seconds . I figured that out by changind ol-home-start.sh to this:

start=$SECONDS
make reindex-solr
duration=$(( SECONDS - start ))
echo "reindex-solr took $duration seconds"

So one way to improve that pops up right away is to let the two commands in reindex-solr run at the same time. However, this proved more difficult than expected due to xargs and the fact that jobs is not installed by default. So maybe we can come back to this later.

So instead what I tried was using joblibs to spin up separate threads for the iterations of wkeys
Here are my results:

Threads Seconds
Disabled 54
1 55
2 43
4 28
8 25
16 24
32 24
64 23
128 25

However, I noticed what seems to be a good chunk of startup time between the two scripts that run in the makefile.
So I tried merging them into one with a simple change from grep '^/books/' to grep -e '^/books/' -e '^/authors/' - this searches for both patterns.

This seemed to cut the time down to ~20 seconds with 16 threads.

In this case the parallel part takes about 5 seconds.
Everything above the parallel part in update_keys takes about 5 seconds.
Everything after parallel part in update_keys takes about 2.5 seconds.

The whole FnToCLI(main).run() takes about 13 seconds - so not much else is impacting.
That tell us the rest of the 7 seconds are from make, talking to db, and the python script initializing.

These are my findings so far. I think this alone may be enough of an improvement that most users won't notice this issue anymore. I will open a PR later but for now here is my branch.

Edit: wow okay on gitpod the times are even more drastic like less than 10 seconds for the reindex.

@mekarpeles mekarpeles added Priority: 3 Issues that we can consider at our leisure. [managed] and removed Priority: 2 Important, as time permits. [managed] labels Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] Module: Docker Issues related to the configuration or use of Docker. [managed] Priority: 3 Issues that we can consider at our leisure. [managed] Theme: Development Issues related to the developer experience and the dev environment. [managed] Type: Bug Something isn't working. [managed]
Projects
None yet
Development

No branches or pull requests

4 participants