diff --git a/.ci b/.ci index eb157f4..4d1161c 160000 --- a/.ci +++ b/.ci @@ -1 +1 @@ -Subproject commit eb157f4dee7d8074ffce5e8885e85a459bd378a1 +Subproject commit 4d1161c20a1e934d8efcc95ab130e6d1b0917398 diff --git a/.excludecoverage b/.excludecoverage new file mode 100644 index 0000000..659f6e6 --- /dev/null +++ b/.excludecoverage @@ -0,0 +1,7 @@ +_mock.go +_gen.go +_matcher.go +generated/ +tools/ +vendor/ +integration/ diff --git a/.excludelint b/.excludelint index 5093d7e..1008443 100644 --- a/.excludelint +++ b/.excludelint @@ -1,3 +1,5 @@ (vendor/) (generated/) (_mock.go) +(_gen.go) +(_string.go) diff --git a/.excludemetalint b/.excludemetalint index 607d2fa..069e146 100644 --- a/.excludemetalint +++ b/.excludemetalint @@ -1,3 +1,4 @@ vendor/ generated/ _mock.go +_gen.go diff --git a/.metalinter.json b/.metalinter.json index a037198..02f2dda 100644 --- a/.metalinter.json +++ b/.metalinter.json @@ -1,20 +1,32 @@ { "Linters": { - "badtime": {"Command": "badtime", "Pattern": "PATH:LINE:COL:MESSAGE"}, - "deadcode": { "Command": "deadcode -tags integration" }, - "varcheck": { "Command": "varcheck -tags integration" }, - "megacheck": { "Command": "megacheck -tags integration" } }, - "Enable": - [ "deadcode" - , "varcheck" - , "structcheck" - , "goconst" - , "ineffassign" - , "unconvert" - , "misspell" - , "unparam" - , "badtime" - , "megacheck" ], + "badtime": { + "Command": "badtime", + "Pattern": "PATH:LINE:COL:MESSAGE" + }, + "deadcode": { + "Command": "deadcode -tags integration" + }, + "varcheck": { + "Command": "varcheck -tags integration" + }, + "megacheck": { + "Command": "megacheck -tags integration" + } + }, + "Enable": [ + "golint", + "deadcode", + "varcheck", + "structcheck", + "goconst", + "ineffassign", + "unconvert", + "misspell", + "unparam", + "badtime", + "megacheck" + ], "Deadline": "3m", "EnableGC": true } diff --git a/.travis.yml b/.travis.yml index 446611e..f855743 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,12 +1,16 @@ language: go go: - - 1.8.3 - - 1.9.2 + - "1.9.x" + - "1.10.x" install: make install-ci -env: - # Set higher timeouts and package name for travis - - TEST_TIMEOUT_SCALE=20 PACKAGE=github.com/m3db/m3aggregator sudo: required dist: trusty -script: - - make all +env: + global: + - TEST_TIMEOUT_SCALE=20 PACKAGE=github.com/m3db/m3aggregator + matrix: + - MAKE_TARGET="test-ci-unit" + - MAKE_TARGET="test-ci-integration" + - MAKE_TARGET="metalint" + - MAKE_TARGET="test-genny-all" +script: "make $MAKE_TARGET" diff --git a/Makefile b/Makefile index b3be877..5774e50 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,6 @@ coverfile := cover.out coverage_xml := coverage.xml junit_xml := junit.xml test_log := test.log -lint_check := .ci/lint.sh metalint_check := .ci/metalint.sh metalint_config := .metalinter.json metalint_exclude := .excludemetalint @@ -28,6 +27,8 @@ auto_gen := .ci/auto-gen.sh license_dir := .ci/uber-licence license_node_modules := $(license_dir)/node_modules +include $(SELF_DIR)/generated-source-files.mk + BUILD := $(abspath ./bin) LINUX_AMD64_ENV := GOOS=linux GOARCH=amd64 CGO_ENABLED=0 @@ -60,11 +61,6 @@ services-linux-amd64: $(foreach SERVICE,$(SERVICES),$(eval $(SERVICE_RULES))) -.PHONY: lint -lint: - @which golint > /dev/null || go get -u github.com/golang/lint/golint - $(lint_check) - .PHONY: metalint metalint: install-metalinter install-linter-badtime @($(metalint_check) $(metalint_config) $(metalint_exclude) && echo "metalinted successfully!") || (echo "metalinter failed" && exit 1) @@ -139,7 +135,7 @@ clean: @rm -f *.html *.xml *.out *.test .PHONY: all -all: lint metalint test-ci-unit test-ci-integration m3aggregator +all: metalint test-ci-unit test-ci-integration test-genny-all m3aggregator @echo Made all successfully .DEFAULT_GOAL := all diff --git a/aggregator/aggregator_test.go b/aggregator/aggregator_test.go index fe89910..1442a0e 100644 --- a/aggregator/aggregator_test.go +++ b/aggregator/aggregator_test.go @@ -283,7 +283,7 @@ func TestAggregatorAddMetricWithPoliciesListSuccessNoPlacementUpdate(t *testing. agg.shardFn = func([]byte, int) uint32 { return 1 } err := agg.AddMetricWithPoliciesList(testValidMetric, testPoliciesList) require.NoError(t, err) - require.Equal(t, 1, len(agg.shards[1].metricMap.entries)) + require.Equal(t, 1, agg.shards[1].metricMap.entries.Len()) } func TestAggregatorAddMetricWithPoliciesListSuccessWithPlacementUpdate(t *testing.T) { @@ -347,7 +347,7 @@ func TestAggregatorAddMetricWithPoliciesListSuccessWithPlacementUpdate(t *testin require.Equal(t, expected.latestNanos, agg.shards[i].latestWriteableNanos) } } - require.Equal(t, 1, len(agg.shards[1].metricMap.entries)) + require.Equal(t, 1, agg.shards[1].metricMap.entries.Len()) require.Equal(t, newPlacementCutoverNanos, agg.currPlacement.CutoverNanos()) for { diff --git a/aggregator/entry_map_gen.go b/aggregator/entry_map_gen.go new file mode 100644 index 0000000..4dc46da --- /dev/null +++ b/aggregator/entry_map_gen.go @@ -0,0 +1,251 @@ +// This file was automatically generated by genny. +// Any changes will be lost if this file is regenerated. +// see https://github.com/mauricelam/genny + +package aggregator + +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +// entryMapHash is the hash for a given map entry, this is public to support +// iterating over the map using a native Go for loop. +type entryMapHash uint64 + +// entryMapHashFn is the hash function to execute when hashing a key. +type entryMapHashFn func(entryKey) entryMapHash + +// entryMapEqualsFn is the equals key function to execute when detecting equality of a key. +type entryMapEqualsFn func(entryKey, entryKey) bool + +// entryMapCopyFn is the copy key function to execute when copying the key. +type entryMapCopyFn func(entryKey) entryKey + +// entryMapFinalizeFn is the finalize key function to execute when finished with a key. +type entryMapFinalizeFn func(entryKey) + +// entryMap uses the genny package to provide a generic hash map that can be specialized +// by running the following command from this root of the repository: +// ``` +// make hashmap-gen pkg=outpkg key_type=Type value_type=Type out_dir=/tmp +// ``` +// Or if you would like to use bytes or ident.ID as keys you can use the +// partially specialized maps to generate your own maps as well: +// ``` +// make byteshashmap-gen pkg=outpkg value_type=Type out_dir=/tmp +// make idhashmap-gen pkg=outpkg value_type=Type out_dir=/tmp +// ``` +// This will output to stdout the generated source file to use for your map. +// It uses linear probing by incrementing the number of the hash created when +// hashing the identifier if there is a collision. +// entryMap is a value type and not an interface to allow for less painful +// upgrades when adding/removing methods, it is not likely to need mocking so +// an interface would not be super useful either. +type entryMap struct { + _entryMapOptions + + // lookup uses hash of the identifier for the key and the MapEntry value + // wraps the value type and the key (used to ensure lookup is correct + // when dealing with collisions), we use uint64 for the hash partially + // because lookups of maps with uint64 keys has a fast path for Go. + lookup map[entryMapHash]entryMapEntry +} + +// _entryMapOptions is a set of options used when creating an identifier map, it is kept +// private so that implementers of the generated map can specify their own options +// that partially fulfill these options. +type _entryMapOptions struct { + // hash is the hash function to execute when hashing a key. + hash entryMapHashFn + // equals is the equals key function to execute when detecting equality. + equals entryMapEqualsFn + // copy is the copy key function to execute when copying the key. + copy entryMapCopyFn + // finalize is the finalize key function to execute when finished with a + // key, this is optional to specify. + finalize entryMapFinalizeFn + // initialSize is the initial size for the map, use zero to use Go's std map + // initial size and consequently is optional to specify. + initialSize int +} + +// entryMapEntry is an entry in the map, this is public to support iterating +// over the map using a native Go for loop. +type entryMapEntry struct { + // key is used to check equality on lookups to resolve collisions + key _entryMapKey + // value type stored + value elementPtr +} + +type _entryMapKey struct { + key entryKey + finalize bool +} + +// Key returns the map entry key. +func (e entryMapEntry) Key() entryKey { + return e.key.key +} + +// Value returns the map entry value. +func (e entryMapEntry) Value() elementPtr { + return e.value +} + +// _entryMapAlloc is a non-exported function so that when generating the source code +// for the map you can supply a public constructor that sets the correct +// hash, equals, copy, finalize options without users of the map needing to +// implement them themselves. +func _entryMapAlloc(opts _entryMapOptions) *entryMap { + m := &entryMap{_entryMapOptions: opts} + m.Reallocate() + return m +} + +func (m *entryMap) newMapKey(k entryKey, opts _entryMapKeyOptions) _entryMapKey { + key := _entryMapKey{key: k, finalize: opts.finalizeKey} + if !opts.copyKey { + return key + } + + key.key = m.copy(k) + return key +} + +func (m *entryMap) removeMapKey(hash entryMapHash, key _entryMapKey) { + delete(m.lookup, hash) + if key.finalize { + m.finalize(key.key) + } +} + +// Get returns a value in the map for an identifier if found. +func (m *entryMap) Get(k entryKey) (elementPtr, bool) { + hash := m.hash(k) + for entry, ok := m.lookup[hash]; ok; entry, ok = m.lookup[hash] { + if m.equals(entry.key.key, k) { + return entry.value, true + } + // Linear probe to "next" to this entry (really a rehash) + hash++ + } + var empty elementPtr + return empty, false +} + +// Set will set the value for an identifier. +func (m *entryMap) Set(k entryKey, v elementPtr) { + m.set(k, v, _entryMapKeyOptions{ + copyKey: true, + finalizeKey: m.finalize != nil, + }) +} + +// entryMapSetUnsafeOptions is a set of options to use when setting a value with +// the SetUnsafe method. +type entryMapSetUnsafeOptions struct { + NoCopyKey bool + NoFinalizeKey bool +} + +// SetUnsafe will set the value for an identifier with unsafe options for how +// the map treats the key. +func (m *entryMap) SetUnsafe(k entryKey, v elementPtr, opts entryMapSetUnsafeOptions) { + m.set(k, v, _entryMapKeyOptions{ + copyKey: !opts.NoCopyKey, + finalizeKey: !opts.NoFinalizeKey, + }) +} + +type _entryMapKeyOptions struct { + copyKey bool + finalizeKey bool +} + +func (m *entryMap) set(k entryKey, v elementPtr, opts _entryMapKeyOptions) { + hash := m.hash(k) + for entry, ok := m.lookup[hash]; ok; entry, ok = m.lookup[hash] { + if m.equals(entry.key.key, k) { + m.lookup[hash] = entryMapEntry{ + key: entry.key, + value: v, + } + return + } + // Linear probe to "next" to this entry (really a rehash) + hash++ + } + + m.lookup[hash] = entryMapEntry{ + key: m.newMapKey(k, opts), + value: v, + } +} + +// Iter provides the underlying map to allow for using a native Go for loop +// to iterate the map, however callers should only ever read and not write +// the map. +func (m *entryMap) Iter() map[entryMapHash]entryMapEntry { + return m.lookup +} + +// Len returns the number of map entries in the map. +func (m *entryMap) Len() int { + return len(m.lookup) +} + +// Contains returns true if value exists for key, false otherwise, it is +// shorthand for a call to Get that doesn't return the value. +func (m *entryMap) Contains(k entryKey) bool { + _, ok := m.Get(k) + return ok +} + +// Delete will remove a value set in the map for the specified key. +func (m *entryMap) Delete(k entryKey) { + hash := m.hash(k) + for entry, ok := m.lookup[hash]; ok; entry, ok = m.lookup[hash] { + if m.equals(entry.key.key, k) { + m.removeMapKey(hash, entry.key) + return + } + // Linear probe to "next" to this entry (really a rehash) + hash++ + } +} + +// Reset will reset the map by simply deleting all keys to avoid +// allocating a new map. +func (m *entryMap) Reset() { + for hash, entry := range m.lookup { + m.removeMapKey(hash, entry.key) + } +} + +// Reallocate will avoid deleting all keys and reallocate a new +// map, this is useful if you believe you have a large map and +// will not need to grow back to a similar size. +func (m *entryMap) Reallocate() { + if m.initialSize > 0 { + m.lookup = make(map[entryMapHash]entryMapEntry, m.initialSize) + } else { + m.lookup = make(map[entryMapHash]entryMapEntry) + } +} diff --git a/hash/hash.go b/aggregator/entry_new_map.go similarity index 67% rename from hash/hash.go rename to aggregator/entry_new_map.go index c78a0dc..04962cb 100644 --- a/hash/hash.go +++ b/aggregator/entry_new_map.go @@ -18,19 +18,12 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -// Package hash is a temporary measure used as in between while m3aggregator -// is upgraded to use native source generated maps now accessible in m3x, -// these types are missing from m3x when native source generated maps -// were added. -package hash +package aggregator -import "github.com/spaolacci/murmur3" - -// Hash128 is a 128-bit hash of an ID consisting of two unsigned 64-bit ints. -type Hash128 [2]uint64 - -// Murmur3Hash128 computes the 128-bit hash of an id. -func Murmur3Hash128(data []byte) Hash128 { - h0, h1 := murmur3.Sum128(data) - return Hash128{h0, h1} +func newEntryMap() *entryMap { + return _entryMapAlloc(_entryMapOptions{ + hash: func(k entryKey) entryMapHash { return entryMapHash(k.Hash()) }, + equals: func(x, y entryKey) bool { return x.Equal(y) }, + copy: func(k entryKey) entryKey { return k.Clone() }, + }) } diff --git a/aggregator/map.go b/aggregator/map.go index 5c5fedd..e34ff9b 100644 --- a/aggregator/map.go +++ b/aggregator/map.go @@ -21,15 +21,17 @@ package aggregator import ( + "bytes" "container/list" "errors" "math" "sync" "time" - "github.com/m3db/m3aggregator/hash" + "github.com/cespare/xxhash" "github.com/m3db/m3aggregator/rate" "github.com/m3db/m3aggregator/runtime" + "github.com/m3db/m3metrics/metric/id" "github.com/m3db/m3metrics/metric/unaggregated" "github.com/m3db/m3metrics/policy" "github.com/m3db/m3x/clock" @@ -51,7 +53,29 @@ var ( type entryKey struct { metricType unaggregated.Type - idHash hash.Hash128 + metricID id.RawID +} + +func (k entryKey) Hash() uint64 { + // Similar to the standard composite key hashes for Java objects. + hash := uint64(7) + hash = 31*hash + xxhash.Sum64(k.metricID) + hash = 31*hash + uint64(k.metricType) + return hash +} + +func (k entryKey) Equal(other entryKey) bool { + return k.metricType == other.metricType && bytes.Equal(k.metricID, other.metricID) +} + +func (k entryKey) Clone() entryKey { + idLen := len(k.metricID) + copiedID := make(id.RawID, idLen) + copy(copiedID, k.metricID) + return entryKey{ + metricType: k.metricType, + metricID: copiedID, + } } type hashedEntry struct { @@ -75,6 +99,8 @@ func newMetricMapMetrics(scope tally.Scope) metricMapMetrics { } } +type elementPtr *list.Element + // NB(xichen): use a type-specific list for hashedEntry if the conversion // overhead between interface{} and hashedEntry becomes a problem. type metricMap struct { @@ -88,7 +114,7 @@ type metricMap struct { closed bool metricLists *metricLists - entries map[entryKey]*list.Element + entries *entryMap entryList *list.List entryListDelLock sync.Mutex // Must be held when deleting elements from the entry list firstInsertAt time.Time @@ -109,7 +135,7 @@ func newMetricMap(shard uint32, opts Options) *metricMap { entryPool: opts.EntryPool(), batchPercent: opts.EntryCheckBatchPercent(), metricLists: metricLists, - entries: make(map[entryKey]*list.Element), + entries: newEntryMap(), entryList: list.New(), sleepFn: time.Sleep, metrics: newMetricMapMetrics(scope), @@ -132,11 +158,11 @@ func (m *metricMap) AddMetricWithPoliciesList( mu unaggregated.MetricUnion, pl policy.PoliciesList, ) error { - entryKey := entryKey{ + key := entryKey{ metricType: mu.Type, - idHash: hash.Murmur3Hash128(mu.ID), + metricID: mu.ID, } - entry, err := m.findOrCreate(entryKey) + entry, err := m.findOrCreate(key) if err != nil { return err } @@ -232,10 +258,15 @@ func (m *metricMap) findOrCreate(key entryKey) (*Entry, error) { } entry = m.entryPool.Get() entry.ResetSetData(m.metricLists, m.runtimeOpts, m.opts) - m.entries[key] = m.entryList.PushBack(hashedEntry{ - key: key, + clonedKey := key.Clone() + elem := m.entryList.PushBack(hashedEntry{ + key: clonedKey, entry: entry, }) + m.entries.SetUnsafe(clonedKey, elem, entryMapSetUnsafeOptions{ + NoCopyKey: true, + NoFinalizeKey: true, + }) entry.IncWriter() m.Unlock() m.metrics.newEntries.Inc(1) @@ -244,7 +275,7 @@ func (m *metricMap) findOrCreate(key entryKey) (*Entry, error) { } func (m *metricMap) lookupEntryWithLock(key entryKey) (*Entry, bool) { - elem, exists := m.entries[key] + elem, exists := m.entries.Get(key) if !exists { return nil, false } @@ -305,8 +336,9 @@ func (m *metricMap) purgeExpired(now time.Time, entries []hashedEntry) int { m.Lock() for i := range entries { if entries[i].entry.TryExpire(now) { - elem := m.entries[entries[i].key] - delete(m.entries, entries[i].key) + key := entries[i].key + elem, _ := m.entries.Get(key) + m.entries.Delete(key) elem.Value = nil m.entryList.Remove(elem) numExpired++ diff --git a/aggregator/map_test.go b/aggregator/map_test.go index ecf0d97..b306c4f 100644 --- a/aggregator/map_test.go +++ b/aggregator/map_test.go @@ -26,7 +26,6 @@ import ( "testing" "time" - "github.com/m3db/m3aggregator/hash" "github.com/m3db/m3aggregator/runtime" "github.com/m3db/m3metrics/aggregation" "github.com/m3db/m3metrics/metric/id" @@ -54,6 +53,143 @@ var ( } ) +func TestEntryKeyHash(t *testing.T) { + testEntry := entryKey{ + metricType: unaggregated.CounterType, + metricID: []byte("testCounter"), + } + require.Equal(t, uint64(0x6ed72ddde0e10187), testEntry.Hash()) +} + +func TestEntryKeyHashEqual(t *testing.T) { + inputs := []struct { + first entryKey + second entryKey + }{ + { + first: entryKey{ + metricType: unaggregated.CounterType, + metricID: []byte("testCounter"), + }, + second: entryKey{ + metricType: unaggregated.CounterType, + metricID: []byte("testCounter"), + }, + }, + { + first: entryKey{ + metricType: unaggregated.BatchTimerType, + metricID: []byte("testBatchTimer"), + }, + second: entryKey{ + metricType: unaggregated.BatchTimerType, + metricID: []byte("testBatchTimer"), + }, + }, + { + first: entryKey{ + metricType: unaggregated.GaugeType, + metricID: []byte("testGauge"), + }, + second: entryKey{ + metricType: unaggregated.GaugeType, + metricID: []byte("testGauge"), + }, + }, + } + + for _, input := range inputs { + require.True(t, input.first.Equal(input.second)) + require.True(t, input.second.Equal(input.first)) + } +} + +func TestEntryKeyHashNotEqual(t *testing.T) { + inputs := []struct { + first entryKey + second entryKey + }{ + { + first: entryKey{ + metricType: unaggregated.CounterType, + metricID: []byte("foo"), + }, + second: entryKey{ + metricType: unaggregated.BatchTimerType, + metricID: []byte("foo"), + }, + }, + { + first: entryKey{ + metricType: unaggregated.BatchTimerType, + metricID: []byte("foo"), + }, + second: entryKey{ + metricType: unaggregated.BatchTimerType, + metricID: []byte("bar"), + }, + }, + } + + for _, input := range inputs { + require.False(t, input.first.Equal(input.second)) + require.False(t, input.second.Equal(input.first)) + } +} + +func TestEntryKeyClone(t *testing.T) { + testEntry := entryKey{ + metricType: unaggregated.CounterType, + metricID: []byte("foo"), + } + clonedEntry := testEntry.Clone() + require.Equal(t, testEntry, clonedEntry) + + testEntry.metricType = unaggregated.GaugeType + testEntry.metricID[0] = 'b' + require.NotEqual(t, testEntry, clonedEntry) + + require.Equal(t, entryKey{ + metricType: unaggregated.CounterType, + metricID: []byte("foo"), + }, clonedEntry) +} + +func TestEntryMapMetricInsertionDeletion(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + testMetric := unaggregated.MetricUnion{ + Type: unaggregated.CounterType, + ID: id.RawID("foo"), + CounterVal: 1234, + } + opts := testOptions(ctrl) + m := newMetricMap(testShard, opts) + require.NoError(t, m.AddMetricWithPoliciesList(testMetric, testPoliciesList)) + + // Validate that the key exists in the map. + key := entryKey{ + metricType: unaggregated.CounterType, + metricID: id.RawID("foo"), + } + val, exists := m.entries.Get(key) + require.True(t, exists) + require.Equal(t, key, val.Value.(hashedEntry).key) + + // Validate that the metric ID is cloned. + testMetric.Type = unaggregated.BatchTimerType + testMetric.ID[0] = 'b' + val, exists = m.entries.Get(key) + require.True(t, exists) + require.Equal(t, key, val.Value.(hashedEntry).key) + + // Delete the metric and verify the metric is then removed. + m.entries.Delete(key) + _, exists = m.entries.Get(key) + require.False(t, exists) +} + func TestMetricMapAddMetricWithPoliciesListMapClosed(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -76,13 +212,13 @@ func TestMetricMapAddMetricWithPoliciesListNoRateLimit(t *testing.T) { // Add a counter metric and assert there is one entry afterwards. key := entryKey{ metricType: unaggregated.CounterType, - idHash: hash.Murmur3Hash128(testCounterID), + metricID: testCounterID, } require.NoError(t, m.AddMetricWithPoliciesList(testCounter, policies)) - require.Equal(t, 1, len(m.entries)) + require.Equal(t, 1, m.entries.Len()) require.Equal(t, 1, m.entryList.Len()) - elem, exists := m.entries[key] + elem, exists := m.entries.Get(key) require.True(t, exists) entry := elem.Value.(hashedEntry) require.Equal(t, int32(0), atomic.LoadInt32(&entry.entry.numWriters)) @@ -91,9 +227,9 @@ func TestMetricMapAddMetricWithPoliciesListNoRateLimit(t *testing.T) { // Add the same counter and assert there is still one entry. require.NoError(t, m.AddMetricWithPoliciesList(testCounter, policies)) - require.Equal(t, 1, len(m.entries)) + require.Equal(t, 1, m.entries.Len()) require.Equal(t, 1, m.entryList.Len()) - elem2, exists := m.entries[key] + elem2, exists := m.entries.Get(key) require.True(t, exists) entry2 := elem2.Value.(hashedEntry) require.Equal(t, entry, entry2) @@ -104,7 +240,7 @@ func TestMetricMapAddMetricWithPoliciesListNoRateLimit(t *testing.T) { // now two entries. key2 := entryKey{ metricType: unaggregated.GaugeType, - idHash: hash.Murmur3Hash128(testCounterID), + metricID: testCounterID, } metricWithDifferentType := unaggregated.MetricUnion{ Type: unaggregated.GaugeType, @@ -115,11 +251,11 @@ func TestMetricMapAddMetricWithPoliciesListNoRateLimit(t *testing.T) { metricWithDifferentType, testCustomPoliciesList, )) - require.Equal(t, 2, len(m.entries)) + require.Equal(t, 2, m.entries.Len()) require.Equal(t, 2, m.entryList.Len()) require.Equal(t, 3, m.metricLists.Len()) - e1, exists1 := m.entries[key] - e2, exists2 := m.entries[key2] + e1, exists1 := m.entries.Get(key) + e2, exists2 := m.entries.Get(key2) require.True(t, exists1) require.True(t, exists2) require.NotEqual(t, e1, e2) @@ -134,7 +270,7 @@ func TestMetricMapAddMetricWithPoliciesListNoRateLimit(t *testing.T) { metricWithDifferentID, testCustomPoliciesList, )) - require.Equal(t, 3, len(m.entries)) + require.Equal(t, 3, m.entries.Len()) require.Equal(t, 3, m.entryList.Len()) require.Equal(t, 3, m.metricLists.Len()) } @@ -280,18 +416,18 @@ func TestMetricMapDeleteExpired(t *testing.T) { for i := 0; i < numEntries; i++ { key := entryKey{ metricType: unaggregated.CounterType, - idHash: hash.Murmur3Hash128([]byte(fmt.Sprintf("%d", i))), + metricID: []byte(fmt.Sprintf("%d", i)), } if i%2 == 0 { - m.entries[key] = m.entryList.PushBack(hashedEntry{ + m.entries.Set(key, m.entryList.PushBack(hashedEntry{ key: key, entry: NewEntry(m.metricLists, runtime.NewOptions(), liveEntryOpts), - }) + })) } else { - m.entries[key] = m.entryList.PushBack(hashedEntry{ + m.entries.Set(key, m.entryList.PushBack(hashedEntry{ key: key, entry: NewEntry(m.metricLists, runtime.NewOptions(), expiredEntryOpts), - }) + })) } } @@ -299,12 +435,13 @@ func TestMetricMapDeleteExpired(t *testing.T) { m.deleteExpired(opts.EntryCheckInterval()) // Assert there should be only half of the entries left. - require.Equal(t, numEntries/2, len(m.entries)) + require.Equal(t, numEntries/2, m.entries.Len()) require.Equal(t, numEntries/2, m.entryList.Len()) require.Equal(t, len(sleepIntervals), numEntries/defaultSoftDeadlineCheckEvery) - for k, v := range m.entries { - e := v.Value.(hashedEntry) - require.Equal(t, k, e.key) + for _, v := range m.entries.Iter() { + elem := v.Value() + e := elem.Value.(hashedEntry) + require.Equal(t, v.Key(), e.key) require.NotNil(t, e.entry) } } diff --git a/generated-source-files.mk b/generated-source-files.mk new file mode 100644 index 0000000..4d2fc35 --- /dev/null +++ b/generated-source-files.mk @@ -0,0 +1,47 @@ +m3x_package := github.com/m3db/m3x +m3x_package_path := $(gopath_prefix)/$(m3x_package) +m3x_package_min_ver := 29bc232d9ad2e6c4a1804ccce095bb730fb1c6bc +m3aggregator_package_path := $(gopath_prefix)/$(package_root) + +.PHONY: install-m3x-repo +install-m3x-repo: install-glide install-generics-bin + # Check if repository exists, if not get it + test -d $(m3x_package_path) || go get -u $(m3x_package) + test -d $(m3x_package_path)/vendor || (cd $(m3x_package_path) && glide install) + test "$(shell cd $(m3x_package_path) && git diff --shortstat 2>/dev/null)" = "" || ( \ + echo "WARNING: m3x repository is dirty, generated files might not be as expected" \ + ) + # If does exist but not at min version then update it + (cd $(m3x_package_path) && git cat-file -t $(m3x_package_min_ver) > /dev/null) || ( \ + echo "WARNING: m3x repository is below commit $(m3x_package_min_ver), generated files might not be as expected" \ + ) + +# Generation rule for all generated types +.PHONY: genny-all +genny-all: genny-map-all + +# Tests that all currently generated types match their contents if they were regenerated +.PHONY: test-genny-all +test-genny-all: test-genny-map-all + +# Map generation rule for all generated maps +.PHONY: genny-map-all +genny-map-all: genny-map-aggregator-entry + +# Tests that all currently generated maps match their contents if they were regenerated +.PHONY: test-genny-map-all +test-genny-map-all: genny-map-all + @test "$(shell git diff --shortstat 2>/dev/null)" = "" || (git diff --no-color && echo "Check git status, there are dirty files" && exit 1) + @test "$(shell git status --porcelain 2>/dev/null | grep "^??")" = "" || (git status --porcelain && echo "Check git status, there are untracked files" && exit 1) + +# Map generation rule for aggregator/entryMap +.PHONY: genny-map-aggregator-entry +genny-map-aggregator-entry: install-m3x-repo + cd $(m3x_package_path) && make hashmap-gen \ + pkg=aggregator \ + key_type=entryKey \ + value_type=elementPtr \ + out_dir=$(m3aggregator_package_path)/aggregator \ + rename_type_prefix=entry + # Rename both generated map and constructor files + mv -f $(m3aggregator_package_path)/aggregator/map_gen.go $(m3aggregator_package_path)/aggregator/entry_map_gen.go diff --git a/glide.lock b/glide.lock index 9a608c5..68b6134 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ hash: 7de644447e05a04b5b6d3f8d9232790ac65dc70dcadf2185c32641d46e1e8fcc -updated: 2018-04-19T13:46:10.784417-04:00 +updated: 2018-04-23T13:53:29.072857-04:00 imports: - name: github.com/apache/thrift version: 9549b25c77587b29be4e0b5c258221a4ed85d37a @@ -11,6 +11,8 @@ imports: - quantile - name: github.com/boltdb/bolt version: 583e8937c61f1af6513608ccc75c97b6abdf4ff9 +- name: github.com/cespare/xxhash + version: 48099fad606eafc26e3a569fad19ff510fff4df6 - name: github.com/cockroachdb/cmux version: 112f0506e7743d64a6eb8fedbcff13d9979bbf92 - name: github.com/coreos/bbolt @@ -195,9 +197,7 @@ imports: - clock - close - config - - context - errors - - ident - instrument - log - net @@ -271,7 +271,7 @@ imports: - name: go.uber.org/multierr version: 3c4937480c32f4c13a875a1829af76c98ca3d40a - name: go.uber.org/zap - version: 35aad584952c3e7020db7b839f6b102de6271f89 + version: eeedf312bc6c57391d84767a4cd413f02a917974 subpackages: - buffer - internal/bufferpool