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 Approve
Thiago F. Pappacena (community) Approve
Tom Wardill 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
1diff --git a/Makefile b/Makefile
2index 1ecfd8e..68b80d2 100644
3--- a/Makefile
4+++ b/Makefile
5@@ -110,15 +110,25 @@ run-pack: $(ENV)
6 $(PYTHON) turnipserver.py
7
8 run-worker: $(ENV)
9- $(CELERY) -A turnip.tasks worker \
10+ $(CELERY) -A turnip.tasks worker -n default-worker \
11 --loglevel=debug \
12 --concurrency=20 \
13 --pool=gevent \
14- --queue=repacks,celery
15+ --prefetch-multiplier=1 \
16+ --queue=celery
17+
18+run-repack-worker: $(ENV)
19+ $(CELERY) -A turnip.tasks worker -n repack-worker \
20+ --loglevel=debug \
21+ --concurrency=1 \
22+ --pool=gevent \
23+ --prefetch-multiplier=1 \
24+ --queue=repacks
25
26 run:
27 make run-api &\
28 make run-pack &\
29+ make run-repack-worker&\
30 make run-worker&\
31 wait;
32
33@@ -126,7 +136,9 @@ stop:
34 -pkill -f 'make run-api'
35 -pkill -f 'make run-pack'
36 -pkill -f 'make run-worker'
37+ -pkill -f 'make run-repack-worker'
38 -pkill -f '$(CELERY) -A tasks worker'
39+ -pkill -f '$(CELERY) -A tasks repack-worker'
40
41 $(PIP_CACHE): $(ENV)
42 mkdir -p $(PIP_CACHE)
43diff --git a/charm/turnip-celery/config.yaml b/charm/turnip-celery/config.yaml
44index 5d0773a..6cc7929 100644
45--- a/charm/turnip-celery/config.yaml
46+++ b/charm/turnip-celery/config.yaml
47@@ -15,7 +15,4 @@ options:
48 type: int
49 default: 1
50 description: Celery prefetch multiplier for each worker.
51- celery_queues:
52- type: string
53- default: repacks,celery
54- description: Celery queues, comma-separated and enumerated without spaces.
55+
56diff --git a/charm/turnip-celery/lib/charms/turnip/celery.py b/charm/turnip-celery/lib/charms/turnip/celery.py
57index 4217c25..b589249 100644
58--- a/charm/turnip-celery/lib/charms/turnip/celery.py
59+++ b/charm/turnip-celery/lib/charms/turnip/celery.py
60@@ -39,6 +39,14 @@ def configure_celery():
61 context, perms=0o644)
62 if host.service_running('turnip-celery'):
63 host.service_stop('turnip-celery')
64+ templating.render(
65+ 'turnip-celery-repack.service.j2',
66+ '/lib/systemd/system/turnip-celery-repack.service',
67+ context, perms=0o644)
68+ if host.service_running('turnip-celery-repack'):
69+ host.service_stop('turnip-celery-repack')
70 reload_systemd()
71 if not host.service_resume('turnip-celery'):
72 raise RuntimeError('Failed to start turnip-celery')
73+ if not host.service_resume('turnip-celery-repack'):
74+ raise RuntimeError('Failed to start turnip-celery-repack')
75diff --git a/charm/turnip-celery/templates/logrotate.j2 b/charm/turnip-celery/templates/logrotate.j2
76index 5087f4f..4db2d62 100644
77--- a/charm/turnip-celery/templates/logrotate.j2
78+++ b/charm/turnip-celery/templates/logrotate.j2
79@@ -11,3 +11,16 @@
80 endscript
81 }
82
83+{{ base_dir }}/logs/turnip-celery-repacks.log {
84+ rotate 90
85+ daily
86+ dateext
87+ delaycompress
88+ compress
89+ missingok
90+ create 0644 {{ user }} {{ group }}
91+ postrotate
92+ service turnip-celery-repack reload
93+ endscript
94+}
95+
96diff --git a/charm/turnip-celery/templates/turnip-celery-repack.service.j2 b/charm/turnip-celery/templates/turnip-celery-repack.service.j2
97new file mode 100644
98index 0000000..6de5c7d
99--- /dev/null
100+++ b/charm/turnip-celery/templates/turnip-celery-repack.service.j2
101@@ -0,0 +1,26 @@
102+[Unit]
103+Description=Turnip celery repack worker
104+After=network.target
105+{%- if nfs %}
106+BindsTo={{ data_mount_unit }}
107+After={{ data_mount_unit }}
108+{%- endif %}
109+
110+[Service]
111+User={{ user }}
112+Group={{ group }}
113+WorkingDirectory={{ code_dir }}
114+Environment=PATH={{ venv_dir }}/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
115+Environment=REPO_STORE={{ data_dir }}/repos
116+Environment=CELERY_BROKER={{ celery_broker }}
117+ExecStart={{ 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
118+ExecReload=/bin/kill -s HUP $MAINPID
119+ExecStop=/bin/kill -s TERM $MAINPID
120+LimitNOFILE=1048576
121+PrivateTmp=true
122+
123+[Install]
124+WantedBy=multi-user.target
125+{%- if nfs %}
126+WantedBy={{ data_mount_unit }}
127+{%- endif %}
128diff --git a/charm/turnip-celery/templates/turnip-celery.service.j2 b/charm/turnip-celery/templates/turnip-celery.service.j2
129index b612efa..d53b367 100644
130--- a/charm/turnip-celery/templates/turnip-celery.service.j2
131+++ b/charm/turnip-celery/templates/turnip-celery.service.j2
132@@ -16,7 +16,7 @@ Environment=TURNIP_LOG_DIR={{ logs_dir }}
133 Environment=CELERY_BROKER={{ celery_broker }}
134 Environment=VIRTINFO_ENDPOINT={{ virtinfo_endpoint }}
135 Environment=VIRTINFO_TIMEOUT={{ virtinfo_timeout }}
136-ExecStart={{ 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 }}
137+ExecStart={{ 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
138 ExecReload=/bin/kill -s HUP $MAINPID
139 ExecStop=/bin/kill -s TERM $MAINPID
140 LimitNOFILE=1048576

Subscribers

People subscribed via source and target branches