add overpass-api role #3

RoPP · 2017-06-10T13:33:12Z

Ne pas merger !

Il manque encore quelques morceaux…

jocelynj · 2017-06-10T16:10:15Z

roles/overpass-api/tasks/compile.yml

+    validate_certs: false
+    remote_src: true
+  become: yes
+  become_user: "{{ user }}"


I'm fine with downloading a tar.gz, but would it be a better idea to clone the git repository ?

Better use the released versions at dev.overpass-api.de/releases. Releases on github are sometimes not updated with the latest changes!

Actually, I plan to use released version of overpass, at least at beginning.

@mmd-osm is there any difference between github and dev.o.d versions ?

Yes, at this time, the github release lacks a few fixes which have been published on dev.o.d. I guess Roland just forgot to update github as well.

Oh ok. I'll use dev.o.d then.

jocelynj · 2017-06-10T16:12:46Z

roles/overpass-api/tasks/compile.yml

+  args:
+    chdir: "{{ project_dir }}/Overpass-API-osm3s-{{ overpass_release }}/src"
+  become: yes
+  become_user: "{{ user }}"


Be careful that this shell command means that ansible will execute these programs each time the ansible script is executed. Maybe a check like creates: 'bin file' could be added ?

Good idea :)

jocelynj · 2017-06-10T16:15:29Z

roles/overpass-api/tasks/compile.yml

+    - zlib1g-dev
+    - bzip2
+    - wget
+    - liblz4-dev


I don't really like to remove packages - it could cause issue if they are needed for something else. For exemple, bzip2, wget, git and make are useful independently of this ansible script.

I removed packages by force of habit. I'm more used to create docker container which should be as small as possible.

jocelynj · 2017-06-10T16:17:31Z

roles/overpass-api/tasks/set_up_database.yml

+- name: import planet
+  shell: osmconvert {{ database_dir }}/planet-latest.osm.pbf --out-osm | update_database --db-dir={{ database_dir }} --meta
+  become: true
+  become_user: "{{ user }}"


Good idea to import the planet, but it shouldn't be re-imported if it was already imported.
It might also a bit too long for an ansible script - I'm not sure if ansible/ssh is robust enough to execute commands that can last several hours.

You might want use the cloning feature and download the current database from dev.overpass-API.de/clone instead. Please note that this would imply gzip compression at this time.

Yes, it was a quick and dirty test. import is now execute at first boot only.

jocelynj · 2017-08-14T12:55:26Z

What is the current status of this branch ?

Is is mergeable ? Should I review it one more time ?

RoPP · 2017-08-14T15:23:33Z

Every review is alway welcome! Anyway this PR can be merged, it works well. There probably some remaining small bugs but we'll see…

jocelynj · 2017-08-16T12:19:54Z

hosts

@@ -193,6 +193,10 @@ osm26.openstreetmap.fr
 osm27.openstreetmap.fr
 osm28.openstreetmap.fr

+[overpass-api]
+osm999.openstreetmap.fr overpass_version=skip


Is osm999 a custom machine ?
I suppose that I shouldn't merge it ?

Yes, it was a local vm for testing

jocelynj · 2017-08-16T12:23:17Z

roles/overpass-api/defaults/main.yml

@@ -0,0 +1,7 @@
+---
+user: overpass


Should it be overpass_user instead of user ?
I'm not sure if it is possible to set a parameter for a host and a single role with ansible.

Yes, overpass_user is more consistent with other parameters.

jocelynj · 2017-08-16T12:26:31Z

roles/overpass-api/tasks/compile.yml

+    chdir: "{{ project_dir }}/osm-3s_{{ overpass_version }}"
+
+- name: install overpass-api cgi-bin
+  command: cp -r {{ project_dir }}/osm-3s_{{ overpass_version }}/cgi-bin/ {{ overpass_webroot_dir }}/


It could be done with ansible copy module with remote_src=yes, but it is fine as it is.

Unfortunatly, "Currently remote_src does not support recursive copying."

Fine, let's keep the current code as it is :)

jocelynj · 2017-08-16T12:28:02Z

roles/overpass-api/tasks/main.yml

@@ -0,0 +1,67 @@
+- include: ../../../shared/project-account.yml


It might be clearer with user=overpass or user="{{ overpass_user}}" ?

Unfortunatly, we cannot do variables substitution with include directive.

But we can use set_fact

Hum, I'm using this in roles/taginfo/tasks/main.yml:

- include: ../../../shared/project-account.yml user=taginfo

Oh, you're right. I probably read doc too quickly :) Thx for the tip.

jocelynj · 2017-08-16T12:29:58Z

roles/overpass-api/templates/index.html

+  <meta http-equiv="X-UA-Compatible" content="ie=edge">
+  <title>api.openstreetmap.fr</title>
+  <style>
+    @import url(https://fonts.googleapis.com/css?family=Ubuntu+Condensed);


I would prefer not to use font from google, but we can keep this code as it is.

I pick this font becausi it seems to be one used on http://openstreetmap.fr/ but it's not really important.

mmd-osm · 2017-08-17T05:23:46Z

roles/overpass-api/templates/default.conf

+  rewrite ^/overpass(.*)$ /cgi-bin/interpreter last;
+
+  location /cgi-bin/ {
+    gzip off;


Mobile apps would be happy with compression to reduce data volume

Yes, desktop apps as well :)

mmd-osm · 2017-08-17T05:25:19Z

roles/overpass-api/templates/overpass-areas-builder.service

+ExecStart=/usr/local/bin/rules_loop.sh {{ overpass_database_dir }}
+Nice=19
+Restart=always
+RestartSec=30s


This job will run for several hours (delta mode is unavailable due to lack of attic data). Maybe have a longer wait time in case job causes too much load.

RestartSec is used when the service failled, in this case, when rules_loop.sh crash.
Actually, in this case it will never be hit as rules_loop.sh is mainly a while true loop.

Ah, yes, you're right. Somehow got sidetracked by the name "restart". So, the same applies to the script itself then, if wait time is too low in there.

Signed-off-by: Rodolphe Pelloux-Prayer <rodolphe@damsy.net>

jocelynj · 2017-08-17T20:10:38Z

@RoPP : Can you confirm that I can merge it now ? (I'm not sure if all remarks by @mmd-osm were resolved)

I did some simple tests to dev.api.openstreetmap.fr from http://overpass-turbo.eu/ and it seems to work correctly. Do you think that we can now install a whole planet instead of France only ?

RoPP · 2017-08-17T20:22:24Z

@jocelynj Yes, we can merge it and setup new VM for whole planet.

add overpass-api role

jocelynj · 2017-08-18T15:16:26Z

Thanks, I've merged to master.

mmd-osm · 2017-08-19T17:29:22Z

We have a few cgi-bin scripts, like convert, kill_my_queries, timestamp both as executable and shell script. Do you plan to also include them?

mmd-osm · 2017-08-29T20:51:37Z

roles/overpass-api/templates/overpass-areas-dispatcher.service

+[Service]
+Type=simple
+User={{ overpass_user }}
+ExecStartPre=-/bin/rm {{ overpass_database_dir }}/osm3s_{{ overpass_version }}_areas


Can we remove those rm ... ExecStartPre? See http://wiki.openstreetmap.org/w/index.php?title=Overpass_API/Installation&diff=1502925&oldid=1459308&rcid=&curid=72215:

Do not automatically remove any of the lock files, socket files or shared memory files. They work as canaries, i.e. hitting existing files is almost always an indicator for bigger trouble elsewhere. Please ask back in those cases.

(this affects all dispatcher scripts)

Make sense. These PreExecStart statements are from wiki but I didn't check if they're usefull or needed. Thanks for clarification.

Those wiki scripts were added by third party users, have never been reviewed and caused serious issues down the road (segmentation fault). Roland puts extra scrutiny on those now, making sure they're ok.

RoPP · 2017-08-30T06:32:27Z

Btw, all scripts in cgi-bin (20!) are now available but I'm not very confortable with that. Are all these scripts useful?

mmd-osm · 2017-08-30T06:39:23Z

Good point, I reviewed those put up the following list:

Optional

xapi / xapi_meta require /tmp/translate_xapi/ directory in version 0.7.54 for temporary results. XAPI is very much outdated these days, so could skip that
draw_line, sketch-* - see http://wiki.openstreetmap.org/wiki/Overpass_API#Public_transport_example. Something I would remove, but I leave it as optional.
ping - used to be a CPU hog on overpass-api.de, as every instance in the world would query the central instance, rather than their own instance. Keep a close eye on it. block it, it called to often

Remove

augmented_* doesn't make sense since you don't have an attic database
template - don't know what it's good for -> remove
trigger_clone - you don't want to offer cloning feature -> remove

Unclear

sitemap

jocelynj reviewed Jun 10, 2017

View reviewed changes

RoPP force-pushed the wip_overpass_api branch from 116a8e1 to 2723304 Compare June 18, 2017 19:08

RoPP force-pushed the wip_overpass_api branch 3 times, most recently from 3341df7 to a10a70e Compare June 30, 2017 20:04

RoPP added 2 commits July 16, 2017 15:34

chore: set 4 space indentation for py file

d5f73b7

feat: create /ssd/{{ user }} dir if /ssd exists

b24b321

RoPP force-pushed the wip_overpass_api branch 3 times, most recently from 7de12fe to 360f78c Compare July 21, 2017 12:24

RoPP changed the title ~~WIP: add overpass-api role~~ add overpass-api role Aug 14, 2017

jocelynj reviewed Aug 16, 2017

View reviewed changes

mmd-osm reviewed Aug 17, 2017

View reviewed changes

RoPP force-pushed the wip_overpass_api branch from 360f78c to 025617c Compare August 17, 2017 19:21

feat: add overpass-api role

492d59b

Signed-off-by: Rodolphe Pelloux-Prayer <rodolphe@damsy.net>

RoPP force-pushed the wip_overpass_api branch from 025617c to 492d59b Compare August 17, 2017 19:30

mmd-osm mentioned this pull request Aug 17, 2017

Remove French instance from server list? tyrasd/overpass-turbo#333

Closed

jocelynj merged commit dd99b69 into osm-fr:master Aug 18, 2017

jocelynj added a commit that referenced this pull request Aug 18, 2017

Merge pull request #3 from RoPP/overpass_api

f71149a

add overpass-api role

mmd-osm reviewed Aug 29, 2017

View reviewed changes

RoPP deleted the wip_overpass_api branch September 21, 2017 07:19

		@@ -0,0 +1,67 @@
		- include: ../../../shared/project-account.yml

add overpass-api role #3

add overpass-api role #3

Conversation

RoPP commented Jun 10, 2017

Choose a reason for hiding this comment

mmd-osm Jun 18, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jocelynj Jun 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmd-osm Jun 18, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jocelynj commented Aug 14, 2017

RoPP commented Aug 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoPP Aug 17, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmd-osm Aug 17, 2017 • edited

Choose a reason for hiding this comment

jocelynj commented Aug 17, 2017

RoPP commented Aug 17, 2017

jocelynj commented Aug 18, 2017

mmd-osm commented Aug 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoPP commented Aug 30, 2017

mmd-osm commented Aug 30, 2017 • edited

Recommended

Optional

Remove

Unclear

mmd-osm Jun 18, 2017 •

edited

jocelynj Jun 10, 2017 •

edited

mmd-osm Jun 18, 2017 •

edited

RoPP Aug 17, 2017 •

edited

mmd-osm Aug 17, 2017 •

edited

mmd-osm commented Aug 30, 2017 •

edited