[dbnode] Add process limits validation (and docs) #1118

prateek · 2018-10-19T23:55:04Z

No description provided.

codecov · 2018-10-20T00:02:04Z

Codecov Report

Merging #1118 into master will increase coverage by <.1%.
The diff coverage is 75.6%.

@@           Coverage Diff            @@
##           master   #1118     +/-   ##
========================================
+ Coverage    71.1%   71.1%   +<.1%     
========================================
  Files         734     737      +3     
  Lines       61781   61855     +74     
========================================
+ Hits        43928   43997     +69     
+ Misses      15010   15006      -4     
- Partials     2843    2852      +9

Flag	Coverage Δ
#aggregator	`81.6% <ø> (-0.1%)`	⬇️
#cluster	`85.7% <ø> (ø)`	⬆️
#collector	`78.1% <ø> (ø)`	⬆️
#dbnode	`80.8% <85.3%> (ø)`	⬆️
#m3em	`73.2% <ø> (ø)`	⬆️
#m3ninx	`75.2% <ø> (-0.1%)`	⬇️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`18.3% <ø> (ø)`	⬆️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`61.7% <ø> (ø)`	⬆️
#x	`74.4% <63.6%> (-0.8%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53c7f2b...84f708d. Read the comment docs.

robskillington · 2018-10-20T00:24:45Z

src/dbnode/server/limits.go

+		return nil
+	}
+
+	return fmt.Errorf(`invalid configuration found [%v]. refer to https://m3db.github.io/m3/operational_guide/kernel_tuning/ for more information`, multiErr.FinalError())


Interesting, should we perhaps add like a meta doc package so that we can replace all the links at a later time?

e.g.

docs.Path("operational_guide/kernel_tuning")

Perhaps also a log field for the doc link?

robskillington · 2018-10-20T02:01:14Z

src/x/os/limits_linux.go

+		return 0, err
+	}
+
+	return int64(num), 0


Shouldn’t this be ‘int64(num), nil’?

richardartoul · 2018-10-22T15:19:12Z

docs/how_to/cluster_hard_way.md

-Alternatively, if you wish to have `m3dbnode` run under `systemd` you can use our [service example](https://github.com/m3db/m3/tree/master/integrations/systemd/m3dbnode.service) which will set sane defaults.
-
-Before running the process make sure the limits are set, if running manually you can raise the limit for the current user with `ulimit -n 500000`.
+[Recommended kernel configuration](../operational_guide/kernel_tuning.md).


Can you make this a complete sentence that explains why its important that people follow this link. I.E something like: Make sure to review our recommended kernel configuration before running M3DB in production as M3DB may exceed the default limits for some default kernel values

Also its called Kernel Configuration here and Kernel Tuning below. Lets pick one (configuration seems more consistent to me)

sounds good

richardartoul · 2018-10-22T15:20:02Z

docs/operational_guide/kernel_tuning.md

+Kernel Tuning
+=============
+
+This document lists the Kernel tweaks `m3dbnode` needs to run well.


is that what we actually call the binary? I always refer to it as m3db or M3DB when I'm writing documentation. We should probably be consistent about that

yeah i'm onboard with being consistent. I'll change to M3DB for now.

richardartoul · 2018-10-22T15:21:33Z

docs/operational_guide/kernel_tuning.md

+## vm.swappiness
+`vm.swappiness` controls how much the virtual memory subsystem will try to swap to disk. By default, the kernel configures this value to `60`, and will try to swap out items in memory even when there is plenty of RAM available to the system.
+
+For performance, we recommend sizing clusters such that `m3dbnode` is running on a substrate such that no-swapping is necessary. And therefore recommend setting the value of `vm.swappiness` to `1`. This tells the kernel to swap as little as possible, without altogether disabling swapping.


Maybe leave out For performance here

"We recommend provisioning m3db clusters such that they use significantly less memory than the maximum on each host. As a result, we recommend setting the value of vm.swappiness to 1 to prevent the O.S from prematurely swapping pages to disk unless it is absolutely necessary."

sure, tweaked to be a little clearer

richardartoul · 2018-10-22T15:32:01Z

docs/operational_guide/kernel_tuning.md

+
+
+## rlimits
+`m3dbnode` also can use a high number of files and we suggest setting a high max open number of files due to per partition fileset volumes.


The explanations of why seem a little out of place. I understand the desire to include them but it really breaks the flow. If you want to keep them in could we add like a ### Rationale subheading to each of these? That way you can just scan through and it just lists variables and what our recommended value is and if you care you can read the rationale

the vm.max_map_count section does the same thing - explain why we need to tweak a value and then what the value should be tweaked to.

richardartoul · 2018-10-22T15:34:55Z

src/dbnode/server/limits.go

+	maxSwappiness = 1
+)
+
+func validateProcessLimits() error {


I thought we were gonna emit these log messages at regular intervals as well if they were misconfigured? Seems like a long-lived struct with an internal goroutine that might be useful

richardartoul · 2018-10-22T15:36:26Z

src/x/os/limits_linux.go

+}
+
+func sysctl(key string) (string, error) {
+	path := sysctlDir + strings.Replace(name, ".", "/", -1)


I assume "name" is a typo here?

richardartoul

LGTM other than minor comments

richardartoul · 2018-11-16T18:12:32Z

docs/operational_guide/kernel_configuration.md

+sysctl -w vm.max_map_count=262144
+```
+
+To set this value permanently, update the `vm.max_map_count` setting in `/etc/sysctl.conf`.


This doc: https://github.com/m3db/m3/blob/cece4eae36f72767fe107b5ce1e015d9ac9548dd/docs/how_to/cluster_hard_way.md

references this file:

https://github.com/m3db/m3/blob/master/integrations/systemd/m3dbnode.service

Do we need to add any of these other configs in there?

richardartoul · 2018-11-16T18:13:05Z

docs/operational_guide/kernel_configuration.md

+your_m3db_user        soft nofile 500000
+```
+
+Alternatively, if you wish to have M3DB run under `systemd` you can use our [service example](https://github.com/m3db/m3/tree/master/integrations/systemd/m3dbnode.service) which will set sane defaults.


Oh ok I see it is here. Can any of the other values go in there?

naw the rest of it is specific to systemd

richardartoul · 2018-11-16T18:13:49Z

src/x/docs/docs.go

+)
+
+// Path returns the url to the
+func Path(section string) string {


robskillington · 2018-11-16T18:51:43Z

src/dbnode/server/server.go

@@ -630,6 +633,21 @@ func interrupt() <-chan os.Signal {
 	return c
 }

+func bgValidateProcessLimits(logger xlog.Logger) {


This is annoying, but should we add a signal to stop this goroutine at some point?

Just because if we start using this more in integration tests having a dangling goroutine may become annoying to deal with.

talked offline: added max monitor duration to control this

robskillington

LGTM other than minor comment

robskillington · 2018-11-16T18:54:32Z

I rebased on master for you, hopefully CI passes.

robskillington reviewed Oct 20, 2018

View reviewed changes

richardartoul reviewed Oct 22, 2018

View reviewed changes

richardartoul approved these changes Nov 16, 2018

View reviewed changes

robskillington reviewed Nov 16, 2018

View reviewed changes

robskillington approved these changes Nov 16, 2018

View reviewed changes

prateek added 6 commits November 16, 2018 13:53

[dbnode] Add process limits validation (and docs)

0038132

grammar

6a4f3ce

metalint ...

f3ce416

feedback

b67dc9a

comment

6334edb

metalint

8b9be43

robskillington force-pushed the prateek/dbnode/process-limits branch from 996be0e to 8b9be43 Compare November 16, 2018 18:53

This was referenced Nov 16, 2018

Document tuning of vm.max_map_count in deployment guide #660

Closed

M3DB should warn about low vm.max_map_count values #960

Closed

feedback

84f708d

prateek merged commit b11e2bf into master Nov 16, 2018

prateek deleted the prateek/dbnode/process-limits branch November 16, 2018 19:39



		## rlimits
		`m3dbnode` also can use a high number of files and we suggest setting a high max open number of files due to per partition fileset volumes.

[dbnode] Add process limits validation (and docs) #1118

[dbnode] Add process limits validation (and docs) #1118

Conversation

prateek commented Oct 19, 2018

codecov bot commented Oct 20, 2018 • edited Loading

Codecov Report

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

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

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

richardartoul left a 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

robskillington left a comment

Choose a reason for hiding this comment

robskillington commented Nov 16, 2018

codecov bot commented Oct 20, 2018 •

edited

Loading