mediagis / nominatim-docker

100% working container for Nominatim
Creative Commons Zero v1.0 Universal
1.08k stars 442 forks source link

Continous replication, shutdown fix, output apache logs #373

Closed pgassmann closed 1 year ago

pgassmann commented 2 years ago

start continuous replication process in start.sh to keep nominatim up-to-date if REPLICATION_URL is set and FREEZE is not set to true

I added UPDATE_MODE to configure how to run replication

also fix the issue that postgres and apache are not stopped properly when the docker container is stopped.

when command is set as string, docker will launch the script in /bin/sh and the actual script will run in a subshell and not receive the signals. with ["/app/start.sh"] the script runs directly as PID 1 and properly executes the shutdown trap

also forward apache logs to docker

also warm cache after each startup

pgassmann commented 2 years ago

Fixes #284

leonardehrenfried commented 2 years ago

Hi, thanks for the contribution. I think the graceful shutdown is great. I'm travelling without a computer so can't actually test it, but perhaps @philipkozeny can.

However, I think we ought to discuss automatically starting the replication. I'm leaning towards agreeing with you but since we don't do it at the moment we ought to think about it a little more. My gut feeling is that there isn't a real reason why we don't - we just never implemented it.

Does it have any effects on how we test the image, @pgassmann?

pgassmann commented 2 years ago

@leonardehrenfried the test workflow runs nominatim replication --once while the continous replication is already running in the container. This creates a conflict when its trying to import the data.

Perhaps it would make sense to add a REPLICATION_UPDATE=once/continous/never configuration option.

pgassmann commented 2 years ago

I added UPDATE_MODE to configure how to run replication

leonardehrenfried commented 2 years ago

Btw, lots of people want to know what kind of hardware you need for a planet import. If you'd like you can share your experiences and specs in this thread: https://github.com/mediagis/nominatim-docker/discussions/265

pgassmann commented 2 years ago

@philipkozeny @leonardehrenfried What's missing to merge this?

leonardehrenfried commented 2 years ago

I would really like @philipkozeny to have a final look as I did the review on a phone.

pgassmann commented 1 year ago

Can you rewrite our test case to check if replication works?

How could we check that? Currently you only check that the update command runs and it still responds.

That still works the same if you don't set UPDATE_MODE.

To test UPDATE_MODE we could check the /var/log/replication.log in the container if it contains "Update completed. " after some time.

I updated the ci to check the update mode and increased sleep to be on the safe side that the import is finished. from the logs it looked very close or not even fully finished.

pgassmann commented 1 year ago

the UPDATE_MODE checks should fail now for 4.0. Do you want to skip the checks if version < 4.1 or should we also update the scripts for 4.0?

philipkozeny commented 1 year ago

the UPDATE_MODE checks should fail now for 4.0. Do you want to skip the checks if version < 4.1 or should we also update the scripts for 4.0?

we can skip them for 4.0

pgassmann commented 1 year ago

@philipkozeny I updated the steps to use different volumes/bindmounts to start with fresh imports. The UPDATE_MODE tests should now be disabled for 4.0. I hope the syntax is right and the jobs run this time.

philipkozeny commented 1 year ago

still failing with a syntax error: https://github.com/mediagis/nominatim-docker/actions/runs/3082389729

leonardehrenfried commented 1 year ago

I think you need to use single quotes for the version number.

pgassmann commented 1 year ago

10 days ago data was no longer available. changed back to 4 days ago.

pgassmann commented 1 year ago

ready to merge and update the image on dockerhub?

philipkozeny commented 1 year ago

Thanks again @pgassmann for the great PR.

@all-contributors please add @pgassmann for doc, code, test.

allcontributors[bot] commented 1 year ago

@philipkozeny

I've put up a pull request to add @pgassmann! :tada:

leonardehrenfried commented 1 year ago

Thanks for the contribution. That was a nice CH-AT-DE cooperation!