Simplify M3DB config #1371

richardartoul · 2019-02-12T16:18:16Z

Hard code all our defaults for pooling and filesystem and M3 client so they don't need to be provided in every YAML

codecov · 2019-02-12T17:17:16Z

Codecov Report

Merging #1371 into master will decrease coverage by 3.4%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #1371      +/-   ##
=========================================
- Coverage    70.6%   67.1%    -3.5%     
=========================================
  Files         823     666     -157     
  Lines       70369   59352   -11017     
=========================================
- Hits        49692   39870    -9822     
+ Misses      17455   16897     -558     
+ Partials     3222    2585     -637

Flag	Coverage Δ
#aggregator	`78.7% <ø> (-3.6%)`	⬇️
#cluster	`84.1% <ø> (-1.8%)`	⬇️
#collector	`63.7% <ø> (ø)`	⬆️
#dbnode	`74.8% <0%> (-6.1%)`	⬇️
#m3em	`57% <ø> (-16.2%)`	⬇️
#m3ninx	`68.4% <ø> (-5.8%)`	⬇️
#m3nsch	`79.1% <ø> (+27.9%)`	⬆️
#metrics	`17.6% <ø> (ø)`	⬆️
#msg	`74.9% <ø> (-0.3%)`	⬇️
#query	`69.1% <ø> (+4.7%)`	⬆️
#x	`72.2% <ø> (-3.9%)`	⬇️

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 4f3fd6e...0df88d3. Read the comment docs.

codecov · 2019-02-12T17:17:21Z

Codecov Report

Merging #1371 into master will decrease coverage by 0.1%.
The diff coverage is 62.4%.

@@           Coverage Diff            @@
##           master   #1371     +/-   ##
========================================
- Coverage    70.7%   70.5%   -0.2%     
========================================
  Files         823     823             
  Lines       70484   70754    +270     
========================================
+ Hits        49840   49890     +50     
- Misses      17415   17569    +154     
- Partials     3229    3295     +66

Flag	Coverage Δ
#aggregator	`82.3% <ø> (-0.1%)`	⬇️
#cluster	`85.9% <ø> (ø)`	⬆️
#collector	`63.7% <ø> (ø)`	⬆️
#dbnode	`80.4% <62.1%> (-0.6%)`	⬇️
#m3em	`73.2% <ø> (ø)`	⬆️
#m3ninx	`73.9% <ø> (-0.3%)`	⬇️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.6% <ø> (ø)`	⬆️
#msg	`74.9% <ø> (ø)`	⬆️
#query	`64.7% <100%> (ø)`	⬆️
#x	`75.6% <ø> (-0.5%)`	⬇️

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 7450997...d2dffb6. Read the comment docs.

src/cmd/services/m3dbnode/config/fs.go

src/dbnode/server/server.go

This reverts commit 61f2c669587373e658cea22f83a803ec4a62ba63.

src/cmd/services/m3dbnode/config/config.go

src/cmd/services/m3dbnode/config/fs.go

richardartoul · 2019-02-13T16:56:42Z

@benraskin92 Yeah I'll add a "complete" YAML file that has everything filled out and then we can start adding comments to it or something

richardartoul · 2019-02-13T17:14:11Z

@benraskin92 I created a file called m3dbnode-all-config.yml with comments that we can point people to who want to see what all the options are

src/cmd/services/m3dbnode/config/config_test.go

arnikola · 2019-02-14T03:33:15Z

src/cmd/services/m3dbnode/config/pooling.go

+		"tagEncoder":   defaultPoolPolicy,
+		"tagDecoder":   defaultPoolPolicy,
+		"context": poolPolicyDefault{
+			size:                262144,


nit: throw these numbers into consts?

I kinda like it how it is, reads like a YAML file / table. They basically are constants since everything is declared at the top anyways

Fair enough, I was thinking specifically for 262144 and some of the other values which are commonly used, but I guess doing that would kinda imply that they're all tied together

arnikola · 2019-02-14T03:35:27Z

src/cmd/services/m3dbnode/config/pooling.go

+	if err := p.IdentifierPool.initDefaultsAndValidate("identifier"); err != nil {
+		return err
+	}
+	if err := p.FetchBlockMetadataResultsPool.initDefaultsAndValidate("fetchBlockMetadataResults"); err != nil {


I think you may have FetchBlockMetadataResultsPool twice here

nah its dumb, there is FetchBlockMetadataResultsPool and FetchBlocksMetadataResultsPool (note the s)

arnikola · 2019-02-14T03:36:01Z

src/cmd/services/m3dbnode/config/pooling.go

+	if err := p.BlockMetadataSlicePool.initDefaultsAndValidate("blockMetadataSlice"); err != nil {
+		return err
+	}
+	if err := p.BlocksMetadataPool.initDefaultsAndValidate("blocksMetadata"); err != nil {


May have BlocksMetadataPool twice too

same comment as above, they're different (note the s)

arnikola · 2019-02-14T03:37:47Z

src/query/server/server.go

@@ -351,6 +351,7 @@ func newM3DBStorage(
 			etcdCfg = &cfg.ClusterManagement.Etcd

 		case len(cfg.Clusters) == 1 &&
+			cfg.Clusters[0].Client.EnvironmentConfig != nil &&


Is this fine to fail if configs are not set?

I don't follow. Could you elaborate?

My bad, somehow got myself convinced you could have cfg.Clusters[0].Client.EnvironmentConfig be nil and continue here properly at the moment

src/query/api/v1/httpd/handler.go

arnikola · 2019-02-14T03:42:05Z

src/dbnode/client/options.go

+		xretry.NewOptions().
+			SetInitialBackoff(500 * time.Millisecond).
+			SetBackoffFactor(2).
+			SetMaxRetries(3).


nit: Is it intentional to have the write retrier have 3 backoff factor and 2 retries and the fetch retrier to have it flipped?

Yeah I'm just copya-pastaing what was in our old YAML files

arnikola · 2019-02-14T15:54:59Z

src/dbnode/config/m3dbnode-all-config.yml

+    samplingRate: 1.0
+    extended: none
+
+  limits:


Any chance you could add something like this to this config? I've been meaning to add it to defaults and the docs to have users use collision free id generation by default

# Configuration setting for generating metric IDs from tags. tagOptions: metricName: "_new" idScheme: quoted

arnikola · 2019-02-14T15:58:56Z

src/dbnode/server/server.go

-	if writeBatchPoolPolicy.Size == 0 {
-		// If no value set, calculate a reasonabble value based on the commit log
+	var writeBatchPoolSize int
+	if policy.WriteBatchPool.Size != nil {


Might need to check that it's not 0 too?

0 is a valid pool size I think, basically turns off pooling

arnikola · 2019-02-14T16:02:19Z

src/query/server/server.go

@@ -351,6 +351,7 @@ func newM3DBStorage(
 			etcdCfg = &cfg.ClusterManagement.Etcd

 		case len(cfg.Clusters) == 1 &&
+			cfg.Clusters[0].Client.EnvironmentConfig != nil &&


My bad, somehow got myself convinced you could have cfg.Clusters[0].Client.EnvironmentConfig be nil and continue here properly at the moment

arnikola · 2019-02-14T16:03:40Z

src/cmd/services/m3dbnode/config/pooling.go

+		"tagEncoder":   defaultPoolPolicy,
+		"tagDecoder":   defaultPoolPolicy,
+		"context": poolPolicyDefault{
+			size:                262144,


Fair enough, I was thinking specifically for 262144 and some of the other values which are commonly used, but I guess doing that would kinda imply that they're all tied together

richardartoul force-pushed the ra/simple-conf branch from 521ef48 to f880207 Compare February 12, 2019 21:07

richardartoul changed the title ~~[WIP - Dont Review] - Simplify M3DB config~~ Simplify M3DB config - Part 1 Feb 12, 2019

richardartoul requested review from benraskin92, robskillington, prateek and justinjc and removed request for benraskin92 February 12, 2019 21:30

richardartoul force-pushed the ra/simple-conf branch from 8d8d154 to 0127691 Compare February 12, 2019 21:38

arnikola approved these changes Feb 12, 2019

View reviewed changes

src/cmd/services/m3dbnode/config/fs.go Outdated Show resolved Hide resolved

src/cmd/services/m3dbnode/config/fs.go Show resolved Hide resolved

src/dbnode/server/server.go Outdated Show resolved Hide resolved

richardartoul changed the title ~~Simplify M3DB config - Part 1~~ Simplify M3DB config Feb 12, 2019

Richard Artoul added 17 commits February 12, 2019 17:15

wip

a32c9f9

pooling passing

f221c68

delete test yolo shard

108904f

rename method

55a4eec

Revert "rename method"

6f186e5

This reverts commit 61f2c669587373e658cea22f83a803ec4a62ba63.

Add default pool policy

41b8fb0

fix broken test

84483fe

valid refills

26f150c

delete unused var block

c54c3f4

fix bug in parsing values and fix tests

de21c88

fix broken config and test

d1a29ed

default values for filesystem;

6725b03

Fix more config stuff

59ba7f9

remove now-default values from config yamls

b85a430

fix validaiton

1b55e90

make repair config optional for now

828878d

dont run repair code in server.go

dd49fe2

arnikola reviewed Feb 13, 2019

View reviewed changes

src/cmd/services/m3dbnode/config/config.go Outdated Show resolved Hide resolved

src/cmd/services/m3dbnode/config/config.go Show resolved Hide resolved

src/cmd/services/m3dbnode/config/fs.go Outdated Show resolved Hide resolved

src/cmd/services/m3dbnode/config/fs.go Show resolved Hide resolved

Richard Artoul added 2 commits February 13, 2019 09:42

fix tests

5afac43

wip

f9fab84

richardartoul mentioned this pull request Feb 13, 2019

Add postings list cache to M3DB index #1370

Merged

Richard Artoul added 2 commits February 13, 2019 11:53

fix broken test

b4fe06b

remove bounds checks

9bad14f

Add commented all-config file

5fffe3c

Richard Artoul added 16 commits February 13, 2019 12:57

fix broken test

b031b75

add pool validation

2b0d2f2

more pooling config nonsense

bb5b371

rewrite everything

7cc246b

use constants

06211ca

Fix comment explaining how to add new pool

6d152bf

add constant for default pooling type

067dbee

fix error message

2559486

add comment

44ae437

fix broken test

1ef370b

use validate method on client

1ff90a3

fix broken test

088d102

fix integration tests timing out

93ac499

fix broken test

521d31c

fix last broken test

d2dffb6

fix last broken test for eral

5fd8cae

arnikola reviewed Feb 14, 2019

View reviewed changes

remove print statements

1c955dd

arnikola reviewed Feb 14, 2019

View reviewed changes

richardartoul merged commit 1464f33 into master Feb 14, 2019

robskillington deleted the ra/simple-conf branch February 14, 2019 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify M3DB config #1371

Simplify M3DB config #1371

richardartoul commented Feb 12, 2019 •

edited

Loading

codecov bot commented Feb 12, 2019

codecov bot commented Feb 12, 2019 •

edited

Loading

richardartoul commented Feb 13, 2019

richardartoul commented Feb 13, 2019

arnikola Feb 14, 2019

richardartoul Feb 14, 2019

arnikola Feb 14, 2019

arnikola Feb 14, 2019

richardartoul Feb 14, 2019

arnikola Feb 14, 2019

arnikola Feb 14, 2019

richardartoul Feb 14, 2019

arnikola Feb 14, 2019

richardartoul Feb 14, 2019

arnikola Feb 14, 2019

arnikola Feb 14, 2019

richardartoul Feb 14, 2019

arnikola Feb 14, 2019

arnikola Feb 14, 2019

richardartoul Feb 14, 2019

arnikola Feb 14, 2019

arnikola Feb 14, 2019

Simplify M3DB config #1371

Simplify M3DB config #1371

Conversation

richardartoul commented Feb 12, 2019 • edited Loading

codecov bot commented Feb 12, 2019

Codecov Report

codecov bot commented Feb 12, 2019 • edited Loading

Codecov Report

richardartoul commented Feb 13, 2019

richardartoul commented Feb 13, 2019

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 commented Feb 12, 2019 •

edited

Loading

codecov bot commented Feb 12, 2019 •

edited

Loading