Avoid final garbage value from Fetch()

[thepaul]

I'm not 100% certain this is the right fix for everyone- I haven't
checked the librrd api or anything, but at least for my use case I
always get one garbage value at the end of my Fetch() results. Taking
out this "+ 1" makes the data match expectations.

[hnakamur]

Fetch() calls the C function rrd_fetch_r() in rrdtool
https://github.com/oetiker/rrdtool-1.x/blob/1.4.8/src/rrd_fetch.c#L183-L203
and rrd_fetch_r() calls rrd_fetch_fn()
https://github.com/oetiker/rrdtool-1.x/blob/1.4.8/src/rrd_fetch.c#L205-L483

In rrd_fetch_fn(), one malloc() is called for allocating a memory for two dimensional data
(dsCnt * rows * sizeof(rrd_value_t))
https://github.com/oetiker/rrdtool-1.x/blob/1.4.8/src/rrd_fetch.c#L340-L362

Since we are mapping Values []float64 in FetchResult to the malloc'ed memory,
we must use the same dsCnt and rows values.

rrd/rrd.go

Lines 396 to 405 in f3b7823

	type FetchResult struct {
	Filename string
	Cf string
	Start time.Time
	End time.Time
	Step time.Duration
	DsNames []string
	RowCnt int
	values []float64
	}

rrd/rrd_c.go

Lines 444 to 451 in f3b7823

	rowCnt := (int(cEnd)-int(cStart))/int(cStep) + 1
	valuesLen := dsCnt * rowCnt
	values := make([]float64, valuesLen)
	sliceHeader := (*reflect.SliceHeader)((unsafe.Pointer(&values)))
	sliceHeader.Cap = valuesLen
	sliceHeader.Len = valuesLen
	sliceHeader.Data = uintptr(unsafe.Pointer(cData))
	return FetchResult{filename, cf, start, end, step, dsNames, rowCnt, values}, nil

I skimmed the rrd_fetch_fn() source code,
https://github.com/oetiker/rrdtool-1.x/blob/1.4.8/src/rrd_fetch.c#L205-L483
but I could not figure out the last data at rows is garbage or not.

I think you can skip reading the last data without this pull request.

[thepaul]

Sure, you can skip reading the last data if you know about the problem. There is a workaround.

But handing back uninitialized values is usually not considered great library behavior.

[hnakamur]

I checked the rrdtutorial example
and noticed
rrdtool fetch test.rrd AVERAGE --start 920804400 --end 920809200
prints nan at 920809200. I also tested it myself with rrdtool 1.3.8 and confirmed this behavior.

The tutorial saids

The number that is written behind 920809200: in the list above covers the time range from 920808900 to 920809200, EXCLUDING 920809200.

Since Fetch() is meant to get the same result as rrdfetch, I think it's OK as it is.
But I leave the final decision to @ziutek

[thepaul]

I wasn't getting NaN, though. I was getting random garbage values.

[thepaul]

this may be convincing:

t.go

package main

import (
    "fmt"
    "os"
    "strconv"
    "time"

    "github.com/ziutek/rrd"
)

func main() {
    start, _ := strconv.Atoi(os.Args[2])
    end, _ := strconv.Atoi(os.Args[3])
    result, err := rrd.Fetch(os.Args[1], "AVERAGE", time.Unix(int64(start), 0),
        time.Unix(int64(end), 0), time.Second)
    if err != nil {
        panic(err)
    }
    for i := 0; i < result.RowCnt; i++ {
        fmt.Printf(" %d: %.10e\n",
            result.Start.Add(time.Duration(i+1)*result.Step).Unix(),
            result.ValueAt(0, i))
    }
}

output of rrdtool fetch on the test.rrd from the rrdtutorial example:

~% rrdtool fetch test.rrd AVERAGE --start 920804400 --end 920809200
                          speed

 920804700: -nan
 920805000: 4.0000000000e-02
 920805300: 2.0000000000e-02
 920805600: 0.0000000000e+00
 920805900: 0.0000000000e+00
 920806200: 3.3333333333e-02
 920806500: 3.3333333333e-02
 920806800: 3.3333333333e-02
 920807100: 2.0000000000e-02
 920807400: 2.0000000000e-02
 920807700: 2.0000000000e-02
 920808000: 1.3333333333e-02
 920808300: 1.6666666667e-02
 920808600: 6.6666666667e-03
 920808900: 3.3333333333e-03
 920809200: -nan
 920809500: -nan

output of test program 't':

~% ./t test.rrd 920804400 920809200
 920804700: NaN
 920805000: 4.0000000000e-02
 920805300: 2.0000000000e-02
 920805600: 0.0000000000e+00
 920805900: 0.0000000000e+00
 920806200: 3.3333333333e-02
 920806500: 3.3333333333e-02
 920806800: 3.3333333333e-02
 920807100: 2.0000000000e-02
 920807400: 2.0000000000e-02
 920807700: 2.0000000000e-02
 920808000: 1.3333333333e-02
 920808300: 1.6666666667e-02
 920808600: 6.6666666667e-03
 920808900: 3.3333333333e-03
 920809200: NaN
 920809500: NaN
 920809800: 0.0000000000e+00

..and in some cases (not yet sure which) the last value is something besides 0.0.

[hnakamur]

@thepaul Yes, I'm convinced now.
Thanks for your test :-)

Could you test your pull request for a case with dsCnt > 1?