From 64a12c939c17111907b28725eda4ef78ee291c0c Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 16 May 2017 17:56:16 -0500 Subject: [PATCH 01/14] Fix typo in error message --- pql/ast.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pql/ast.go b/pql/ast.go index cf404ad12..13a989b03 100644 --- a/pql/ast.go +++ b/pql/ast.go @@ -72,7 +72,7 @@ func (c *Call) UintArg(key string) (uint64, bool, error) { case uint64: return tval, true, nil default: - return 0, true, fmt.Errorf("could not convert %v of type %T to uint64 in Calll.UintArg", tval, tval) + return 0, true, fmt.Errorf("could not convert %v of type %T to uint64 in Call.UintArg", tval, tval) } } From db256c91ccd4af7b3556bac215d556bebfa9c895 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Tue, 16 May 2017 22:54:17 -0500 Subject: [PATCH 02/14] Only autocomplete on single match --- webui/assets/main.js | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/webui/assets/main.js b/webui/assets/main.js index 887181338..3969ed650 100644 --- a/webui/assets/main.js +++ b/webui/assets/main.js @@ -85,20 +85,27 @@ class REPL { } var input_word = repl.input.value.substring(word_start, repl.input.selectionEnd) - // check for keyword match and insert - // this just stops at the first match + // check for keyword match and insert if exactly one match + var matches = [] for(var keyword in keywords) { if(keyword.startsWith(input_word)){ - var cursor_pos = repl.input.selectionEnd - var completion = keyword.substring(input_word.length) - var before = repl.input.value.substring(0, cursor_pos) - var after = repl.input.value.substring(cursor_pos) - repl.input.value = before + completion + after - var new_pos = cursor_pos + completion.length - keywords[keyword] - repl.input.setSelectionRange(new_pos, new_pos) - break + matches.push(keyword) } } + if(matches.length > 1) { + // display in some dynamic element + } + + if(matches.length == 1) { + var cursor_pos = repl.input.selectionEnd + var completion = matches[0].substring(input_word.length) + var before = repl.input.value.substring(0, cursor_pos) + var after = repl.input.value.substring(cursor_pos) + repl.input.value = before + completion + after + var new_pos = cursor_pos + completion.length - keywords[matches[0]] + repl.input.setSelectionRange(new_pos, new_pos) + } + } }) repl.button.onclick = function() { From 39e46e2c483e1e669508ac35245263e499e6e575 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 18 May 2017 18:30:25 -0500 Subject: [PATCH 03/14] Factor out autocompleter --- webui/assets/main.js | 119 ++++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 53 deletions(-) diff --git a/webui/assets/main.js b/webui/assets/main.js index 3969ed650..eea168185 100644 --- a/webui/assets/main.js +++ b/webui/assets/main.js @@ -1,8 +1,9 @@ class REPL { - constructor(input, output, button) { + constructor(input, output, button, completer) { this.input = input this.output = output this.button = button + this.completer = completer this.history = [] this.history_index = 0 this.history_buffer = '' @@ -16,21 +17,6 @@ class REPL { UP_ARROW: 38, DOWN_ARROW: 40 } - var keywords = { - // keyword: length of substring that comes after cursor - "SetBit()": 1, - "ClearBit()": 1, - "SetRowAttrs()": 1, - "SetColumnAttrs()": 1, - "Bitmap()": 1, - "Union()": 1, - "Intersect()": 1, - "Difference()": 1, - "Count()": 1, - "Range()": 1, - "TopN()": 1, - "frame=": 0, - } this.input.addEventListener("keydown", function(e) { if (e.keyCode == keys.UP_ARROW) { @@ -70,42 +56,7 @@ class REPL { } if (e.keyCode == keys.TAB) { e.preventDefault() - - // extract word fragment ending at cursor. a word fragment: - // - starts with last nonalpha character before cursor (or beginning of string) - // - ends at cursor - var word_start = repl.input.selectionEnd-1 - while(word_start>0) { - var c = repl.input.value.charCodeAt(word_start) - if(!((c>64 && c<91) || (c>96 && c<123))) { - word_start++ - break - } - word_start-- - } - var input_word = repl.input.value.substring(word_start, repl.input.selectionEnd) - - // check for keyword match and insert if exactly one match - var matches = [] - for(var keyword in keywords) { - if(keyword.startsWith(input_word)){ - matches.push(keyword) - } - } - if(matches.length > 1) { - // display in some dynamic element - } - - if(matches.length == 1) { - var cursor_pos = repl.input.selectionEnd - var completion = matches[0].substring(input_word.length) - var before = repl.input.value.substring(0, cursor_pos) - var after = repl.input.value.substring(cursor_pos) - repl.input.value = before + completion + after - var new_pos = cursor_pos + completion.length - keywords[matches[0]] - repl.input.setSelectionRange(new_pos, new_pos) - } - + repl.completer.complete() } }) repl.button.onclick = function() { @@ -451,11 +402,73 @@ Date.prototype.timeNow = function () { populate_version() + +class Autocompleter { + constructor(input) { + this.input = input + this.keyword_map = this.static_keywords + } + + get static_keywords() { + return { + // keyword: length of substring that comes after cursor + "SetBit()": 1, + "ClearBit()": 1, + "SetRowAttrs()": 1, + "SetColumnAttrs()": 1, + "Bitmap()": 1, + "Union()": 1, + "Intersect()": 1, + "Difference()": 1, + "Count()": 1, + "Range()": 1, + "TopN()": 1, + "frame=": 0, + } + } + + complete() { + var completer = this + // extract word fragment ending at cursor. a word fragment: + // - starts with last nonalpha character before cursor (or beginning of string) + // - ends at cursor + var word_start = completer.input.selectionEnd-1 + while(word_start>0) { + var c = completer.input.value.charCodeAt(word_start) + if(!((c>64 && c<91) || (c>96 && c<123))) { + word_start++ + break + } + word_start-- + } + var input_word = completer.input.value.substring(word_start, completer.input.selectionEnd) + + // check for keyword match and insert if exactly one match + var matches = [] + for(var keyword in this.keyword_map) { + if(keyword.startsWith(input_word)){ + matches.push(keyword) + } + } + + if(matches.length == 1) { + var cursor_pos = completer.input.selectionEnd + var completion = matches[0].substring(input_word.length) + var before = completer.input.value.substring(0, cursor_pos) + var after = completer.input.value.substring(cursor_pos) + completer.input.value = before + completion + after + var new_pos = cursor_pos + completion.length - this.keyword_map[matches[0]] + completer.input.setSelectionRange(new_pos, new_pos) + } + } +} + var input = document.getElementById('query') var output = document.getElementById('outputs') var button = document.getElementById('query-btn') -repl = new REPL(input, output, button) +autocompleter = new Autocompleter(input) +repl = new REPL(input, output, button, autocompleter) repl.populate_index_dropdown() repl.bind_events() From 75c59387f686b288c5775993902cb1f469b632c5 Mon Sep 17 00:00:00 2001 From: Alan Bernstein Date: Thu, 18 May 2017 18:34:23 -0500 Subject: [PATCH 04/14] Skeleton for new autocomplete functionality --- webui/assets/main.js | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/webui/assets/main.js b/webui/assets/main.js index eea168185..db54a3b1e 100644 --- a/webui/assets/main.js +++ b/webui/assets/main.js @@ -404,9 +404,11 @@ populate_version() class Autocompleter { - constructor(input) { + constructor(input, output) { this.input = input + this.output = output this.keyword_map = this.static_keywords + this.init_dynamic_keywords() } get static_keywords() { @@ -450,8 +452,12 @@ class Autocompleter { matches.push(keyword) } } + if(matches.length > 1) { + // completer.output.innerHTML = whatever + } if(matches.length == 1) { + // completer.output.innerHTML = "" var cursor_pos = completer.input.selectionEnd var completion = matches[0].substring(input_word.length) var before = completer.input.value.substring(0, cursor_pos) @@ -461,13 +467,28 @@ class Autocompleter { completer.input.setSelectionRange(new_pos, new_pos) } } + + init_dynamic_keywords() { + // hit /schema, parse indexes, frames, rowlabels, columnlabels, add to list + } + + add_keyword() { + // call when index or frame created in webui + } + + remove_keyword() { + // call when index or frame deleted in webui + // issue: if e.g. multiple indexes have same frame, removing one removes all. + // solution: maintain count. requires more elaborate representation of keywords. + } } var input = document.getElementById('query') var output = document.getElementById('outputs') var button = document.getElementById('query-btn') +var autocomplete_output = document.getElementById('autocomplete-container') -autocompleter = new Autocompleter(input) +autocompleter = new Autocompleter(input, autocomplete_output) repl = new REPL(input, output, button, autocompleter) repl.populate_index_dropdown() repl.bind_events() From 1c09af132e8e55832df9566ab8922185d7c75aec Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Thu, 18 May 2017 22:35:24 -0600 Subject: [PATCH 05/14] Check for duplicate attributes under read lock on insert. Changes the behavior of `AttrStore.SetAttrs()` to first verify that the attributes haven't changed using a read-only lock before obtaining a write lock to update the attributes. Since the write lock serializes access, the previous insert time sufferred lock contention when inserting a lot of duplicate attributes. With the `RWMutex`, reads can be done in parallel so multiple requests don't block each other. Below is a simple benchmark showing the performance at 1, 5, & 10 goroutines: BenchmarkAttrStore_Duplicate 100000 137120 ns/op 8428 B/op 76 allocs/op BenchmarkAttrStore_Duplicate-5 100000 417459 ns/op 8431 B/op 76 allocs/op BenchmarkAttrStore_Duplicate-10 100000 139466 ns/op 8433 B/op 76 allocs/op BenchmarkAttrStore_Duplicate 20000000 703 ns/op 368 B/op 5 allocs/op BenchmarkAttrStore_Duplicate-5 100000000 213 ns/op 368 B/op 5 allocs/op BenchmarkAttrStore_Duplicate-10 100000000 223 ns/op 368 B/op 5 allocs/op --- attr.go | 30 +++++++++++++++++++++++++++--- attr_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/attr.go b/attr.go index 2e45aadf8..6f12ffdc0 100644 --- a/attr.go +++ b/attr.go @@ -41,7 +41,7 @@ const ( // AttrStore represents a storage layer for attributes. type AttrStore struct { - mu sync.Mutex + mu sync.RWMutex path string db *bolt.DB @@ -92,8 +92,8 @@ func (s *AttrStore) Close() error { // Attrs returns a set of attributes by ID. func (s *AttrStore) Attrs(id uint64) (m map[string]interface{}, err error) { - s.mu.Lock() - defer s.mu.Unlock() + s.mu.RLock() + defer s.mu.RUnlock() // Check cache for map. if m = s.attrs[id]; m != nil { @@ -119,6 +119,19 @@ func (s *AttrStore) Attrs(id uint64) (m map[string]interface{}, err error) { // SetAttrs sets attribute values for a given ID. func (s *AttrStore) SetAttrs(id uint64, m map[string]interface{}) error { + // Ignore empty maps. + if len(m) == 0 { + return nil + } + + // Check if the attributes already exist under a read-only lock. + if attr, err := s.Attrs(id); err != nil { + return err + } else if attr != nil && mapContains(attr, m) { + return nil + } + + // Obtain write lock. s.mu.Lock() defer s.mu.Unlock() @@ -493,3 +506,14 @@ func (cur *blockCursor) next() (key, value []byte) { return key, value } + +// mapContains returns true if all keys & values of subset are in m. +func mapContains(m, subset map[string]interface{}) bool { + for k, v := range subset { + value, ok := m[k] + if !ok || value != v { + return false + } + } + return true +} diff --git a/attr_test.go b/attr_test.go index dda280214..d9884cedc 100644 --- a/attr_test.go +++ b/attr_test.go @@ -18,6 +18,8 @@ import ( "io/ioutil" "os" "reflect" + "runtime" + "sync" "testing" "github.com/pilosa/pilosa" @@ -143,6 +145,38 @@ func NewAttrStore() *AttrStore { return &AttrStore{AttrStore: pilosa.NewAttrStore(f.Name())} } +func BenchmarkAttrStore_Duplicate(b *testing.B) { + s := MustOpenAttrStore() + defer s.Close() + + // Set attributes. + const n = 5 + for i := 0; i < n; i++ { + if err := s.SetAttrs(uint64(i), map[string]interface{}{"A": 100, "B": "foo", "C": true, "D": 100.2}); err != nil { + b.Fatal(err) + } + } + + b.ReportAllocs() + b.ResetTimer() + + // Update attributes with an existing subset. + cpuN := runtime.GOMAXPROCS(0) + var wg sync.WaitGroup + for i := 0; i < cpuN; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < b.N/cpuN; j++ { + if err := s.SetAttrs(uint64(j%n), map[string]interface{}{"A": int64(100), "B": "foo", "D": 100.2}); err != nil { + b.Fatal(err) + } + } + }() + } + wg.Wait() +} + // MustOpenAttrStore returns a new, opened attribute store at a temporary path. Panic on error. func MustOpenAttrStore() *AttrStore { s := NewAttrStore() From b386fc9eacdb59d0d49aa6a25ee75f7bcd3fe8dd Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Tue, 23 May 2017 09:45:21 -0500 Subject: [PATCH 06/14] discriminate error messages in server.Open --- server.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server.go b/server.go index 4f8f45653..fc8b3608f 100644 --- a/server.go +++ b/server.go @@ -101,7 +101,7 @@ func (s *Server) Open() error { // Open HTTP listener to determine port (if specified as :0). ln, err := net.Listen("tcp", ":"+port) if err != nil { - return err + return fmt.Errorf("net.Listen: %v", err) } s.ln = ln @@ -115,16 +115,16 @@ func (s *Server) Open() error { // Open holder. if err := s.Holder.Open(); err != nil { - return err + return fmt.Errorf("opening Holder: %v", err) } if err := s.BroadcastReceiver.Start(s); err != nil { - return err + return fmt.Errorf("starting BroadcastReceiver: %v", err) } // Open NodeSet communication if err := s.Cluster.NodeSet.Open(); err != nil { - return err + return fmt.Errorf("opening NodeSet: %v", err) } // Create executor for executing queries. From 0f45d27028ba2aca0dbe7397b3ec874eacc950c2 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Tue, 23 May 2017 14:40:58 -0500 Subject: [PATCH 07/14] #33 validate config --- cmd/root.go | 38 +++++++++++++++++++++++++++++++++++++- cmd/root_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index d50dd05b4..caea25992 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -22,6 +22,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" + "reflect" ) var ( @@ -109,6 +110,11 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error v.AutomaticEnv() c := v.GetString("config") + var flagErr error + validTags := make(map[string]bool) + flags.VisitAll(func(f *pflag.Flag) { + validTags[f.Name] = true + }) // add config file to viper if c != "" { @@ -118,10 +124,16 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error if err != nil { return fmt.Errorf("error reading configuration file '%s': %v", c, err) } + + for _, key := range v.AllKeys() { + if _, ok := validTags[key]; !ok { + return fmt.Errorf("invalid tag: %v", key) + } + } + } // set all values from viper - var flagErr error flags.VisitAll(func(f *pflag.Flag) { if flagErr != nil { return @@ -151,3 +163,27 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error }) return flagErr } + +func GetValidTags(v interface{}) map[string]bool { + validTag := make(map[string]bool) + conf := reflect.ValueOf(v) + + for i := 0; i < conf.Type().NumField(); i++ { + field := conf.Field(i) + if field.Kind() == reflect.Struct { + tag := conf.Type().Field(i).Tag.Get("toml") + tagString := strings.Split(tag, ",") + val := reflect.ValueOf(field.Interface()) + for j := 0; j < val.Type().NumField(); j++ { + subTag := val.Type().Field(j).Tag.Get("toml") + subTagString := strings.Split(subTag, ",") + validTag[fmt.Sprintf("%s.%s", tagString[0], subTagString[0])] = true + } + } else { + tomlTag := conf.Type().Field(i).Tag.Get("toml") + s := strings.Split(tomlTag, ",") + validTag[s[0]] = true + } + } + return validTag +} diff --git a/cmd/root_test.go b/cmd/root_test.go index 16bc82a8a..d3aeeab1f 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -171,3 +171,27 @@ func TestRootCommand(t *testing.T) { t.Fatalf("Expected standard usage message from RootCommand, but err: '%v', output: '%s'", err, outStr) } } + +func TestRootCommand_Config(t *testing.T) { + file, err := ioutil.TempFile("", "test.conf") + if err != nil { + panic(err) + } + config := `data-dir = "/tmp/pil5_0" +bind = "127.0.0.1:15000" + +[cluster] + poll-interval = "2m0s" + replicas = 2 + partitions = 128 + hosts = [ + "127.0.0.1:15000", + "127.0.0.1:15001", + ]` + file.Write([]byte(config)) + file.Close() + _, err = ExecNewRootCommand(t, "server", "--config", file.Name()) + if err.Error() != "invalid tag: cluster.partitions" { + t.Fatalf("Expected invalid tag, but err: '%v'", err) + } +} From 0a19e9f413a6b715b5864453f17586be849f47fb Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Tue, 23 May 2017 14:44:16 -0500 Subject: [PATCH 08/14] #33 don't need reflect for get valid field --- cmd/root.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index caea25992..e506f2fed 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -22,7 +22,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" - "reflect" ) var ( @@ -163,27 +162,3 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error }) return flagErr } - -func GetValidTags(v interface{}) map[string]bool { - validTag := make(map[string]bool) - conf := reflect.ValueOf(v) - - for i := 0; i < conf.Type().NumField(); i++ { - field := conf.Field(i) - if field.Kind() == reflect.Struct { - tag := conf.Type().Field(i).Tag.Get("toml") - tagString := strings.Split(tag, ",") - val := reflect.ValueOf(field.Interface()) - for j := 0; j < val.Type().NumField(); j++ { - subTag := val.Type().Field(j).Tag.Get("toml") - subTagString := strings.Split(subTag, ",") - validTag[fmt.Sprintf("%s.%s", tagString[0], subTagString[0])] = true - } - } else { - tomlTag := conf.Type().Field(i).Tag.Get("toml") - s := strings.Split(tomlTag, ",") - validTag[s[0]] = true - } - } - return validTag -} From 108c644938fadc62a8a0c304f8c85c0b08926ee3 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Wed, 24 May 2017 13:57:47 -0500 Subject: [PATCH 09/14] #312 validate unkown query params --- handler.go | 21 +++++++++++++++++++++ handler_test.go | 10 ++++++++++ 2 files changed, 31 insertions(+) diff --git a/handler.go b/handler.go index c4a98d04c..a2e8868f1 100644 --- a/handler.go +++ b/handler.go @@ -42,6 +42,7 @@ import ( _ "github.com/pilosa/pilosa/statik" "github.com/rakyll/statik/fs" + "unicode" ) // Handler represents an HTTP handler. @@ -843,6 +844,12 @@ func (h *Handler) readProtobufQueryRequest(r *http.Request) (*QueryRequest, erro // readURLQueryRequest parses query parameters from URL parameters from r. func (h *Handler) readURLQueryRequest(r *http.Request) (*QueryRequest, error) { q := r.URL.Query() + validQuery := h.getValidURLQuery(r) + for key, _ := range q { + if _, ok := validQuery[key]; !ok { + return nil, errors.New("invalid query params") + } + } // Parse query string. buf, err := ioutil.ReadAll(r.Body) @@ -875,6 +882,20 @@ func (h *Handler) readURLQueryRequest(r *http.Request) (*QueryRequest, error) { }, nil } +func (h *Handler) getValidURLQuery(r *http.Request) map[string]bool { + validQuery := make(map[string]bool) + args := reflect.ValueOf(QueryRequest{}) + + for i := 0; i < args.Type().NumField(); i++ { + fieldName := args.Type().Field(i).Name + chars := []rune(fieldName) + chars[0] = unicode.ToLower(chars[0]) + fieldName = string(chars) + validQuery[fieldName] = true + } + return validQuery +} + // writeQueryResponse writes the response from the executor to w. func (h *Handler) writeQueryResponse(w http.ResponseWriter, r *http.Request, resp *QueryResponse) error { if strings.Contains(r.Header.Get("Accept"), "application/x-protobuf") { diff --git a/handler_test.go b/handler_test.go index 6744e74ea..427ad043e 100644 --- a/handler_test.go +++ b/handler_test.go @@ -209,6 +209,16 @@ func TestHandler_Query_Args_Err(t *testing.T) { t.Fatalf("unexpected body: %q", body) } } +func TestHandler_Query_Params_Err(t *testing.T) { + w := httptest.NewRecorder() + NewHandler().ServeHTTP(w, MustNewHTTPRequest("POST", "/index/idx0/query?slices=0,1&db=sample", strings.NewReader("Bitmap(id=100)"))) + if w.Code != http.StatusBadRequest { + t.Fatalf("unexpected status code: %d", w.Code) + } else if body := w.Body.String(); body != `{"error":"invalid query params"}`+"\n" { + t.Fatalf("unexpected body: %q", body) + } + +} // Ensure the handler can execute a query with a uint64 response as JSON. func TestHandler_Query_Uint64_JSON(t *testing.T) { From 15853fb448b691fc6d7bd2f167f7c7eec489cd6f Mon Sep 17 00:00:00 2001 From: Yuce Tekol Date: Thu, 25 May 2017 15:49:55 +0300 Subject: [PATCH 10/14] Fix name, label validation off by one error; added tests --- pilosa.go | 4 ++-- pilosa_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 pilosa_test.go diff --git a/pilosa.go b/pilosa.go index a192364ed..13ade19ea 100644 --- a/pilosa.go +++ b/pilosa.go @@ -49,10 +49,10 @@ var ( ) // Regular expression to validate index and frame names. -var nameRegexp = regexp.MustCompile(`^[a-z][a-z0-9_-]{0,64}$`) +var nameRegexp = regexp.MustCompile(`^[a-z][a-z0-9_-]{0,63}$`) // Regular expression to validate row and column labels. -var labelRegexp = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_-]{0,64}$`) +var labelRegexp = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_-]{0,63}$`) // ColumnAttrSet represents a set of attributes for a vertical column in an index. // Can have a set of attributes attached to it. diff --git a/pilosa_test.go b/pilosa_test.go new file mode 100644 index 000000000..ad5f9b762 --- /dev/null +++ b/pilosa_test.go @@ -0,0 +1,55 @@ +package pilosa_test + +import ( + "testing" + + "github.com/pilosa/pilosa" +) + +func TestValidateName(t *testing.T) { + names := []string{ + "a", "ab", "ab1", "b-c", "d_e", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + } + for _, name := range names { + if pilosa.ValidateName(name) != nil { + t.Fatalf("Should be valid index name: %s", name) + } + } +} + +func TestValidateNameInvalid(t *testing.T) { + names := []string{ + "", "'", "^", "/", "\\", "A", "*", "a:b", "valid?no", "yüce", "1", "_", "-", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1", + } + for _, name := range names { + if pilosa.ValidateName(name) == nil { + t.Fatalf("Should be invalid index name: %s", name) + } + } +} + +func TestValidateLabel(t *testing.T) { + labels := []string{ + "a", "ab", "ab1", "d_e", "A", "Bc", "B1", "aB", "b-c", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + } + for _, label := range labels { + if pilosa.ValidateLabel(label) != nil { + t.Fatalf("Should be valid label: %s", label) + } + } +} + +func TestValidateLabelInvalid(t *testing.T) { + labels := []string{ + "", "1", "_", "-", "'", "^", "/", "\\", "*", "a:b", "valid?no", "yüce", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1", + } + for _, label := range labels { + if pilosa.ValidateLabel(label) == nil { + t.Fatalf("Should be invalid label: %s", label) + } + } +} From e34e9f9789a3fbb2266413298c03362f04a0d046 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Thu, 25 May 2017 11:48:08 -0500 Subject: [PATCH 11/14] fixed comment, update validQueryArgs --- ctl/export.go | 2 +- handler.go | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/ctl/export.go b/ctl/export.go index fa6277f16..295e10039 100644 --- a/ctl/export.go +++ b/ctl/export.go @@ -31,7 +31,7 @@ type ExportCommand struct { // Name of the index & frame to export from. Index string Frame string - View string + View string // Filename to export to. Path string diff --git a/handler.go b/handler.go index a2e8868f1..f7b956bc6 100644 --- a/handler.go +++ b/handler.go @@ -844,7 +844,7 @@ func (h *Handler) readProtobufQueryRequest(r *http.Request) (*QueryRequest, erro // readURLQueryRequest parses query parameters from URL parameters from r. func (h *Handler) readURLQueryRequest(r *http.Request) (*QueryRequest, error) { q := r.URL.Query() - validQuery := h.getValidURLQuery(r) + validQuery := validOptions(QueryRequest{}) for key, _ := range q { if _, ok := validQuery[key]; !ok { return nil, errors.New("invalid query params") @@ -882,12 +882,13 @@ func (h *Handler) readURLQueryRequest(r *http.Request) (*QueryRequest, error) { }, nil } -func (h *Handler) getValidURLQuery(r *http.Request) map[string]bool { +// validOptions return all attributes of an interface with lower first character. +func validOptions(v interface{}) map[string]bool { validQuery := make(map[string]bool) - args := reflect.ValueOf(QueryRequest{}) + argsType := reflect.ValueOf(v) - for i := 0; i < args.Type().NumField(); i++ { - fieldName := args.Type().Field(i).Name + for i := 0; i < argsType.Type().NumField(); i++ { + fieldName := argsType.Type().Field(i).Name chars := []rune(fieldName) chars[0] = unicode.ToLower(chars[0]) fieldName = string(chars) From 51db1c77848327981a0eeb412401dfbbfde0603e Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Thu, 25 May 2017 12:07:09 -0500 Subject: [PATCH 12/14] fix review --- cmd/root.go | 2 +- cmd/root_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index e506f2fed..a34e8b0c7 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -126,7 +126,7 @@ func setAllConfig(v *viper.Viper, flags *pflag.FlagSet, envPrefix string) error for _, key := range v.AllKeys() { if _, ok := validTags[key]; !ok { - return fmt.Errorf("invalid tag: %v", key) + return fmt.Errorf("invalid option in configuration file: %v", key) } } diff --git a/cmd/root_test.go b/cmd/root_test.go index d3aeeab1f..e1aaaf31d 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -178,20 +178,20 @@ func TestRootCommand_Config(t *testing.T) { panic(err) } config := `data-dir = "/tmp/pil5_0" -bind = "127.0.0.1:15000" +bind = "127.0.0.1:10101" [cluster] poll-interval = "2m0s" replicas = 2 partitions = 128 hosts = [ - "127.0.0.1:15000", - "127.0.0.1:15001", + "127.0.0.1:10101", + "127.0.0.1:10111", ]` file.Write([]byte(config)) file.Close() _, err = ExecNewRootCommand(t, "server", "--config", file.Name()) - if err.Error() != "invalid tag: cluster.partitions" { - t.Fatalf("Expected invalid tag, but err: '%v'", err) + if err.Error() != "invalid option in configuration file: cluster.partitions" { + t.Fatalf("Expected invalid option in configuration file, but err: '%v'", err) } } From 3706fc90222d64877a2c79f994a44b5583fff615 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Thu, 25 May 2017 13:04:38 -0500 Subject: [PATCH 13/14] changed argType --- handler.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/handler.go b/handler.go index f7b956bc6..c4a2c2a06 100644 --- a/handler.go +++ b/handler.go @@ -885,10 +885,10 @@ func (h *Handler) readURLQueryRequest(r *http.Request) (*QueryRequest, error) { // validOptions return all attributes of an interface with lower first character. func validOptions(v interface{}) map[string]bool { validQuery := make(map[string]bool) - argsType := reflect.ValueOf(v) + argsType := reflect.ValueOf(v).Type() - for i := 0; i < argsType.Type().NumField(); i++ { - fieldName := argsType.Type().Field(i).Name + for i := 0; i < argsType.NumField(); i++ { + fieldName := argsType.Field(i).Name chars := []rune(fieldName) chars[0] = unicode.ToLower(chars[0]) fieldName = string(chars) From a9614734228bf13cf8781060f6ddc0ccf847c4e8 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 24 May 2017 20:10:52 -0600 Subject: [PATCH 14/14] Add statsd gauges for row count. --- fragment.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fragment.go b/fragment.go index 8c0964da1..bf29cae03 100644 --- a/fragment.go +++ b/fragment.go @@ -86,6 +86,9 @@ type Fragment struct { cache Cache CacheSize uint32 + // Stats reporting. + maxRowID uint64 + // Cache containing full rows (not just counts). rowCache BitmapCache @@ -166,6 +169,11 @@ func (f *Fragment) Open() error { // Clear checksums. f.checksums = make(map[int][]byte) + // Read last bit to determine max row. + pos := f.storage.Max() + f.maxRowID = pos / SliceWidth + f.stats.Gauge("rows", float64(f.maxRowID)) + return nil }(); err != nil { f.close() @@ -409,6 +417,12 @@ func (f *Fragment) setBit(rowID, columnID uint64) (changed bool, err error) { f.stats.Count("setN", 1) + // Update row count if they have increased. + if rowID > f.maxRowID { + f.maxRowID = rowID + f.stats.Gauge("rows", float64(f.maxRowID)) + } + return changed, nil }