juju-core

Merge lp:~dave-cheney/juju-core/119-fix-mongo-ulimits into lp:~juju/juju-core/trunk

  1. 119-fix-mongo-ulimits
  2. Merge into trunk
Proposed by Dave Cheney
Status: Merged
Approved by: William Reade
Approved revision: no longer in the source branch.
Merged at revision: 1204
Proposed branch: lp:~dave-cheney/juju-core/119-fix-mongo-ulimits
Merge into: lp:~juju/juju-core/trunk
Prerequisite: lp:~dave-cheney/juju-core/120-upstart-add-limit-verb
Diff against target: 102 lines (+25/-7)
2 files modified
environs/cloudinit/cloudinit.go (+20/-2)
environs/cloudinit/cloudinit_test.go (+5/-5)
To merge this branch: bzr merge lp:~dave-cheney/juju-core/119-fix-mongo-ulimits
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+161276@code.launchpad.net

Description of the change

environs/cloudinit: increase fd limit for mongodb

https://codereview.appspot.com/8551048/

To post a comment you must log in.
Revision history for this message
Dimiter Naydenov (dimitern) wrote :

LGTM with one suggestion.

https://codereview.appspot.com/8551048/diff/1/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/8551048/diff/1/environs/cloudinit/cloudinit.go#newcode301
environs/cloudinit/cloudinit.go:301: "nofile": fmt.Sprintf("%d %d",
max(maxConns, 65000), max(maxConns, 65000)),
should we have var maxFiles = max(maxConns, 65000) up there and use it
here twice?

https://codereview.appspot.com/8551048/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM with one suggestion slightly different from dimiter's.

https://codereview.appspot.com/8551048/diff/1/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/8551048/diff/1/environs/cloudinit/cloudinit.go#newcode301
environs/cloudinit/cloudinit.go:301: "nofile": fmt.Sprintf("%d %d",
max(maxConns, 65000), max(maxConns, 65000)),
i think we should put all the constants up at the top of the file.
const (
      maxMongoFiles = 65000
      maxAgentFiles = 20000
)

rather than trying to pretend that they're the same thing
with the same maxConns constant.

https://codereview.appspot.com/8551048/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

On 2013/04/29 10:11:06, rog wrote:
> LGTM with one suggestion slightly different from dimiter's.

https://codereview.appspot.com/8551048/diff/1/environs/cloudinit/cloudinit.go
> File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/8551048/diff/1/environs/cloudinit/cloudinit.go#newcode301
> environs/cloudinit/cloudinit.go:301: "nofile": fmt.Sprintf("%d %d",
> max(maxConns, 65000), max(maxConns, 65000)),
> i think we should put all the constants up at the top of the file.
> const (
> maxMongoFiles = 65000
> maxAgentFiles = 20000
> )

> rather than trying to pretend that they're the same thing
> with the same maxConns constant.

(and lose the unnecessary max logic)

https://codereview.appspot.com/8551048/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'environs/cloudinit/cloudinit.go'
2--- environs/cloudinit/cloudinit.go 2013-04-24 05:07:39 +0000
3+++ environs/cloudinit/cloudinit.go 2013-04-27 06:02:25 +0000
4@@ -17,6 +17,10 @@
5 "path"
6 )
7
8+// maxConns controls the various ulimit parameters passed
9+// to upstart jobs for the state and api servers.
10+const maxConns = 20000
11+
12 // MachineConfig represents initialization information for a new juju machine.
13 type MachineConfig struct {
14 // StateServer specifies whether the new machine will run the
15@@ -267,8 +271,11 @@
16 conf := &upstart.Conf{
17 Service: *svc,
18 Desc: fmt.Sprintf("juju %s agent", tag),
19- Cmd: cmd,
20- Out: logPath,
21+ Limit: map[string]string{
22+ "nofile": fmt.Sprintf("%d %d", maxConns, maxConns),
23+ },
24+ Cmd: cmd,
25+ Out: logPath,
26 }
27 cmds, err := conf.InstallCommands()
28 if err != nil {
29@@ -290,6 +297,10 @@
30 conf := &upstart.Conf{
31 Service: *svc,
32 Desc: "juju state database",
33+ Limit: map[string]string{
34+ "nofile": fmt.Sprintf("%d %d", max(maxConns, 65000), max(maxConns, 65000)),
35+ "nproc": fmt.Sprintf("%d %d", maxConns, maxConns),
36+ },
37 Cmd: "/usr/bin/mongod" +
38 " --auth" +
39 " --dbpath=/var/lib/juju/db" +
40@@ -310,6 +321,13 @@
41 return nil
42 }
43
44+func max(a, b int) int {
45+ if a > b {
46+ return a
47+ }
48+ return b
49+}
50+
51 // versionDir converts a tools URL into a name
52 // to use as a directory for storing the tools executables in
53 // by using the last element stripped of its extension.
54
55=== modified file 'environs/cloudinit/cloudinit_test.go'
56--- environs/cloudinit/cloudinit_test.go 2013-04-24 05:07:39 +0000
57+++ environs/cloudinit/cloudinit_test.go 2013-04-27 06:02:25 +0000
58@@ -87,7 +87,7 @@
59 dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.0
60 dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.1
61 dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.2
62-cat >> /etc/init/juju-db\.conf << 'EOF'\\ndescription "juju state database"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nexec /usr/bin/mongod --auth --dbpath=/var/lib/juju/db --sslOnNormalPorts --sslPEMKeyFile '/var/lib/juju/server\.pem' --sslPEMKeyPassword ignored --bind_ip 0\.0\.0\.0 --port 37017 --noprealloc --syslog --smallfiles\\nEOF\\n
63+cat >> /etc/init/juju-db\.conf << 'EOF'\\ndescription "juju state database"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 65000 65000\\nlimit nproc 20000 20000\\n\\nexec /usr/bin/mongod --auth --dbpath=/var/lib/juju/db --sslOnNormalPorts --sslPEMKeyFile '/var/lib/juju/server\.pem' --sslPEMKeyPassword ignored --bind_ip 0\.0\.0\.0 --port 37017 --noprealloc --syslog --smallfiles\\nEOF\\n
64 start juju-db
65 mkdir -p '/var/lib/juju/agents/bootstrap'
66 echo 'datadir: /var/lib/juju\\nstateservercert:\\n[^']+stateserverkey:\\n[^']+mongoport: 37017\\napiport: 17070\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n addrs:\\n - localhost:37017\\n cacert:\\n[^']+ tag: bootstrap\\n password: ""\\noldapipassword: ""\\napiinfo:\\n addrs:\\n - localhost:17070\\n cacert:\\n[^']+ tag: bootstrap\\n password: ""\\n' > '/var/lib/juju/agents/bootstrap/agent\.conf'
67@@ -100,7 +100,7 @@
68 echo 'datadir: /var/lib/juju\\nstateservercert:\\n[^']+stateserverkey:\\n[^']+mongoport: 37017\\napiport: 17070\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n addrs:\\n - localhost:37017\\n cacert:\\n[^']+ tag: machine-0\\n password: ""\\noldapipassword: ""\\napiinfo:\\n addrs:\\n - localhost:17070\\n cacert:\\n[^']+ tag: machine-0\\n password: ""\\n' > '/var/lib/juju/agents/machine-0/agent\.conf'
69 chmod 600 '/var/lib/juju/agents/machine-0/agent\.conf'
70 ln -s 1\.2\.3-precise-amd64 '/var/lib/juju/tools/machine-0'
71-cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --log-file /var/log/juju/machine-0\.log --data-dir '/var/lib/juju' --machine-id 0 --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
72+cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --log-file /var/log/juju/machine-0\.log --data-dir '/var/lib/juju' --machine-id 0 --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
73 start jujud-machine-0
74 `,
75 }, {
76@@ -143,7 +143,7 @@
77 dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.0
78 dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.1
79 dd bs=1M count=1 if=/dev/zero of=/var/lib/juju/db/journal/prealloc\.2
80-cat >> /etc/init/juju-db\.conf << 'EOF'\\ndescription "juju state database"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nexec /usr/bin/mongod --auth --dbpath=/var/lib/juju/db --sslOnNormalPorts --sslPEMKeyFile '/var/lib/juju/server\.pem' --sslPEMKeyPassword ignored --bind_ip 0\.0\.0\.0 --port 37017 --noprealloc --syslog --smallfiles\\nEOF\\n
81+cat >> /etc/init/juju-db\.conf << 'EOF'\\ndescription "juju state database"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 65000 65000\\nlimit nproc 20000 20000\\n\\nexec /usr/bin/mongod --auth --dbpath=/var/lib/juju/db --sslOnNormalPorts --sslPEMKeyFile '/var/lib/juju/server\.pem' --sslPEMKeyPassword ignored --bind_ip 0\.0\.0\.0 --port 37017 --noprealloc --syslog --smallfiles\\nEOF\\n
82 start juju-db
83 mkdir -p '/var/lib/juju/agents/bootstrap'
84 echo 'datadir: /var/lib/juju\\nstateservercert:\\n[^']+stateserverkey:\\n[^']+mongoport: 37017\\napiport: 17070\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n addrs:\\n - localhost:37017\\n cacert:\\n[^']+ tag: bootstrap\\n password: ""\\noldapipassword: ""\\napiinfo:\\n addrs:\\n - localhost:17070\\n cacert:\\n[^']+ tag: bootstrap\\n password: ""\\n' > '/var/lib/juju/agents/bootstrap/agent\.conf'
85@@ -156,7 +156,7 @@
86 echo 'datadir: /var/lib/juju\\nstateservercert:\\n[^']+stateserverkey:\\n[^']+mongoport: 37017\\napiport: 17070\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n addrs:\\n - localhost:37017\\n cacert:\\n[^']+ tag: machine-0\\n password: ""\\noldapipassword: ""\\napiinfo:\\n addrs:\\n - localhost:17070\\n cacert:\\n[^']+ tag: machine-0\\n password: ""\\n' > '/var/lib/juju/agents/machine-0/agent\.conf'
87 chmod 600 '/var/lib/juju/agents/machine-0/agent\.conf'
88 ln -s 1\.2\.3-raring-amd64 '/var/lib/juju/tools/machine-0'
89-cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --log-file /var/log/juju/machine-0\.log --data-dir '/var/lib/juju' --machine-id 0 --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
90+cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --log-file /var/log/juju/machine-0\.log --data-dir '/var/lib/juju' --machine-id 0 --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
91 start jujud-machine-0
92 `,
93 }, {
94@@ -194,7 +194,7 @@
95 echo 'datadir: /var/lib/juju\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n addrs:\\n - state-addr\.example\.com:12345\\n cacert:\\n[^']+ tag: machine-99\\n password: ""\\noldapipassword: ""\\napiinfo:\\n addrs:\\n - state-addr\.example\.com:54321\\n cacert:\\n[^']+ tag: machine-99\\n password: ""\\n' > '/var/lib/juju/agents/machine-99/agent\.conf'
96 chmod 600 '/var/lib/juju/agents/machine-99/agent\.conf'
97 ln -s 1\.2\.3-linux-amd64 '/var/lib/juju/tools/machine-99'
98-cat >> /etc/init/jujud-machine-99\.conf << 'EOF'\\ndescription "juju machine-99 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nexec /var/lib/juju/tools/machine-99/jujud machine --log-file /var/log/juju/machine-99\.log --data-dir '/var/lib/juju' --machine-id 99 --debug >> /var/log/juju/machine-99\.log 2>&1\\nEOF\\n
99+cat >> /etc/init/jujud-machine-99\.conf << 'EOF'\\ndescription "juju machine-99 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-99/jujud machine --log-file /var/log/juju/machine-99\.log --data-dir '/var/lib/juju' --machine-id 99 --debug >> /var/log/juju/machine-99\.log 2>&1\\nEOF\\n
100 start jujud-machine-99
101 `,
102 },

Subscribers

People subscribed via source and target branches