You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ygy <gi...@git.apache.org> on 2015/06/15 18:59:39 UTC
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
GitHub user ygy opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/691
Riak uses ulimit configuration recommended in Basho's docs
Riak has a specific requirement for the number of the allowed open files. It is described at http://docs.basho.com/riak/latest/ops/tuning/open-files-limit/.
Riak generates a warning message if the limit is bellow 65536.
```
# /etc/init.d/riak start
Starting riak: !!!!
!!!! WARNING: ulimit -n is 1024; 65536 is the recommended minimum.
!!!!
```
This PR provides configuration parameter for the number of the open files. The number of open files will default to the recommended value (65536) in case provided value is less than it.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ygy/incubator-brooklyn feature/riak-ulimit
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/691.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #691
----
commit 8214380d0ab631666c982ce9dbca4fd9691e43b4
Author: Yavor Yanchev <ya...@yanchev.com>
Date: 2015-06-15T16:29:13Z
Riak starts with a ulimit configuration based on the Basho's
recommentadtions
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by mikezaccardo <gi...@git.apache.org>.
Github user mikezaccardo commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#issuecomment-112476857
(LGTM)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by ygy <gi...@git.apache.org>.
Github user ygy commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#discussion_r32501514
--- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
@@ -314,6 +314,7 @@ public void launch() {
if (isPackageInstall()) {
commands.add(addSbinPathCommand());
+ commands.add(sudo(format("ulimit -n %s", getNumberOfOpenFiles())));
--- End diff --
There will be no problem with setting the hard limit since the ulimit command is executed with elevated privileges (sudo).
RedHat based distro
```
[root@localhost ~]# cat /etc/redhat-release
CentOS release 6.6 (Final)
[root@localhost ~]# ulimit -n
1024
[root@localhost ~]# ulimit -n 65536
[root@localhost ~]# ulimit -Sn
65536
[root@localhost ~]# ulimit -Hn
65536
```
Ubuntu
```
# cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=14.04
DISTRIB_CODENAME=trusty
DISTRIB_DESCRIPTION="Ubuntu 14.04.2 LTS"
# ulimit -n 65536
# ulimit -Sn
65536
# ulimit -Hn
65536
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#discussion_r32467955
--- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
@@ -594,4 +595,16 @@ private void addRiakOnPath(ScriptHelper scriptHelper) {
// log.warn("riak command not found on PATH. Altering future commands' environment variables from {} to {}", getShellEnvironment(), newPathVariable);
scriptHelper.environmentVariablesReset(newPathVariable);
}
+
+ @Override
+ public Integer getNumberOfOpenFiles() {
+ Integer defaultNumberOfFiles = RiakNode.RIAK_NUM_OPEN_FILES.getConfigKey().getDefaultValue();
+ Integer numberOfFiles = entity.getConfig(RiakNode.RIAK_NUM_OPEN_FILES);
+ if (numberOfFiles.compareTo(defaultNumberOfFiles) < 0) {
+ log.warn("Specified number of open files : {} : less than the required minimum. Using defaults : {}.", numberOfFiles, defaultNumberOfFiles);
+ entity.setAttribute(RiakNode.RIAK_NUM_OPEN_FILES, defaultNumberOfFiles);
--- End diff --
Warning the user is a good idea, but I think it's surprising if the value he sets is not applied. He must have a good reason to change the default value.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#issuecomment-112441693
Looks good after the changes, can be merged.
Only note is that the image used might set ulimit > 65536, so the command will actually decrease it in this case, but I don't think it's worth supporting it right now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#discussion_r32467657
--- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -72,7 +72,12 @@
ConfigKey<String> RIAK_CONF_ADDITIONAL_CONTENT = ConfigKeys.newStringConfigKey(
"riak.riakConf.additionalContent", "Template file (in freemarker format) for setting up additional settings in the riak.conf file", "");
-
+
+ // numberOpenFiles' default value (65536) is based on the Basho's recommendation - http://docs.basho.com/riak/latest/ops/tuning/open-files-limit/
+ @SetFromFlag("numberOpenFiles")
+ AttributeSensorAndConfigKey<Integer, Integer> RIAK_NUM_OPEN_FILES = ConfigKeys.newIntegerSensorAndConfigKey(
--- End diff --
Using a plain `ConfigKey<Integer>` is fine here. You need an `AttributeSensor` only if reading the value back from the machine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/691
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#discussion_r32467322
--- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeDriver.java ---
@@ -45,4 +45,6 @@
void bucketTypeUpdate(String bucketTypeName, String bucketTypeProperties);
void bucketTypeActivate(String bucketTypeName);
+
+ Integer getNumberOfOpenFiles();
--- End diff --
No need to add it to the interface, usually used when accessed from freemarker templates.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#discussion_r32467270
--- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
@@ -314,6 +314,7 @@ public void launch() {
if (isPackageInstall()) {
commands.add(addSbinPathCommand());
+ commands.add(sudo(format("ulimit -n %s", getNumberOfOpenFiles())));
--- End diff --
This will increase the soft limit, but what about the hard limit? Distributions usually have a hard limit < 65536 so this will fail.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by ygy <gi...@git.apache.org>.
Github user ygy commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#discussion_r32501629
--- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNodeSshDriver.java ---
@@ -594,4 +595,16 @@ private void addRiakOnPath(ScriptHelper scriptHelper) {
// log.warn("riak command not found on PATH. Altering future commands' environment variables from {} to {}", getShellEnvironment(), newPathVariable);
scriptHelper.environmentVariablesReset(newPathVariable);
}
+
+ @Override
+ public Integer getNumberOfOpenFiles() {
+ Integer defaultNumberOfFiles = RiakNode.RIAK_NUM_OPEN_FILES.getConfigKey().getDefaultValue();
+ Integer numberOfFiles = entity.getConfig(RiakNode.RIAK_NUM_OPEN_FILES);
+ if (numberOfFiles.compareTo(defaultNumberOfFiles) < 0) {
+ log.warn("Specified number of open files : {} : less than the required minimum. Using defaults : {}.", numberOfFiles, defaultNumberOfFiles);
+ entity.setAttribute(RiakNode.RIAK_NUM_OPEN_FILES, defaultNumberOfFiles);
--- End diff --
Completely agree. It would be better to notify the user and stop the installation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Riak uses ulimit configuration re...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/691#discussion_r32467435
--- Diff: software/nosql/src/main/java/brooklyn/entity/nosql/riak/RiakNode.java ---
@@ -72,7 +72,12 @@
ConfigKey<String> RIAK_CONF_ADDITIONAL_CONTENT = ConfigKeys.newStringConfigKey(
"riak.riakConf.additionalContent", "Template file (in freemarker format) for setting up additional settings in the riak.conf file", "");
-
+
+ // numberOpenFiles' default value (65536) is based on the Basho's recommendation - http://docs.basho.com/riak/latest/ops/tuning/open-files-limit/
+ @SetFromFlag("numberOpenFiles")
--- End diff --
Better name it **max**OpenFiles? Setting this will change the limit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---