Merge ~ilasc/turnip:one-repack-task-at-a-time into turnip:master

Proposed by Ioana Lasc
Status: Merged
Approved by: Ioana Lasc
Approved revision: 85c51f08ad5c0bcb918f2b2b10f8659df773c624
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ilasc/turnip:one-repack-task-at-a-time
Merge into: turnip:master
Diff against target: 140 lines (+63/-7)
6 files modified
Makefile (+14/-2)
charm/turnip-celery/config.yaml (+1/-4)
charm/turnip-celery/lib/charms/turnip/celery.py (+8/-0)
charm/turnip-celery/templates/logrotate.j2 (+13/-0)
charm/turnip-celery/templates/turnip-celery-repack.service.j2 (+26/-0)
charm/turnip-celery/templates/turnip-celery.service.j2 (+1/-1)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Thiago F. Pappacena (community) Approve
Tom Wardill (community) Approve
Review via email: mp+402770@code.launchpad.net

Commit message

Run a dedicated repack celery worker

Description of the change

This proposes we run a dedicated repack worker and limit its concurrency to 1.
I've hardcoded the worker name (repack-worker), dedicated queue name (repacks), concurrency (1) and prefetch-multiplier (1) for it.
For the default worker I've specified the default queue (celery) everything else comes on this queue.
All of the above can be of course turned into configurable items.
Local end to end test results with the 2 workers and queues documented here: https://docs.google.com/document/d/1nhwft8gOB24_c34is3UOqWm7r-v0uKwazPvQRo7WgU0/edit?usp=sharing

To post a comment you must log in.
Revision history for this message
Tom Wardill (twom) wrote :

Looks decent, just some naming and a missing argument I think.

review: Needs Fixing
Revision history for this message
Ioana Lasc (ilasc) wrote :

Thanks Tom, all done.

Revision history for this message
Ioana Lasc (ilasc) wrote :

Added separate repack service, MP ready for review.

Revision history for this message
Tom Wardill (twom) wrote :

This looks good to me now, thanks.

The charm part looks okay afaics, but I've never done reactive charms, so might be worth checking with someone else :)

review: Approve
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

LGTM

review: Approve
Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Otto Co-Pilot (otto-copilot) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/Makefile b/Makefile
index 1ecfd8e..68b80d2 100644
--- a/Makefile
+++ b/Makefile
@@ -110,15 +110,25 @@ run-pack: $(ENV)
110 $(PYTHON) turnipserver.py110 $(PYTHON) turnipserver.py
111111
112run-worker: $(ENV)112run-worker: $(ENV)
113 $(CELERY) -A turnip.tasks worker \113 $(CELERY) -A turnip.tasks worker -n default-worker \
114 --loglevel=debug \114 --loglevel=debug \
115 --concurrency=20 \115 --concurrency=20 \
116 --pool=gevent \116 --pool=gevent \
117 --queue=repacks,celery117 --prefetch-multiplier=1 \
118 --queue=celery
119
120run-repack-worker: $(ENV)
121 $(CELERY) -A turnip.tasks worker -n repack-worker \
122 --loglevel=debug \
123 --concurrency=1 \
124 --pool=gevent \
125 --prefetch-multiplier=1 \
126 --queue=repacks
118127
119run:128run:
120 make run-api &\129 make run-api &\
121 make run-pack &\130 make run-pack &\
131 make run-repack-worker&\
122 make run-worker&\132 make run-worker&\
123 wait;133 wait;
124134
@@ -126,7 +136,9 @@ stop:
126 -pkill -f 'make run-api'136 -pkill -f 'make run-api'
127 -pkill -f 'make run-pack'137 -pkill -f 'make run-pack'
128 -pkill -f 'make run-worker'138 -pkill -f 'make run-worker'
139 -pkill -f 'make run-repack-worker'
129 -pkill -f '$(CELERY) -A tasks worker'140 -pkill -f '$(CELERY) -A tasks worker'
141 -pkill -f '$(CELERY) -A tasks repack-worker'
130142
131$(PIP_CACHE): $(ENV)143$(PIP_CACHE): $(ENV)
132 mkdir -p $(PIP_CACHE)144 mkdir -p $(PIP_CACHE)
diff --git a/charm/turnip-celery/config.yaml b/charm/turnip-celery/config.yaml
index 5d0773a..6cc7929 100644
--- a/charm/turnip-celery/config.yaml
+++ b/charm/turnip-celery/config.yaml
@@ -15,7 +15,4 @@ options:
15 type: int15 type: int
16 default: 116 default: 1
17 description: Celery prefetch multiplier for each worker.17 description: Celery prefetch multiplier for each worker.
18 celery_queues:18
19 type: string
20 default: repacks,celery
21 description: Celery queues, comma-separated and enumerated without spaces.
diff --git a/charm/turnip-celery/lib/charms/turnip/celery.py b/charm/turnip-celery/lib/charms/turnip/celery.py
index 4217c25..b589249 100644
--- a/charm/turnip-celery/lib/charms/turnip/celery.py
+++ b/charm/turnip-celery/lib/charms/turnip/celery.py
@@ -39,6 +39,14 @@ def configure_celery():
39 context, perms=0o644)39 context, perms=0o644)
40 if host.service_running('turnip-celery'):40 if host.service_running('turnip-celery'):
41 host.service_stop('turnip-celery')41 host.service_stop('turnip-celery')
42 templating.render(
43 'turnip-celery-repack.service.j2',
44 '/lib/systemd/system/turnip-celery-repack.service',
45 context, perms=0o644)
46 if host.service_running('turnip-celery-repack'):
47 host.service_stop('turnip-celery-repack')
42 reload_systemd()48 reload_systemd()
43 if not host.service_resume('turnip-celery'):49 if not host.service_resume('turnip-celery'):
44 raise RuntimeError('Failed to start turnip-celery')50 raise RuntimeError('Failed to start turnip-celery')
51 if not host.service_resume('turnip-celery-repack'):
52 raise RuntimeError('Failed to start turnip-celery-repack')
diff --git a/charm/turnip-celery/templates/logrotate.j2 b/charm/turnip-celery/templates/logrotate.j2
index 5087f4f..4db2d62 100644
--- a/charm/turnip-celery/templates/logrotate.j2
+++ b/charm/turnip-celery/templates/logrotate.j2
@@ -11,3 +11,16 @@
11 endscript11 endscript
12}12}
1313
14{{ base_dir }}/logs/turnip-celery-repacks.log {
15 rotate 90
16 daily
17 dateext
18 delaycompress
19 compress
20 missingok
21 create 0644 {{ user }} {{ group }}
22 postrotate
23 service turnip-celery-repack reload
24 endscript
25}
26
diff --git a/charm/turnip-celery/templates/turnip-celery-repack.service.j2 b/charm/turnip-celery/templates/turnip-celery-repack.service.j2
14new file mode 10064427new file mode 100644
index 0000000..6de5c7d
--- /dev/null
+++ b/charm/turnip-celery/templates/turnip-celery-repack.service.j2
@@ -0,0 +1,26 @@
1[Unit]
2Description=Turnip celery repack worker
3After=network.target
4{%- if nfs %}
5BindsTo={{ data_mount_unit }}
6After={{ data_mount_unit }}
7{%- endif %}
8
9[Service]
10User={{ user }}
11Group={{ group }}
12WorkingDirectory={{ code_dir }}
13Environment=PATH={{ venv_dir }}/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
14Environment=REPO_STORE={{ data_dir }}/repos
15Environment=CELERY_BROKER={{ celery_broker }}
16ExecStart={{ venv_dir }}/bin/celery -A turnip.tasks worker -n repack-worker --logfile={{ logs_dir }}/turnip-celery-repacks.log --loglevel=DEBUG --pool=gevent --prefetch-multiplier=1 --queue=repacks --concurrency=1
17ExecReload=/bin/kill -s HUP $MAINPID
18ExecStop=/bin/kill -s TERM $MAINPID
19LimitNOFILE=1048576
20PrivateTmp=true
21
22[Install]
23WantedBy=multi-user.target
24{%- if nfs %}
25WantedBy={{ data_mount_unit }}
26{%- endif %}
diff --git a/charm/turnip-celery/templates/turnip-celery.service.j2 b/charm/turnip-celery/templates/turnip-celery.service.j2
index b612efa..d53b367 100644
--- a/charm/turnip-celery/templates/turnip-celery.service.j2
+++ b/charm/turnip-celery/templates/turnip-celery.service.j2
@@ -16,7 +16,7 @@ Environment=TURNIP_LOG_DIR={{ logs_dir }}
16Environment=CELERY_BROKER={{ celery_broker }}16Environment=CELERY_BROKER={{ celery_broker }}
17Environment=VIRTINFO_ENDPOINT={{ virtinfo_endpoint }}17Environment=VIRTINFO_ENDPOINT={{ virtinfo_endpoint }}
18Environment=VIRTINFO_TIMEOUT={{ virtinfo_timeout }}18Environment=VIRTINFO_TIMEOUT={{ virtinfo_timeout }}
19ExecStart={{ venv_dir }}/bin/celery -A turnip.tasks worker --logfile={{ logs_dir }}/turnip-celery.log --loglevel=DEBUG --pool=gevent --prefetch-multiplier={{ prefetch_multiplier }} --queue={{ celery_queues }}19ExecStart={{ venv_dir }}/bin/celery -A turnip.tasks worker -n default-worker --logfile={{ logs_dir }}/turnip-celery.log --loglevel=DEBUG --pool=gevent --prefetch-multiplier={{ prefetch_multiplier }} --queue=celery
20ExecReload=/bin/kill -s HUP $MAINPID20ExecReload=/bin/kill -s HUP $MAINPID
21ExecStop=/bin/kill -s TERM $MAINPID21ExecStop=/bin/kill -s TERM $MAINPID
22LimitNOFILE=104857622LimitNOFILE=1048576

Subscribers

People subscribed via source and target branches