set deadline before stats write/read

stats/config/init.go

@@ -3,6 +3,7 @@ package config
 import (
 	"flag"
 	"strings"
+	"time"
 
 	"github.com/grafana/metrictank/stats"
 	"github.com/raintank/worldping-api/pkg/log"
@@ -14,13 +15,15 @@ var prefix string
 var addr string
 var interval int
 var bufferSize int
+var timeout time.Duration
 
 func ConfigSetup() {
 	inStats := flag.NewFlagSet("stats", flag.ExitOnError)
 	inStats.BoolVar(&enabled, "enabled", true, "enable sending graphite messages for instrumentation")
 	inStats.StringVar(&prefix, "prefix", "metrictank.stats.default.$instance", "stats prefix (will add trailing dot automatically if needed)")
 	inStats.StringVar(&addr, "addr", "localhost:2003", "graphite address")
 	inStats.IntVar(&interval, "interval", 1, "interval at which to send statistics")
+	inStats.DurationVar(&timeout, "timeout", time.Second*10, "timeout after which a write is considered not successful")
 	inStats.IntVar(&bufferSize, "buffer-size", 20000, "how many messages (holding all measurements from one interval. rule of thumb: a message is ~25kB) to buffer up in case graphite endpoint is unavailable. With the default of 20k you will use max about 500MB and bridge 5 hours of downtime when needed")
 	globalconf.Register("stats", inStats)
 }
@@ -36,7 +39,7 @@ func ConfigProcess(instance string) {
 func Start() {
 	if enabled {
 		stats.NewMemoryReporter()
-		stats.NewGraphite(prefix, addr, interval, bufferSize)
+		stats.NewGraphite(prefix, addr, interval, bufferSize, timeout)
 	} else {
 		stats.NewDevnull()
 		log.Warn("running metrictank without instrumentation.")

stats/out_graphite.go

@@ -2,6 +2,7 @@ package stats
 
 import (
 	"bytes"
+	"io"
 	"net"
 	"time"
 
@@ -25,10 +26,11 @@ type Graphite struct {
 	prefix []byte
 	addr   string
 
+	timeout    time.Duration
 	toGraphite chan []byte
 }
 
-func NewGraphite(prefix, addr string, interval int, bufferSize int) {
+func NewGraphite(prefix, addr string, interval, bufferSize int, timeout time.Duration) {
 	if len(prefix) != 0 && prefix[len(prefix)-1] != '.' {
 		prefix = prefix + "."
 	}
@@ -44,6 +46,7 @@ func NewGraphite(prefix, addr string, interval int, bufferSize int) {
 		prefix:     []byte(prefix),
 		addr:       addr,
 		toGraphite: make(chan []byte, bufferSize),
+		timeout:    timeout,
 	}
 	go g.writer()
 	go g.reporter(interval)
@@ -80,7 +83,6 @@ func (g *Graphite) reporter(interval int) {
 }
 
 // writer connects to graphite and submits all pending data to it
-// TODO: conn.Write() returns no error for a while when the remote endpoint is down, the reconnect happens with a delay. this can also cause lost data for a second or two.
 func (g *Graphite) writer() {
 	var conn net.Conn
 	var err error
@@ -92,6 +94,7 @@ func (g *Graphite) writer() {
 			conn, err = net.Dial("tcp", g.addr)
 			if err == nil {
 				log.Info("stats now connected to %s", g.addr)
+				go g.checkEOF(conn)
 			} else {
 				log.Warn("stats dialing %s failed: %s. will retry", g.addr, err.Error())
 			}
@@ -105,6 +108,7 @@ func (g *Graphite) writer() {
 		var ok bool
 		for !ok {
 			conn = assureConn()
+			conn.SetDeadline(time.Now().Add(g.timeout))
 			pre := time.Now()
 			_, err = conn.Write(buf)
 			if err == nil {
@@ -118,3 +122,31 @@ func (g *Graphite) writer() {
 		}
 	}
 }
+
+// normally the remote end should never write anything back
+// but we know when we get EOF that the other end closed the conn
+// if not for this, we can happily write and flush without getting errors (in Go) but getting RST tcp packets back (!)
+// props to Tv` for this trick.
+func (g *Graphite) checkEOF(conn net.Conn) {
+	b := make([]byte, 1024)
+	for {
+		num, err := conn.Read(b)
+		if err == io.EOF {
+			log.Info("checkEOF conn.Read returned EOF -> conn is closed. closing conn explicitly")
+			conn.Close()
+			return
+		}
+
+		// just in case i misunderstand something or the remote behaves badly
+		if num != 0 {
+			log.Info("checkEOF conn.Read data? did not expect that.  data: %s\n", b[:num])
+			continue
+		}
+
+		if err != io.EOF {
+			log.Warn("checkEOF conn.Read returned err != EOF, which is unexpected.  closing conn. error: %s\n", err)
+			conn.Close()
+			return
+		}
+	}
+}