Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow suffixes on memory size in config #719

Merged
merged 4 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions collect/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,15 @@ func (i *InMemCollector) checkAlloc() {
var mem runtime.MemStats
runtime.ReadMemStats(&mem)
i.Metrics.Gauge("memory_heap_allocation", int64(mem.Alloc))
if err != nil || inMemConfig.MaxAlloc == 0 || mem.Alloc < inMemConfig.MaxAlloc {
if err != nil || inMemConfig.MaxAlloc == 0 || mem.Alloc < uint64(inMemConfig.MaxAlloc) {
return
}

// Figure out what fraction of the total cache we should remove. We'd like it to be
// enough to get us below the max capacity, but not TOO much below.
// Because our impact numbers are only the data size, reducing by enough to reach
// max alloc will actually do more than that.
totalToRemove := mem.Alloc - inMemConfig.MaxAlloc
totalToRemove := mem.Alloc - uint64(inMemConfig.MaxAlloc)

// The size of the cache exceeds the user's intended allocation, so we're going to
// remove the traces from the cache that have had the most impact on allocation.
Expand Down
2 changes: 1 addition & 1 deletion collect/collect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ func TestStableMaxAlloc(t *testing.T) {
var mem runtime.MemStats
runtime.ReadMemStats(&mem)
// Set MaxAlloc, which should cause cache evictions.
conf.GetInMemoryCollectorCacheCapacityVal.MaxAlloc = mem.Alloc * 99 / 100
conf.GetInMemoryCollectorCacheCapacityVal.MaxAlloc = config.MemorySize(mem.Alloc * 99 / 100)

coll.mutex.Unlock()

Expand Down
9 changes: 5 additions & 4 deletions config.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Honeycomb Refinery Configuration Documentation

This is the documentation for the configuration file for Honeycomb's Refinery.
It was automatically generated on 2023-06-12 at 18:45:41 UTC.
It was automatically generated on 2023-06-20 at 16:34:41 UTC.

## The Config file

Expand Down Expand Up @@ -804,7 +804,6 @@ Depending on deployment details, system memory information may not be available.
If it is not, a warning will be logged and the value of MaxAlloc will be used.
If this value is 0, MaxAlloc will be used.
Requires MaxAlloc to be nonzero.
TODO: NOT YET IMPLEMENTED

- Eligible for live reload.
- Type: `percentage`
Expand All @@ -815,12 +814,14 @@ TODO: NOT YET IMPLEMENTED

MaxAlloc is the maximum number of bytes that should be allocated by the collector.

If set, it must be an integer >= 0.
If set, this must be a memory size.
64-bit values are supported.
Sizes with standard unit suffixes like "MB" and "GiB" are also supported.
The full list of supported values can be found at https://pkg.go.dev/github.com/docker/go-units#pkg-constants.
See MaxMemory for more details.

- Eligible for live reload.
- Type: `int`
- Type: `memorysize`
---
## Buffer Sizes

Expand Down
32 changes: 31 additions & 1 deletion config/configLoadHelpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func Test_formatFromResponse(t *testing.T) {
}

// Verifies that we can load a time.Duration from a string.
func Test_load(t *testing.T) {
func Test_loadDuration(t *testing.T) {
type dur struct {
D Duration
}
Expand Down Expand Up @@ -97,3 +97,33 @@ func Test_load(t *testing.T) {
})
}
}

// Verifies that we can load a memory size from a string.
func Test_loadMemsize(t *testing.T) {
type mem struct {
M MemorySize `yaml:"M" json:"M" toml:"M"`
}

tests := []struct {
name string
format Format
text string
into any
want any
wantErr bool
}{
{"yaml", FormatYAML, `M: 1Gb`, &mem{}, &mem{MemorySize(0x4000_0000)}, false},
{"json", FormatJSON, `{"M": "1Gb"}`, &mem{}, &mem{MemorySize(0x4000_0000)}, false},
{"toml", FormatTOML, `M="1Gb"`, &mem{}, &mem{MemorySize(0x4000_0000)}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := load(strings.NewReader(tt.text), tt.format, tt.into); (err != nil) != tt.wantErr {
t.Errorf("load() error = %v, wantErr %v", err, tt.wantErr)
}
if !reflect.DeepEqual(tt.into, tt.want) {
t.Errorf("load() = %#v, want %#v", tt.into, tt.want)
}
})
}
}
2 changes: 1 addition & 1 deletion config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func TestMaxAlloc(t *testing.T) {
c, err := getConfig([]string{"--no-validate", "--config", config, "--rules_config", rules})
assert.NoError(t, err)

expected := uint64(16 * 1024 * 1024 * 1024)
expected := MemorySize(16 * 1024 * 1024 * 1024)
inMemConfig, err := c.GetInMemCollectorCacheCapacity()
assert.NoError(t, err)
assert.Equal(t, expected, inMemConfig.MaxAlloc)
Expand Down
23 changes: 20 additions & 3 deletions config/file_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"sync"
"time"

"github.com/docker/go-units"
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
"gopkg.in/yaml.v3"
)

Expand All @@ -29,6 +30,22 @@ func (d *Duration) UnmarshalText(text []byte) error {
return nil
}

// We also use a special type for memory sizes
type MemorySize uint64

func (m MemorySize) MarshalText() ([]byte, error) {
return []byte(units.HumanSize(float64(m))), nil
}

func (m *MemorySize) UnmarshalText(text []byte) error {
size, err := units.RAMInBytes(string(text))
if err != nil {
return err
}
*m = MemorySize(size)
return nil
}

type fileConfig struct {
mainConfig *configContents
mainHash string
Expand Down Expand Up @@ -167,9 +184,9 @@ type RedisPeerManagementConfig struct {

type CollectionConfig struct {
// CacheCapacity must be less than math.MaxInt32
CacheCapacity int `yaml:"CacheCapacity" default:"10_000"`
MaxMemory int `yaml:"MaxMemory" default:"75"`
MaxAlloc uint64 `yaml:"MaxAlloc"`
CacheCapacity int `yaml:"CacheCapacity" default:"10_000"`
MaxMemory int `yaml:"MaxMemory" default:"75"`
MaxAlloc MemorySize `yaml:"MaxAlloc"`
}

type BufferSizeConfig struct {
Expand Down
11 changes: 6 additions & 5 deletions config/metadata/configMeta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -919,22 +919,23 @@ groups:
warning will be logged and the value of MaxAlloc will be used.
If this value is 0, MaxAlloc will be used. Requires MaxAlloc to be
nonzero.
TODO: NOT YET IMPLEMENTED

- name: MaxAlloc
v1group: InMemCollector
v1name: MaxAlloc
type: int
valuetype: nondefault
type: memorysize
valuetype: memorysize
default: 0
reload: true
validations:
- type: requiredWith
arg: MaxMemory
summary: is the maximum number of bytes that should be allocated by the collector.
description: >
If set, it must be an integer >= 0. 64-bit values are
supported.
If set, this must be a memory size. 64-bit values are supported. Sizes
with standard unit suffixes like "MB" and "GiB" are also supported.
The full list of supported values can be found at
https://pkg.go.dev/github.com/docker/go-units#pkg-constants.
See MaxMemory for more details.

- name: BufferSizes
Expand Down
9 changes: 9 additions & 0 deletions config/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"
"time"

"github.com/docker/go-units"
"golang.org/x/exp/slices"
)

Expand Down Expand Up @@ -136,6 +137,14 @@ func validateDatatype(k string, v any, typ string) string {
if _, err := time.ParseDuration(v.(string)); err != nil {
return fmt.Sprintf("field %s (%v) must be a valid duration like '3m30s' or '100ms'", k, v)
}
case "memorysize":
if !isString(v) {
// we support pure numbers here, so if it's not already a string, then stringize it
v = fmt.Sprintf("%v", v)
}
if _, err := units.FromHumanSize(v.(string)); err != nil {
return fmt.Sprintf("field %s (%v) must be a valid memory size like '1Gb' or '100_000_000'", k, v)
}
case "hostport":
if !isString(v) {
return fmt.Sprintf("field %s must be a hostport", k)
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/creasty/defaults v1.7.0
github.com/davecgh/go-spew v1.1.1
github.com/dgryski/go-wyhash v0.0.0-20191203203029-c4841ae36371
github.com/docker/go-units v0.5.0
github.com/facebookgo/inject v0.0.0-20180706035515-f23751cae28b
github.com/facebookgo/startstop v0.0.0-20161013234910-bc158412526d
github.com/gomodule/redigo v1.8.9
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ github.com/dgryski/go-wyhash v0.0.0-20191203203029-c4841ae36371 h1:bz5ApY1kzFBvw
github.com/dgryski/go-wyhash v0.0.0-20191203203029-c4841ae36371/go.mod h1:/ENMIO1SQeJ5YQeUWWpbX8f+bS8INHrrhFjXgEqi4LA=
github.com/dgryski/trifles v0.0.0-20200323201526-dd97f9abfb48 h1:fRzb/w+pyskVMQ+UbP35JkH8yB7MYb4q/qhBarqZE6g=
github.com/dgryski/trifles v0.0.0-20200323201526-dd97f9abfb48/go.mod h1:if7Fbed8SFyPtHLHbg49SI7NAdJiC5WIA09pe59rfAA=
github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4=
github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a h1:yDWHCSQ40h88yih2JAcL6Ls/kVkSE8GFACTGVnMPruw=
github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a/go.mod h1:7Ga40egUymuWXxAe151lTNnCv97MddSOVsjpPPkityA=
github.com/facebookgo/ensure v0.0.0-20200202191622-63f1cf65ac4c h1:8ISkoahWXwZR41ois5lSJBSVw4D0OV19Ht/JSTzvSv0=
Expand Down
2 changes: 1 addition & 1 deletion rules.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Honeycomb Refinery Rules Documentation

This is the documentation for the rules configuration for Honeycomb's Refinery.
It was automatically generated on 2023-06-12 at 18:45:41 UTC.
It was automatically generated on 2023-06-17 at 16:49:03 UTC.

## The Rules file

Expand Down
19 changes: 19 additions & 0 deletions tools/convert/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"regexp"
"strings"
"time"

"github.com/docker/go-units"
)

// This file contains template helper functions, which must be listed in this
Expand All @@ -28,6 +30,7 @@ func helpers() template.FuncMap {
"indentRest": indentRest,
"join": join,
"makeSlice": makeSlice,
"memorysize": memorysize,
"meta": meta,
"nonDefaultOnly": nonDefaultOnly,
"nonEmptyString": nonEmptyString,
Expand Down Expand Up @@ -158,6 +161,22 @@ func makeSlice(a ...string) []string {
return a
}

// memorysize takes a memory size (if the previous value had it) and returns a string representation
// of memory size in human-readable form.
func memorysize(data map[string]any, key, oldkey string, example string) string {
i64 := int64(0)
if value, ok := _fetch(data, oldkey); ok && value != "" {
switch i := value.(type) {
case int64:
i64 = i
case int:
i64 = int64(i)
}
return units.HumanSize(float64(i64))
}
return fmt.Sprintf(`# %s: %v`, key, yamlf(example))
}

func meta(s string) string {
return "{{ " + s + " }}"
}
Expand Down
11 changes: 7 additions & 4 deletions tools/convert/templates/configV2.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
## Honeycomb Refinery Configuration ##
######################################
#
# created {{ now }} from {{ .Input }} using a template generated on 2023-06-12 at 15:58:04 UTC
# created {{ now }} from {{ .Input }} using a template generated on 2023-06-20 at 16:34:39 UTC

# This file contains the configuration for the Honeycomb Refinery.
# More stuff will go here later.
Expand Down Expand Up @@ -740,7 +740,7 @@ Collection:
## details, system memory information may not be available. If it is not,
## a warning will be logged and the value of MaxAlloc will be used. If
## this value is 0, MaxAlloc will be used. Requires MaxAlloc to be
## nonzero. TODO: NOT YET IMPLEMENTED
## nonzero.
##
## default: 75
## Eligible for live reload.
Expand All @@ -749,11 +749,14 @@ Collection:
## MaxAlloc is the maximum number of bytes that should be allocated by
## the collector.
##
## If set, it must be an integer >= 0. 64-bit values are supported. See
## If set, this must be a memory size. 64-bit values are supported. Sizes
## with standard unit suffixes like "MB" and "GiB" are also supported.
## The full list of supported values can be found at
## https://pkg.go.dev/github.com/docker/go-units#pkg-constants. See
## MaxMemory for more details.
##
## Eligible for live reload.
{{ nonDefaultOnly .Data "MaxAlloc" "InMemCollector.MaxAlloc" 0 }}
{{ memorysize .Data "MaxAlloc" "InMemCollector.MaxAlloc" "" }}

##################
## Buffer Sizes ##
Expand Down
2 changes: 2 additions & 0 deletions tools/convert/templates/genfield.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
{{ printf "nonEmptyString .Data \"%s\" \"%s\" %#v" $field.Name $oldname $field.Example| meta | indent 4 }}
{{- else if eq $field.ValueType "secondstoduration" }}
{{ printf "secondsToDuration .Data \"%s\" \"%s\" %#v" $field.Name $oldname $field.Example| meta | indent 4 }}
{{- else if eq $field.ValueType "memorysize" }}
{{ printf "memorysize .Data \"%s\" \"%s\" %#v" $field.Name $oldname $field.Example| meta | indent 4 }}
{{- else if eq $field.ValueType "choice" }}
{{ printf "Options: %s" (join $field.Choices " ") | comment | indent 4 }}
{{ printf "choice .Data \"%s\" \"%s\" %s \"%s\"" $field.Name $oldname (genSlice $field.Choices) $field.Default | meta | indent 4 }}
Expand Down