From 501b4ceedbad4e4f5ff8946c2b4f13024669a5ae Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Fri, 3 Apr 2015 15:18:13 -0700 Subject: [PATCH] Don't panic if presented with a field of unknown type This can happen, though is very unlikely. If this node receives encoded data, to be written to disk, and is queried for that data before its metastore is updated, there will be no field mapping for the data during decode. All this can happen because data is encoded by the node that first received the write request, not the node that actually writes the data to disk. So if this happens, skip the data. --- CHANGELOG.md | 1 + database.go | 25 +++++++++++++++++-------- influxdb.go | 4 ++++ server.go | 4 ++-- tx.go | 19 ++++++++++--------- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c377c511b03..6a79fcaf3ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - [#2165](https://github.com/influxdb/influxdb/pull/2165): Monitoring database and retention policy are not configurable. - [#2167](https://github.com/influxdb/influxdb/pull/2167): Add broker log recovery. - [#2050](https://github.com/influxdb/influxdb/issues/2050): Refactor `results` to `response`. Breaking Go Client change. +- [#2166](https://github.com/influxdb/influxdb/pull/2166): Don't panic if presented with a field of unknown type. ## v0.9.0-rc19 [2015-04-01] diff --git a/database.go b/database.go index 2c1a58531ec..b2cb800a87c 100644 --- a/database.go +++ b/database.go @@ -840,7 +840,12 @@ func (f *FieldCodec) DecodeByID(targetID uint8, b []byte) (interface{}, error) { } field, ok := f.fieldsByID[b[0]] if !ok { - panic(fmt.Sprintf("field ID %d has no mapping", b[0])) + // This can happen, though is very unlikely. If this node receives encoded data, to be written + // to disk, and is queried for that data before its metastore is updated, there will be no field + // mapping for the data during decode. All this can happen because data is encoded by the node + // that first received the write request, not the node that actually writes the data to disk. + // So if this happens, the read must be aborted. + return 0, ErrFieldUnmappedID } var value interface{} @@ -875,9 +880,9 @@ func (f *FieldCodec) DecodeByID(targetID uint8, b []byte) (interface{}, error) { } // DecodeFields decodes a byte slice into a set of field ids and values. -func (f *FieldCodec) DecodeFields(b []byte) map[uint8]interface{} { +func (f *FieldCodec) DecodeFields(b []byte) (map[uint8]interface{}, error) { if len(b) == 0 { - return nil + return nil, nil } // Create a map to hold the decoded data. @@ -893,7 +898,8 @@ func (f *FieldCodec) DecodeFields(b []byte) map[uint8]interface{} { fieldID := b[0] field := f.fieldsByID[fieldID] if field == nil { - panic(fmt.Sprintf("field ID %d has no mapping", fieldID)) + // See note in DecodeByID() regarding field-mapping failures. + return nil, ErrFieldUnmappedID } var value interface{} @@ -923,12 +929,15 @@ func (f *FieldCodec) DecodeFields(b []byte) map[uint8]interface{} { } - return values + return values, nil } // DecodeFieldsWithNames decodes a byte slice into a set of field names and values -func (f *FieldCodec) DecodeFieldsWithNames(b []byte) map[string]interface{} { - fields := f.DecodeFields(b) +func (f *FieldCodec) DecodeFieldsWithNames(b []byte) (map[string]interface{}, error) { + fields, err := f.DecodeFields(b) + if err != nil { + return nil, err + } m := make(map[string]interface{}) for id, v := range fields { field := f.fieldsByID[id] @@ -936,7 +945,7 @@ func (f *FieldCodec) DecodeFieldsWithNames(b []byte) map[string]interface{} { m[field.Name] = v } } - return m + return m, nil } // FieldByName returns the field by its name. It will return a nil if not found diff --git a/influxdb.go b/influxdb.go index f84d6db635d..0ea79615d3b 100644 --- a/influxdb.go +++ b/influxdb.go @@ -121,6 +121,10 @@ var ( // ErrFieldNotFound is returned when a field cannot be found. ErrFieldNotFound = errors.New("field not found") + // ErrFieldUnmappedID is returned when the system is presented, during decode, with a field ID + // there is no mapping for. + ErrFieldUnmappedID = errors.New("field ID not mapped") + // ErrSeriesNotFound is returned when looking up a non-existent series by database, name and tags ErrSeriesNotFound = errors.New("series not found") diff --git a/server.go b/server.go index 9132dedcc77..6b42020b375 100644 --- a/server.go +++ b/server.go @@ -2015,8 +2015,8 @@ func (s *Server) ReadSeries(database, retentionPolicy, name string, tags map[str // Decode into a raw value map. codec := NewFieldCodec(mm) - rawFields := codec.DecodeFields(data) - if rawFields == nil { + rawFields, err := codec.DecodeFields(data) + if err != nil || rawFields == nil { return nil, nil } diff --git a/tx.go b/tx.go index a394decbcbf..fc1bf578405 100644 --- a/tx.go +++ b/tx.go @@ -389,13 +389,14 @@ func (l *LocalMapper) Next() (seriesID uint32, timestamp int64, value interface{ var value interface{} var err error if l.isRaw && len(l.selectFields) > 1 { - fieldsWithNames := l.decoder.DecodeFieldsWithNames(l.valueBuffer[min]) - value = fieldsWithNames + if fieldsWithNames, err := l.decoder.DecodeFieldsWithNames(l.valueBuffer[min]); err == nil { + value = fieldsWithNames - // if there's a where clause, make sure we don't need to filter this value - if l.filters[min] != nil { - if !matchesWhere(l.filters[min], fieldsWithNames) { - value = nil + // if there's a where clause, make sure we don't need to filter this value + if l.filters[min] != nil { + if !matchesWhere(l.filters[min], fieldsWithNames) { + value = nil + } } } } else { @@ -409,8 +410,8 @@ func (l *LocalMapper) Next() (seriesID uint32, timestamp int64, value interface{ value = nil } } else { // decode everything - fieldsWithNames := l.decoder.DecodeFieldsWithNames(l.valueBuffer[min]) - if !matchesWhere(l.filters[min], fieldsWithNames) { + fieldsWithNames, err := l.decoder.DecodeFieldsWithNames(l.valueBuffer[min]) + if err != nil || !matchesWhere(l.filters[min], fieldsWithNames) { value = nil } } @@ -462,5 +463,5 @@ func splitIdent(s string) (db, rp, m string, err error) { type fieldDecoder interface { DecodeByID(fieldID uint8, b []byte) (interface{}, error) FieldByName(name string) *Field - DecodeFieldsWithNames(b []byte) map[string]interface{} + DecodeFieldsWithNames(b []byte) (map[string]interface{}, error) }