From 68f341ea9f351769ce9649044fd9dabad0d51e21 Mon Sep 17 00:00:00 2001
From: Matt Jaffee <matthew.jaffee@gmail.com>
Date: Thu, 19 Jul 2018 12:06:31 -0500
Subject: [PATCH] fix race cond in translate_test

implements locked Reader method on test.TranslateFile. This race condition is
only present in tests, and is due to a replica referring directly to a primary
instead of through an http.TranslateStore as it would in production. This patch
requires the replica to obtain a lock while accessing the primary, and requires
test.Reopen to obtain a lock when swapping out the pilosa.TranslateStore.

I was able to reproduce the race by running: "go test
-run=TestTranslateFile_PrimaryTranslateStore -race -count=10", and could not
reproduce it after this patch.

Somewhat unrelated, I came across a "fatal error: fault" triggered by trying to
print an open pilosa.TranslateFile. This seems to be related to the mmapped
".data" field. I wrote the test to document the issue, but I don't think it's
easily fixable.
---
 translate_test.go | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/translate_test.go b/translate_test.go
index c73853d69..693cca759 100644
--- a/translate_test.go
+++ b/translate_test.go
@@ -4,11 +4,13 @@ import (
 	"bufio"
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"math/rand"
 	"os"
 	"reflect"
 	"strconv"
+	"sync"
 	"testing"
 	"time"
 
@@ -356,6 +358,24 @@ func TestTranslateFile_Reader(t *testing.T) {
 	})
 }
 
+func TestPrintTranslateFile(t *testing.T) {
+	// I think this is related to the mmap in s.Open.
+	t.Skip("causes fatal error: fault")
+	f, err := ioutil.TempFile("", "")
+	if err != nil {
+		panic(err)
+	}
+	f.Close()
+
+	s := pilosa.NewTranslateFile()
+	s.Path = f.Name()
+	err = s.Open()
+	if err != nil {
+		t.Fatalf("opening : %v", err)
+	}
+	fmt.Println("blah ", s)
+}
+
 func TestTranslateFile_PrimaryTranslateStore(t *testing.T) {
 	// Create a primary store that accepts writes.
 	primary := MustOpenTranslateFile()
@@ -495,6 +515,7 @@ func BenchmarkTranslateFile_TranslateColumnToString(b *testing.B) {
 }
 
 type TranslateFile struct {
+	lock sync.Mutex
 	*pilosa.TranslateFile
 }
 
@@ -510,6 +531,12 @@ func NewTranslateFile() *TranslateFile {
 	return s
 }
 
+func (t *TranslateFile) Reader(ctx context.Context, offset int64) (io.ReadCloser, error) {
+	t.lock.Lock()
+	defer t.lock.Unlock()
+	return t.TranslateFile.Reader(ctx, offset)
+}
+
 func MustOpenTranslateFile() *TranslateFile {
 	s := NewTranslateFile()
 	if err := s.Open(); err != nil {
@@ -536,7 +563,9 @@ func (s *TranslateFile) Reopen() error {
 		return err
 	}
 
+	s.lock.Lock()
 	s.TranslateFile = pilosa.NewTranslateFile()
+	s.lock.Unlock()
 	s.Path = prev.Path
 	s.PrimaryTranslateStore = prev.PrimaryTranslateStore
 	if err := s.Open(); err != nil {