cluster panic : should check field when new point

[CrazyJvm]

we saw influxdb panic when we use continuous queries in cluster mode, the panic code is :

func (w *WriteShardRequest) unmarshalPoints() []models.Point {
    points := make([]models.Point, len(w.pb.GetPoints()))
    for i, p := range w.pb.GetPoints() {
        pt, err := models.ParsePoints(p)
        if err != nil {
            // A error here means that one node parsed the point correctly but sent an
            // unparseable version to another node.  We could log and drop the point and allow
            // anti-entropy to resolve the discrepancy but this shouldn't ever happen.
            panic(fmt.Sprintf("failed to parse point: `%v`: %v", string(p), err))
        }
        points[i] = pt[0]
    }
    return points
}

after investigation , we found the root cause is lack of field check when new point, the current code will not throw any errors but will make above code panic. The reason why there may exist point without fields is that when cq calculate all points in an interval which there's no point in this interval at all.

func NewPoint(name string, tags Tags, fields Fields, time time.Time) (Point, error) {
    for key, value := range fields {
        if fv, ok := value.(float64); ok {
            // Ensure the caller validates and handles invalid field values
            if math.IsNaN(fv) {
                return nil, fmt.Errorf("NaN is an unsupported value for field %s", key)
            }
        }
    }

    return &point{
        key:    MakeKey([]byte(name), tags),
        time:   time,
        fields: fields.MarshalBinary(),
    }, nil
}

this PR add field check.

one more thing , really should panic here?

panic(fmt.Sprintf("failed to parse point: `%v`: %v", string(p), err))

It's better to log error than panic directly I think.

• CHANGELOG.md updated
• Rebased/mergable
• Tests pass
• Sign CLA (if not already signed)

[otoolep]

@CrazyJvm -- your change is failing due to formatting issues.

https://circleci.com/gh/influxdb/influxdb/7538#artifacts

run 'go tool vet --composites=false ./' to see the errors it flags and correct your source code. bash circle-test.sh returned exit code 1

[CrazyJvm]

retest this please

local : all tests passed and no format problem.

[CrazyJvm]

@otoolep fixed format & test.

[otoolep]

Thanks @CrazyJvm -- can you squash the commits? The messages are not very helpful, so one commit, with one message would be great.

[otoolep]

Minor: can you remove this comment? The code is quite clear without it.

[CrazyJvm]

sure

[otoolep]

This makes sense to me, I agree that a point without fields is nonsensical -- thanks @CrazyJvm

@jwilder -- can I get a +1?

+1 from me pending squash and minor requested changes.

[CrazyJvm]

@otoolep removed comment and squashed commits already, but seems introducing another commit. I'm not sure it's ok for you?

[otoolep]

You merged, you didn't rebase. Try this while on your branch:

git pull --rebase origin master

This assumes your origin is in sync with ours. You will also need to force push to your branch.