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

Cleans up pkg/storage/segment and global state #183

Merged
merged 7 commits into from
May 12, 2021
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
19 changes: 19 additions & 0 deletions .github/workflows/lint-go.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: Go Linting

on:
push:
branches: [main]
pull_request:
branches: [main]

jobs:
go-lint:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Run revive action
uses: petethepig/revive-action@v5
with:
config: revive.toml
exclude: "vendor/..."
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ ifeq ("$(shell go env GOARCH || true)", "arm64")
GODEBUG=asyncpreemptoff=1
endif

ALL_SPIES = "ebpfspy,rbspy,pyspy,debugspy"
ifeq ("$(shell go env GOOS || true)", "linux")
ENABLED_SPIES ?= "ebpfspy,rbspy,pyspy"
else
Expand Down Expand Up @@ -79,7 +80,7 @@ ensure-logrus-not-used:

.PHONY: unused
unused:
staticcheck -f stylish -unused.whole-program ./...
staticcheck -f stylish -tags $(ALL_SPIES) -unused.whole-program ./...

.PHONY: install-dev-tools
install-dev-tools:
Expand Down
2 changes: 1 addition & 1 deletion cmd/pyroscope/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

func main() {
cfg := config.New()
cfg := &config.Config{}
err := cli.Start(cfg)
if err != nil {
os.Stderr.Write([]byte(color.RedString("Error: ") + err.Error() + "\n\n"))
Expand Down
47 changes: 1 addition & 46 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ type Server struct {

// TODO: I don't think a lot of people will change these values.
// I think these should just be constants.
Multiplier int `skip:"true" def:"10"`
MinResolution time.Duration `skip:"true" def:"10s"`
MaxResolution time.Duration `skip:"true" def:"8760h"` // 365 days
StorageMaxDepth int `skip:"true"`
BadgerNoTruncate bool `def:"false" desc:"indicates whether value log files should be truncated to delete corrupt data, if any"`
BadgerNoTruncate bool `def:"false" desc:"indicates whether value log files should be truncated to delete corrupt data, if any"`

MaxNodesSerialization int `def:"2048" desc:"max number of nodes used when saving profiles to disk"`
MaxNodesRender int `def:"8192" desc:"max number of nodes used to display data on the frontend"`
Expand Down Expand Up @@ -96,44 +92,3 @@ type Exec struct {
GroupName string `def:"" desc:"starts process under specified group name"`
PyspyBlocking bool `def:"false" desc:"enables blocking mode for pyspy"`
}

func calculateMaxDepth(min, max time.Duration, multiplier int) int {
depth := 0
for min < max {
min *= time.Duration(multiplier)
depth++
}
return depth
}

// TODO: remove these preset configs
func New() *Config {
return NewForTests("/tmp/")
}

func NewForTests(path string) *Config {
cfg := &Config{
Server: Server{
StoragePath: path,
APIBindAddr: ":4040",

CacheSegmentSize: 10,
CacheTreeSize: 10,
CacheDictionarySize: 10,
CacheDimensionSize: 10,

Multiplier: 10,
MinResolution: 10 * time.Second,
MaxResolution: time.Hour * 24 * 365 * 5,

MaxNodesSerialization: 2048,
MaxNodesRender: 2048,

OutOfSpaceThreshold: 512 * 1024 * 1024, // bytes (default: 512MB)
},
}

cfg.Server.StorageMaxDepth = calculateMaxDepth(cfg.Server.MinResolution, cfg.Server.MaxResolution, cfg.Server.Multiplier)

return cfg
}
21 changes: 21 additions & 0 deletions pkg/storage/segment/constants.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package segment

import "time"

// TODO: at some point we should change it so that segments can support different
// resolution and multiplier values. For now they are constants
const (
multiplier = 10
resolution = 10 * time.Second
)

var durations = []time.Duration{}

func init() {
d := resolution
// TODO: better upper boundary, currently 50 is a magic number
for i := 0; i < 50; i++ {
durations = append(durations, d)
d *= time.Duration(multiplier)
}
}
2 changes: 1 addition & 1 deletion pkg/storage/segment/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (sm *storageMock) Get(st, et time.Time, cb func(depth int, samples, writes

// if you change something in this test make sure it doesn't change test coverage.
func fuzzTest(testWrites bool, writeSize func() int) {
s := New(10*time.Second, 10)
s := New()
m := newMock(10 * time.Second)

r := rand.New(rand.NewSource(1213))
Expand Down
20 changes: 2 additions & 18 deletions pkg/storage/segment/segment.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ type streeNode struct {
children []*streeNode
}

var durations []time.Duration

func (parent *streeNode) replace(child *streeNode) {
i := child.time.Sub(parent.time) / durations[child.depth]
parent.children[i] = child
Expand Down Expand Up @@ -170,26 +168,12 @@ func newNode(t time.Time, depth, multiplier int) *streeNode {
return sn
}

// TODO: global state is not good
func InitializeGlobalState(resolution time.Duration, multiplier int) {
// this is here just to initialize global duration variable
New(resolution, multiplier)
}

func New(resolution time.Duration, multiplier int) *Segment {
func New() *Segment {
st := &Segment{
resolution: resolution,
multiplier: multiplier,
durations: []time.Duration{},
}

// TODO better upper boundary
d := resolution
for i := 0; i < 50; i++ {
st.durations = append(st.durations, d)
d *= time.Duration(multiplier)
durations: durations,
}
durations = st.durations

return st
}
Expand Down
29 changes: 13 additions & 16 deletions pkg/storage/segment/segment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,10 @@ func expectChildrenSamplesAddUpToParentSamples(tn *streeNode) {
}

var _ = Describe("stree", func() {
r := 10 * time.Second
m := 10

Context("Get", func() {
Context("When there's no root", func() {
It("get doesn't fail", func() {
s := New(r, m)
s := New()
Expect(doGet(s, testing.SimpleTime(0), testing.SimpleTime(39))).To(HaveLen(0))
})
})
Expand All @@ -64,7 +61,7 @@ var _ = Describe("stree", func() {
Context("When second insert is far in the future", func() {
It("sets root properly", func() {
log.Println("---")
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(1330),
testing.SimpleTime(1339), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expand All @@ -77,7 +74,7 @@ var _ = Describe("stree", func() {
Context("When second insert is far in the past", func() {
It("sets root properly", func() {
log.Println("---")
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(2030),
testing.SimpleTime(2039), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expand All @@ -91,31 +88,31 @@ var _ = Describe("stree", func() {

Context("When empty", func() {
It("sets root properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(0),
testing.SimpleTime(9), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expect(s.root.depth).To(Equal(0))
})

It("sets root properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(0),
testing.SimpleTime(49), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expect(s.root.depth).To(Equal(1))
})

It("sets root properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(10),
testing.SimpleTime(109), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expect(s.root.depth).To(Equal(2))
})

It("sets root properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(10),
testing.SimpleTime(19), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expand All @@ -126,7 +123,7 @@ var _ = Describe("stree", func() {
})

It("sets root properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(10),
testing.SimpleTime(19), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expand All @@ -139,7 +136,7 @@ var _ = Describe("stree", func() {
})

It("sets root properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(10),
testing.SimpleTime(19), 10, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expand All @@ -152,7 +149,7 @@ var _ = Describe("stree", func() {
})

It("sets root properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(10),
testing.SimpleTime(19), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expand All @@ -171,7 +168,7 @@ var _ = Describe("stree", func() {
})

It("sets root properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(30),
testing.SimpleTime(39), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expand All @@ -191,7 +188,7 @@ var _ = Describe("stree", func() {
})

It("works with 3 mins", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(10),
testing.SimpleTime(70), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expect(s.root).ToNot(BeNil())
Expand All @@ -200,7 +197,7 @@ var _ = Describe("stree", func() {
})

It("sets trie properly, gets work", func() {
s := New(r, m)
s := New()

s.Put(testing.SimpleTime(0),
testing.SimpleTime(9), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
Expand Down
8 changes: 4 additions & 4 deletions pkg/storage/segment/serialization.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ func (s *Segment) Serialize(w io.Writer) error {
return nil
}

func Deserialize(resolution time.Duration, multiplier int, r io.Reader) (*Segment, error) {
s := New(resolution, multiplier)
func Deserialize(r io.Reader) (*Segment, error) {
s := New()
br := bufio.NewReader(r) // TODO if it's already a bytereader skip

// reads serialization format version, see comment at the top
Expand Down Expand Up @@ -155,8 +155,8 @@ func (t *Segment) Bytes() []byte {
return b.Bytes()
}

func FromBytes(resolution time.Duration, multiplier int, p []byte) *Segment {
func FromBytes(p []byte) *Segment {
// TODO: handle error
t, _ := Deserialize(resolution, multiplier, bytes.NewReader(p))
t, _ := Deserialize(bytes.NewReader(p))
return t
}
13 changes: 5 additions & 8 deletions pkg/storage/segment/serialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,9 @@ var serializedExampleV2 = "\x02={\"aggregationType\":\"\",\"sampleRate\":0,\"spy
"\x00\x8a\x92\xb8Ø\xfe\xff\xff\xff\x01\x01\x01\x01\x00\x00\x94\x92\xb8Ø\xfe\xff\xff\xff\x01\x01\x01\x01\x00"

var _ = Describe("stree", func() {
r := 10 * time.Second
m := 10

Context("Serialize / Deserialize", func() {
It("both functions work properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(0),
testing.SimpleTime(9), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
s.Put(testing.SimpleTime(10),
Expand All @@ -39,7 +36,7 @@ var _ = Describe("stree", func() {
serialized := buf.Bytes()
log.Printf("%q", serialized)

s, err := Deserialize(r, m, bytes.NewReader(serialized))
s, err := Deserialize(bytes.NewReader(serialized))
Expect(err).ToNot(HaveOccurred())
var buf2 bytes.Buffer
s.Serialize(&buf2)
Expand All @@ -50,7 +47,7 @@ var _ = Describe("stree", func() {

Context("Serialize", func() {
It("serializes segment tree properly", func() {
s := New(r, m)
s := New()
s.Put(testing.SimpleTime(0),
testing.SimpleTime(9), 1, func(de int, t time.Time, r *big.Rat, a []Addon) {})
s.Put(testing.SimpleTime(10),
Expand All @@ -69,7 +66,7 @@ var _ = Describe("stree", func() {
Context("Deserialize", func() {
Context("v1", func() {
It("deserializes v1 data", func() {
s, err := Deserialize(r, m, bytes.NewReader([]byte(serializedExampleV1)))
s, err := Deserialize(bytes.NewReader([]byte(serializedExampleV1)))
Expect(err).ToNot(HaveOccurred())
Expect(s.root.children[0]).ToNot(BeNil())
Expect(s.root.children[1]).ToNot(BeNil())
Expand All @@ -79,7 +76,7 @@ var _ = Describe("stree", func() {
})
Context("v2", func() {
It("deserializes v2 data", func() {
s, err := Deserialize(r, m, bytes.NewReader([]byte(serializedExampleV2)))
s, err := Deserialize(bytes.NewReader([]byte(serializedExampleV2)))
Expect(err).ToNot(HaveOccurred())
Expect(s.root.children[0]).ToNot(BeNil())
Expect(s.root.children[1]).ToNot(BeNil())
Expand Down
Loading